s3:loadparm: guard free_service_byindex() in lp_servicenumber() with snum_in_use...

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) <noreply@anthropic.com>
Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: David Mulder <dmulder@samba.org>

diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 02ad996c2653577ee8885a04fa17915f49d66ad2..e85b52fdc6d87be8d768ca3a1f8fbf14806309e7 100644 (file)
--- 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. */