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: NEW
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: Samba QA Contact
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-08-08 11:58 UTC by Lev
Modified: 2021-08-17 09:54 UTC (History)
1 user (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

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