From bada89f2c70725620903031fefcd3fc062a57b36 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 10 Apr 2026 14:15:48 -0700 Subject: [PATCH 1/5] s3:loadparm: add lp_register_snum_in_use_fn() callback registration Add a mechanism for smbd to register a callback that checks whether a service number is currently in use by any active connection. This will be used by subsequent commits to guard free_service_byindex() calls in lp_servicenumber() and other sites that currently destroy services without checking if they are in use, which can leave active connections holding stale service numbers that lead to NULL pointer dereferences. The callback is registered by smbd during smbd_process() startup via connections_snum_used. Non-smbd programs (testparm, net, etc.) leave the callback as NULL, meaning no connections exist and it is always safe to free services. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14978 Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Jeremy Allison --- source3/param/loadparm.c | 15 +++++++++++++++ source3/param/loadparm.h | 2 ++ source3/smbd/smb2_process.c | 1 + 3 files changed, 18 insertions(+) diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index b081b917a82..02ad996c265 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -119,6 +119,21 @@ static bool defaults_saved = false; static struct loadparm_global Globals; +/* + * Callback to check if a service number is currently in use + * by any active connection. Registered by smbd at startup. + * When NULL (non-smbd programs), no connections exist so + * it is always safe to free services. + */ +static bool (*snum_in_use)(struct smbd_server_connection *unused, + int snum) = NULL; + +void lp_register_snum_in_use_fn(bool (*fn)(struct smbd_server_connection *, + int)) +{ + snum_in_use = fn; +} + /* This is a default service used to prime a services structure */ static const struct loadparm_service _sDefault = { diff --git a/source3/param/loadparm.h b/source3/param/loadparm.h index 72773a8b2ec..43f6c61804d 100644 --- a/source3/param/loadparm.h +++ b/source3/param/loadparm.h @@ -144,6 +144,8 @@ void lp_killunused(struct smbd_server_connection *sconn, bool (*snumused) (struct smbd_server_connection *, int)); void lp_kill_all_services(void); void lp_killservice(int iServiceIn); +void lp_register_snum_in_use_fn(bool (*fn)(struct smbd_server_connection *, + int)); const char* server_role_str(uint32_t role); enum usershare_err parse_usershare_file(TALLOC_CTX *ctx, SMB_STRUCT_STAT *psbuf, diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c index 9290714e8ae..f16a1496438 100644 --- a/source3/smbd/smb2_process.c +++ b/source3/smbd/smb2_process.c @@ -2031,6 +2031,7 @@ void smbd_process(struct tevent_context *ev_ctx, /* this is needed so that we get decent entries in smbstatus for port 445 connects */ set_remote_machine_name(remaddr, false); + lp_register_snum_in_use_fn(connections_snum_used); reload_services(sconn, conn_snum_used, true); sub_set_socket_ids(remaddr, sconn->remote_hostname, -- 2.47.3 From df7ee7f0fba31d55b8bf2a143b8c885e24baf2eb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 10 Apr 2026 14:19:01 -0700 Subject: [PATCH 2/5] s3:loadparm: guard free_service_byindex() in lp_servicenumber() with snum_in_use check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- source3/param/loadparm.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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. */ -- 2.47.3 From e19ceefe77d1b1a5c6f7a30263d9c9a09e4a4b3a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 10 Apr 2026 14:20:45 -0700 Subject: [PATCH 3/5] s3:srvsvc: guard lp_killservice() in _srvsvc_NetShareDel() with connections_snum_used check _srvsvc_NetShareDel() unconditionally calls lp_killservice() to destroy the service after deleting a share via RPC. If any active connection is still using this service number, the destroyed service can cause a NULL pointer dereference on subsequent requests. Guard the call with connections_snum_used() so the service is only freed when no connections are using it. The periodic load_usershare_shares() sweep will clean up the stale service once all connections have disconnected. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14978 Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Jeremy Allison --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index bcc99e576f8..94130be0352 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -2467,7 +2467,9 @@ WERROR _srvsvc_NetShareDel(struct pipes_struct *p, /* Delete the SD in the database. */ delete_share_security(share_name); - lp_killservice(snum); + if (!connections_snum_used(NULL, snum)) { + lp_killservice(snum); + } return WERR_OK; } -- 2.47.3 From c39998ac7dfb0f827c89f6040763465fc2fe019d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 10 Apr 2026 14:21:55 -0700 Subject: [PATCH 4/5] s3:smbd: guard lp_killservice() in delete_and_reload_printers() with connections_snum_used check delete_and_reload_printers() unconditionally calls lp_killservice() to destroy autoloaded printer services that are no longer in the printer list. If any active connection is still using the printer service number, the destroyed service can cause a NULL pointer dereference on subsequent requests. Guard the call with connections_snum_used() so the service is only freed when no connections are using it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14978 Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Jeremy Allison --- source3/smbd/server_reload.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c index d3322d12f6a..eba87cc98ca 100644 --- a/source3/smbd/server_reload.c +++ b/source3/smbd/server_reload.c @@ -109,7 +109,8 @@ void delete_and_reload_printers(void) /* check printer, but avoid removing non-autoloaded printers */ if (lp_autoloaded(snum) && - !printer_list_printername_exists(pname)) { + !printer_list_printername_exists(pname) && + !connections_snum_used(NULL, snum)) { lp_killservice(snum); } } -- 2.47.3 From b0182f2488772ac40ff3b3ff7d933f360fc16b8d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 10 Apr 2026 14:24:34 -0700 Subject: [PATCH 5/5] s3:loadparm: fix NULL pointer dereference in volume_label() volume_label() calls lp_servicename() as a fallback when lp_volume() returns an empty string. lp_servicename() is a FN_LOCAL_SUBSTITUTED_STRING that falls back to sDefault.szService when the service is invalid. Since sDefault.szService is initialized to NULL and is never set by init_globals(), the substitution returns NULL, and the subsequent strlen() call crashes with a segmentation fault. Add a NULL guard so volume_label() returns an empty string instead of crashing. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14978 Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Jeremy Allison --- source3/param/loadparm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index e85b52fdc6d..dc0d8523172 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -4471,6 +4471,9 @@ const char *volume_label(TALLOC_CTX *ctx, int snum) if (!*label) { label = lp_servicename(ctx, lp_sub, snum); } + if (label == NULL) { + label = ""; + } /* * Volume label can be a max of 32 bytes. Make sure to truncate -- 2.47.3