Assume there are two users "dima" and "lev" connected to two shares "smb3" and "smb4" from the same Windows client: # net conf listshares global clone1 smb2 smb4 smb3 # net status sessions PID Username Group Machine ------------------------------------------------------------------- 23443 ZADARA2\dima ZADARA2\domain users 10.2.11.110 (ipv4:10.2.11.110:55382) 23443 ZADARA2\lev ZADARA2\domain users 10.2.11.110 (ipv4:10.2.11.110:55382) # net status shares Service pid machine Connected at ------------------------------------------------------- smb3 23443 10.2.11.110 Wed Dec 30 14:40:25 2020 smb4 23443 10.2.11.110 Wed Dec 30 14:40:26 2020 smb3 23443 10.2.11.110 Wed Dec 30 14:34:40 2020 smb4 23443 10.2.11.110 Wed Dec 30 00:04:17 2020 Now I want to delete the share "smb3" and close all connections to it: # net conf delshare smb3 delete_share_security: Failed to delete entry for share smb3: NT_STATUS_NOT_FOUND <-- harmless but annoying message from https://github.com/zadarastorage/zadara-samba/commit/ff35160dc6415e1ef5d57717d7636f38ea25a8f0#diff-fa14626f82ba5c586b2cbdcd183432dff0f605d68191cb0717db82a15600dc65 # echo $? 0 # net conf listshares global clone1 smb2 smb4 # smbcontrol all close-share smb3 [2020/12/30 14:45:07.602223, 1, pid=23443] ../../source3/smbd/conn_idle.c:128(conn_force_tdis) conn_force_tdis: Forcing close of share 'smb3' (wire_id=0x22f28611) [2020/12/30 14:45:07.602613, 1, pid=23443] ../../source3/smbd/conn_idle.c:128(conn_force_tdis) conn_force_tdis: Forcing close of share 'smb3' (wire_id=0x8ba65444) [2020/12/30 14:45:07.602813, 1, pid=23443] ../../source3/smbd/conn_idle.c:256(conn_force_tdis_done) conn_force_tdis_done: Closing share 'smb3' (wire_id=0x22f28611) [2020/12/30 14:45:07.603409, 1, pid=23443] ../../source3/smbd/service.c:1229(close_cnum) vm110 (ipv4:10.2.11.110:55382) closed connection to service smb3 [2020/12/30 14:45:07.610345, 1, pid=23443] ../../source3/smbd/conn_idle.c:256(conn_force_tdis_done) conn_force_tdis_done: Closing share 'smb3' (wire_id=0x8ba65444) [2020/12/30 14:45:07.611073, 1, pid=23443] ../../source3/smbd/service.c:1229(close_cnum) vm110 (ipv4:10.2.11.110:55382) closed connection to service smb3 # net status shares Service pid machine Connected at ------------------------------------------------------- smb4 23443 10.2.11.110 Wed Dec 30 14:40:26 2020 smb4 23443 10.2.11.110 Wed Dec 30 00:04:17 2020 However array ServicePtrs[] still contains configuration for "smb3"! # gdb -p 23443 (gdb) p iNumServices $4 = 5 (gdb) p ServicePtrs[4]->szService $5 = 0x7f778cd83ab0 "smb3" (gdb) p ServicePtrs[4]->valid $6 = true (gdb) p *ServicePtrs[4]->hosts_allow $7 = 0x7f778cd6fce0 "10.2.11.110" As a result users still may connect, despite the share was deleted from the registry: C:\Users\dima>net use \\10.2.11.26\smb3 The command completed successfully. [2020/12/30 14:51:39.481539, 1, pid=23443, effective(0, 0), real(0, 0)] ../../source3/smbd/service.c:947(make_connection_snum) vm110 (ipv4:10.2.11.110:55382) connect to service smb3 initially as user ZADARA2\dima (uid=5001116, gid=5000513) (pid 23443) # net status shares Service pid machine Connected at ------------------------------------------------------- smb4 23443 10.2.11.110 Wed Dec 30 14:40:26 2020 smb3 23443 10.2.11.110 Wed Dec 30 14:51:39 2020 smb4 23443 10.2.11.110 Wed Dec 30 00:04:17 2020 I think this is regression in the commit https://github.com/samba-team/samba/commit/7891302ab8eeba8261b92171a4d429e2f538b89a. Before this commit function conn_force_tdis() closed the required connections and then called reload_services(). But now reload_services() is called from conn_force_tdis_done() every time the connection is closed. In the scenario above it is called twice. When the 1st connection is closed, lp_file_list_changed() returns True, but then lp_killunused() -> conn_snum_used() returns True, because of the 2nd connection. And when the 2nd connection is closed, lp_file_list_changed() returns False, because the 1st call to lp_file_list_changed() updated f->modtime. So lp_killunused() nevers calls free_service_byindex(), and ServicePtrs[] remains not updated. * Connection to smb4 is needed to hold the process running after connections to smb3 are closed.
Analysis looks good. Any chance you can suggest a patch for this ? Thanks, Jeremy.
At a naive glance changing the last 'bool test' parameter from "true" to 'false' inside conn_force_tdis_done() might fix this. Can you check ?
(In reply to Jeremy Allison from comment #2) Hi Jeremy, Yes, simple patch diff --git a/source3/smbd/conn_idle.c b/source3/smbd/conn_idle.c index cf5a417bff7..1a774942b25 100644 --- a/source3/smbd/conn_idle.c +++ b/source3/smbd/conn_idle.c @@ -273,5 +273,5 @@ static void conn_force_tdis_done(struct tevent_req *req) * uid in the meantime. Ensure we're still root. */ change_to_root_user(); - reload_services(sconn, conn_snum_used, true); + reload_services(sconn, conn_snum_used, false); } fixed the issue. The only downside I see is that now reload_services() will do all its stuff after each connection close, and not only once, as it was in 4.11, but probably this is good enough. Thanks, Lev.
It's not that bad. Remember this code is only called from conn_force_tdis_done() (not the *FORCE* part :-), not the regular tcon disconnect path. So it's only called when smbcontrol is being used to force disconnect, not a regular client disconnect.
s/not the/note the/ (bad keyboard at home :-).
This could also be fixed by adding a tevent_wait_queue and moving the: change_to_root_user(); reload_services(sconn, conn_snum_used, true); calls into the trigger function of the last queue entry. But this might be too heavyweight a change for something that is only called because of an explicit admin smbcontrol request.
https://gitlab.com/samba-team/devel/samba/-/merge_requests/new?merge_request%5Bsource_branch%5D=jra-bug-14604-fix https://gitlab.com/samba-team/devel/samba/-/pipelines/247332087
https://gitlab.com/samba-team/samba/-/merge_requests/1775
Created attachment 16413 [details] git-am fix for 4.14.next, 4.13.next. Cherry-picked from master.
This bug was referenced in samba master: e4c8cd0781aef2a29bb4db1314c9fcd4f6edcecd
Pushed to autobuild-v4-{14,13}-test.
This bug was referenced in samba v4-13-test: 331e4d8363fee023b9ac56137e48592a9e1323d7
This bug was referenced in samba v4-14-test: 51577d22ef6dcb6099b87295262d84c3a7e989d2
Pushed to both branches. Closing out bug report. Thanks!
This bug was referenced in samba v4-14-stable (Release samba-4.14.0rc4): 51577d22ef6dcb6099b87295262d84c3a7e989d2
This bug was referenced in samba v4-13-stable (Release samba-4.13.5): 331e4d8363fee023b9ac56137e48592a9e1323d7