Bug 14788 - Memory leak if ioctl(FSCTL_VALIDATE_NEGOTIATE_INFO) fails before smbd_smb2_ioctl_send
Summary: Memory leak if ioctl(FSCTL_VALIDATE_NEGOTIATE_INFO) fails before smbd_smb2_io...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.12.14
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jule Anger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-08-08 11:58 UTC by Lev
Modified: 2021-12-15 14:55 UTC (History)
2 users (show)

See Also:


Attachments
Patch for testing (863 bytes, patch)
2021-08-16 15:30 UTC, Stefan Metzmacher
no flags Details
Fixed patch for testing (789 bytes, patch)
2021-08-17 09:54 UTC, Lev
no flags Details
Patches for v4-15-test (39.22 KB, text/plain)
2021-12-01 13:15 UTC, Stefan Metzmacher
slow: review+
Details
Patches for v4-14-test (39.34 KB, patch)
2021-12-01 13:16 UTC, Stefan Metzmacher
slow: review+
Details
Patches for v4-14-test (39.36 KB, patch)
2021-12-07 09:39 UTC, Stefan Metzmacher
slow: review+
Details
Patches for v4-14-test (41.25 KB, patch)
2021-12-08 13:37 UTC, Stefan Metzmacher
slow: review+
Details
Patches for v4-14-test (23.99 KB, patch)
2021-12-08 16:21 UTC, Stefan Metzmacher
metze: review? (slow)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lev 2021-08-08 11:58:16 UTC
To establish the SMB 3.x connection, windows client sends SMB2 request TREE_CONNECT and then ioctl(ctlcode=FSCTL_VALIDATE_NEGOTIATE_INFO). If this ioctl fails in fsctl_validate_neg_info(), it sets the flag disconnect = true, and then smbd_smb2_request_ioctl_done() -> smbd_server_connection_terminate() frees the connection object.

However, if for some reason this ioctl fails earlier, the connection can be leaked. 

Assume user doesn't have access to the root directory of the share. In my environment, for reproduction, I just run `chmod o-x /export/vol-2`.
Then on Windows client I run "net use \\10.2.5.26\vol-2 lev /USER:lev". smbd successfully handles TREE_CONNECT, but can't even start real processing of FSCTL_VALIDATE_NEGOTIATE_INFO, as it fails before, on chdir: smbd_smb2_request_dispatch -> smbd_smb2_request_check_tcon -> change_to_user_and_service -> chdir_current_service -> vfs_ChDir

[2021/08/08 12:59:13.307418, 10, pid=28084, effective(1009, 1009), real(1009, 0), class=smb2] ../../source3/smbd/smb2_server.c:2378(smbd_smb2_request_dispatch)
  smbd_smb2_request_dispatch: opcode[SMB2_OP_TCON] mid = 7
[2021/08/08 12:59:13.308055, 10, pid=28084, effective(0, 0), real(0, 0), class=smb2] ../../source3/smbd/smb2_tcon.c:222(smbd_smb2_tree_connect)
  smbd_smb2_tree_connect: path[\\10.2.5.26\vol-2] share[vol-2]
[2021/08/08 12:59:13.327998, 10, pid=28084, effective(0, 0), real(0, 0), class=smb2] ../../source3/smbd/smb2_server.c:3194(smbd_smb2_request_done_ex)
  smbd_smb2_request_done_ex: mid [7] idx[1] status[NT_STATUS_OK] body[16] dyn[no:0] at ../../source3/smbd/smb2_tcon.c:177
  
[2021/08/08 12:59:13.329014, 10, pid=28084, effective(0, 0), real(0, 0), class=smb2] ../../source3/smbd/smb2_server.c:2378(smbd_smb2_request_dispatch)
  smbd_smb2_request_dispatch: opcode[SMB2_OP_IOCTL] mid = 8
[2021/08/08 12:59:13.329169,  4, pid=28084, effective(0, 0), real(0, 0)] ../../source3/smbd/sec_ctx.c:361(set_sec_ctx_internal)
  setting sec ctx (1009, 1009) - sec_ctx_stack_ndx = 0
[2021/08/08 12:59:13.329272,  5, pid=28084, effective(0, 0), real(0, 0)] ../../libcli/security/security_token.c:57(security_token_debug)
  Security token SIDs (8):
    SID[  0]: S-1-5-21-2552871928-1080719513-403533899-1000
    SID[  1]: S-1-5-21-2552871928-1080719513-403533899-513
    SID[  2]: S-1-22-2-1009
    SID[  3]: S-1-22-2-27
    SID[  4]: S-1-1-0
    SID[  5]: S-1-5-2
    SID[  6]: S-1-5-11
    SID[  7]: S-1-22-1-1009
   Privileges (0x               0):
   Rights (0x               0):
[2021/08/08 12:59:13.329892,  5, pid=28084, effective(0, 0), real(0, 0)] ../../source3/auth/token_util.c:874(debug_unix_user_token)
  UNIX token of user 1009
  Primary group is 1009 and contains 2 supplementary groups
  Group[  0]: 1009
  Group[  1]: 27
[2021/08/08 12:59:13.330152,  4, pid=28084, effective(1009, 1009), real(1009, 0), class=vfs] ../../source3/smbd/vfs.c:923(vfs_ChDir)
  vfs_ChDir to /export/vol-2
[2021/08/08 12:59:13.330244,  0, pid=28084, effective(1009, 1009), real(1009, 0)] ../../source3/smbd/service.c:176(chdir_current_service)
  chdir_current_service: vfs_ChDir(/export/vol-2) failed: Permission denied. Current token: uid=1009, gid=1009, 2 groups: 1009 27
[2021/08/08 12:59:13.330396,  4, pid=28084, effective(1009, 1009), real(1009, 0), class=vfs] ../../source3/smbd/vfs.c:923(vfs_ChDir)
  vfs_ChDir to /export/vol-2
[2021/08/08 12:59:13.330474,  0, pid=28084, effective(1009, 1009), real(1009, 0)] ../../source3/smbd/service.c:188(chdir_current_service)
  chdir_current_service: vfs_ChDir(/export/vol-2) failed: Permission denied. Current token: uid=1009, gid=1009, 2 groups: 1009 27
[2021/08/08 12:59:13.330663,  3, pid=28084, effective(1009, 1009), real(1009, 0), class=smb2] ../../source3/smbd/smb2_server.c:3311(smbd_smb2_request_error_ex)
  smbd_smb2_request_error_ex: smbd_smb2_request_error_ex: idx[1] status[NT_STATUS_ACCESS_DENIED] || at ../../source3/smbd/smb2_server.c:2577
[2021/08/08 12:59:13.330790, 10, pid=28084, effective(1009, 1009), real(1009, 0), class=smb2] ../../source3/smbd/smb2_server.c:3194(smbd_smb2_request_done_ex)
  smbd_smb2_request_done_ex: mid [8] idx[1] status[NT_STATUS_ACCESS_DENIED] body[8] dyn[yes:1] at ../../source3/smbd/smb2_server.c:3368


On Windows I get

C:\Windows\System32>net use \\10.2.5.26\vol-2 lev /USER:lev
System error 5 has occurred.

Access is denied.

But on samba server side I still see the connection, that is never released.

root@vsa-000001bd:~# smbstatus -S -v
using configfile = /etc/samba/smb.conf

Service      pid     Machine       Connected at                     Encryption   Signing     
---------------------------------------------------------------------------------------------
vol-2        28084   10.2.5.92     Sun Aug  8 12:59:13 2021 IDT     -            -           

If Windows continues to try to establish the connection, it can easily lead to out-of-memory on the server side.
Comment 1 Stefan Metzmacher 2021-08-16 15:30:30 UTC
Created attachment 16739 [details]
Patch for testing

Can try and check if this patch helps?
Comment 2 Lev 2021-08-17 09:53:25 UTC
(In reply to Stefan Metzmacher from comment #1)

Hi Stefan,

Unfortunately the patch doesn't fix the issue. The reason is that ioctl FSCTL_VALIDATE_NEGOTIATE_INFO is a signed request (flags=0x8). So in smbd_smb2_request_dispatch() it is handled in the

  } else if (signing_required || (flags & SMB2_HDR_FLAG_SIGNED)) {
    ...

and not in the

  } else if (opcode == SMB2_OP_IOCTL) {
    ...

However if I modify the patch a bit (see attached), so that "if (opcode == SMB2_OP_IOCTL)" is checked unconditionally, the issue as fixed. The connection is successfully created, so Windows doesn't try to establish a new one. The connection is actually useless, "real" commands will fail with NT_STATUS_ACCESS_DENIED, but this is expected.

Notice also that Windows sometimes sends ioctl FSCTL_QUERY_NETWORK_INTERFACE_INFO, that may also fail in smbd_smb2_request_check_tcon(). This doesn't make a real problem, but not sure if this is ok to return NT_STATUS_ACCESS_DENIED here, maybe this ioctl should also be filtered out.

Thanks,
  -Lev.
Comment 3 Lev 2021-08-17 09:54:08 UTC
Created attachment 16740 [details]
Fixed patch for testing
Comment 4 Samba QA Contact 2021-12-01 11:52:04 UTC
This bug was referenced in samba master:

04a79139a42cfd1b607317dec041618cbf629584
735fc34682c541056fd912d07c69f299f961983c
1cd948d8520fd41a4e2f0cc6ee787c1e20211e33
bd3ba3c96e6ba811afd5898ff5470188557a6e33
c850ce96fd32ea91d8a31223bb09dd5b8b98d99e
b3212b359edb78d4c60fed377fa18478c8e75d9a
aab540503434817cc6b2de1d9c507f9d0b3ad980
fb33f145ff598b03a08098b7f12f3c53491f6c04
1744dd8c5bc342a74e397951506468636275fe45
629d161b8f579bc24acfaf3fe02612a5237345b4
f4d0bb164f028da46eab766135bb38175c117deb
Comment 5 Stefan Metzmacher 2021-12-01 13:15:30 UTC
Created attachment 17035 [details]
Patches for v4-15-test
Comment 6 Stefan Metzmacher 2021-12-01 13:16:06 UTC
Created attachment 17036 [details]
Patches for v4-14-test
Comment 7 Ralph Böhme 2021-12-03 11:51:05 UTC
Reassigning to Jule for inclusion in 4.14 and 4.15.
Comment 8 Samba QA Contact 2021-12-06 11:36:09 UTC
This bug was referenced in samba v4-15-test:

eba52e21acbb93b7bb13dc977afe196dc0e08352
685250e62981b5b41a43af2efd8ac61f18942913
4c8c39a7b55ad20a4c5282b0554562a06cf5c687
2209a095ddae1f1ea9acb802c041cb6d259813fa
9e182796362b2ac690556ad28d8a086f4044db8d
2c4c38679338ed62fe309379ee3069605a31bb22
a68e2904eaee1d7185bfe6981193a4bdeae7a2db
2306c9e7d18fe9080a20c2989144a35d43ef2a1d
f57b3ecccc1478bdf743956e7fef222e4891d508
c22480e2640ffc20fb01749f5f6a9ef272d855c8
6f7e39b061134ac2387c1d1ebfbe61c1c1a34422
Comment 9 Jule Anger 2021-12-06 11:57:55 UTC
Pushed to autobuild-v4-{15,14}-test.
Comment 10 Jule Anger 2021-12-06 13:05:24 UTC
Autobuild for v4.14 failed several times with messages like this:


Your autobuild on sn-devel-184 failed after 5.9 minutes
when trying to test samba-ad-dc-1-mitkrb5 with the following error:

   samba-ad-dc-1-mitkrb5: [make] failed 'make -j' with status 2

the autobuild has been abandoned. Please fix the error and resubmit.

A summary of the autobuild process is here:

  https://git.samba.org/janger/samba-autobuild-v4-14-test/autobuild.log

You can see logs of the failed task here:

  https://git.samba.org/janger/samba-autobuild-v4-14-test/samba-ad-dc-1-mitkrb5.stdout
  https://git.samba.org/janger/samba-autobuild-v4-14-test/samba-ad-dc-1-mitkrb5.stderr
Comment 11 Stefan Metzmacher 2021-12-07 09:39:12 UTC
Created attachment 17044 [details]
Patches for v4-14-test

Sorry, in 4.14 samba_cmdline_get_creds() needs to be replaced by popt_get_cmdline_credentials(). This compiles now...
Comment 12 Ralph Böhme 2021-12-07 16:56:56 UTC
Jule, please push the updated patch for 4.14. Thanks!
Comment 13 Stefan Metzmacher 2021-12-08 09:34:37 UTC
(In reply to Ralph Böhme from comment #12)


Pushed to autobuild-v4-14-test
Comment 14 Stefan Metzmacher 2021-12-08 13:37:08 UTC
Created attachment 17054 [details]
Patches for v4-14-test

The following commit was missing in 4.14 and caused all requests to
return NT_STATUS_ACCESS_DENIED.

commit f8f4a9faf099eb768eaa25f1e1a7d126b75291d0
Author:     Stefan Metzmacher <metze@samba.org>
AuthorDate: Tue Jul 13 16:37:42 2021 +0200
Commit:     Stefan Metzmacher <metze@samba.org>
CommitDate: Thu Jul 15 00:06:31 2021 +0000

    s3:smbd: remove dead code from smbd_smb2_request_dispatch()
    
    We have '} else if (signing_required || (flags & SMB2_HDR_FLAG_SIGNED)) {'
    before...
    
    Use 'git show -U52' to see the whole story...
    
    Signed-off-by: Stefan Metzmacher <metze@samba.org>
    Reviewed-by: Jeremy Allison <jra@samba.org>
Comment 15 Samba QA Contact 2021-12-08 14:57:14 UTC
This bug was referenced in samba v4-15-stable (Release samba-4.15.3):

eba52e21acbb93b7bb13dc977afe196dc0e08352
685250e62981b5b41a43af2efd8ac61f18942913
4c8c39a7b55ad20a4c5282b0554562a06cf5c687
2209a095ddae1f1ea9acb802c041cb6d259813fa
9e182796362b2ac690556ad28d8a086f4044db8d
2c4c38679338ed62fe309379ee3069605a31bb22
a68e2904eaee1d7185bfe6981193a4bdeae7a2db
2306c9e7d18fe9080a20c2989144a35d43ef2a1d
f57b3ecccc1478bdf743956e7fef222e4891d508
c22480e2640ffc20fb01749f5f6a9ef272d855c8
6f7e39b061134ac2387c1d1ebfbe61c1c1a34422
Comment 16 Stefan Metzmacher 2021-12-08 16:21:50 UTC
Created attachment 17055 [details]
Patches for v4-14-test

4.14 doesn't support working multichannel...

So we should skip the FSCTL_QUERY_NETWORK_INTERFACE_INFO
related changes.
Comment 17 Samba QA Contact 2021-12-13 09:45:05 UTC
This bug was referenced in samba v4-14-test:

4d2d5a3c66af111700191d384a6afac0fc759928
fd8864ef4fed6674d6f4f9ffedbe9366dd97a648
8eb06f10a12d0c6e787b354085d5b8e4378cf12d
ea6db15c31460e4ec07b70a5139960552632a8ba
fd82e1e4bad25696b62b4b308f2cc81b131e06a8
016d9c40bcafa7c654b56ec432ec23000a9cf4f0
25c97fc3a0f13515ad6910814dc8f19fc862e329
08eb470b9c580d3353c64dad34ba1c1109e1c042
Comment 18 Jule Anger 2021-12-13 13:05:10 UTC
Closing out bug report.

Thanks!
Comment 19 Samba QA Contact 2021-12-15 14:55:45 UTC
This bug was referenced in samba v4-14-stable (Release samba-4.14.11):

4d2d5a3c66af111700191d384a6afac0fc759928
fd8864ef4fed6674d6f4f9ffedbe9366dd97a648
8eb06f10a12d0c6e787b354085d5b8e4378cf12d
ea6db15c31460e4ec07b70a5139960552632a8ba
fd82e1e4bad25696b62b4b308f2cc81b131e06a8
016d9c40bcafa7c654b56ec432ec23000a9cf4f0
25c97fc3a0f13515ad6910814dc8f19fc862e329
08eb470b9c580d3353c64dad34ba1c1109e1c042