From: Jeremy Allison Date: Fri, 10 Apr 2026 21:19:01 +0000 (-0700) Subject: s3:loadparm: guard free_service_byindex() in lp_servicenumber() with snum_in_use... X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=de74e24314ebfbd37d14b8fce21f7382cc6b76db;p=thirdparty%2Fsamba.git s3:loadparm: guard free_service_byindex() in lp_servicenumber() with snum_in_use check lp_servicenumber() calls free_service_byindex() to destroy usershare services when usershare_exists() returns false or when the usershare file has been modified. This is unsafe because active connections may still hold the service number — the destroyed service leaves a NULL ServicePtrs[] entry that causes a NULL pointer dereference when the connection subsequently calls lp_servicename() or similar functions. The crash path is: get_referred_path() -> lp_servicenumber() -> usershare_exists() fails (e.g. EACCES) -> free_service_byindex() destroys service -> later request on same connection -> volume_label() -> lp_servicename() -> FN_LOCAL_SUBSTITUTED_STRING falls back to sDefault.szService (NULL) -> strlen(NULL) -> SIGSEGV Guard both free_service_byindex() call sites with the snum_in_use callback registered in the previous commit. When the service is in use by an active connection, skip the destruction and let the periodic load_usershare_shares() mark-and-sweep handle cleanup safely via its conn_snum_used() check. When snum_in_use is NULL (non-smbd programs), the original behaviour is preserved — services are freed immediately since no connections can exist. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14978 Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Jeremy Allison Reviewed-by: David Mulder --- diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index 02ad996c265..e85b52fdc6d 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -4412,6 +4412,15 @@ int lp_servicenumber(const char *pszServiceName) struct timespec last_mod; if (!usershare_exists(iService, &last_mod)) { + if (snum_in_use != NULL && + snum_in_use(NULL, iService)) { + /* + * Service is in use, don't destroy it. + * The periodic load_usershare_shares() + * sweep will clean it up safely. + */ + return GLOBAL_SECTION_SNUM; + } /* Remove the share security tdb entry for it. */ delete_share_security(lp_const_servicename(iService)); /* Remove it from the array. */ @@ -4423,6 +4432,15 @@ int lp_servicenumber(const char *pszServiceName) /* Has it been modified ? If so delete and reload. */ if (timespec_compare(&ServicePtrs[iService]->usershare_last_mod, &last_mod) < 0) { + if (snum_in_use != NULL && + snum_in_use(NULL, iService)) { + /* + * Service is in use, defer the reload + * to load_usershare_shares() which can + * safely handle in-use services. + */ + return iService; + } /* Remove it from the array. */ free_service_byindex(iService); /* and now reload it. */