Leak of file descriptor in samba 4.5.11

[Kumar, Arjit (SSTO)]

I have ported and tested the patch.
This patch has fixed FD leak.

Regards,
Arjit Kumar


-----Original Message-----
From: Jeremy Allison [mailto:jra at samba.org] 
Sent: Thursday, February 15, 2018 3:06 AM
To: slow at samba.org; samba-technical at lists.samba.org; Kumar, Arjit (SSTO) <arjit.kumar at hpe.com>
Cc: jra at samba.org
Subject: Re: Leak of file descriptor in samba 4.5.11

On Wed Feb 14 17:07:55 UTC 2018 Kumar, Arjit (SSTO) arjit.kumar at hpe.com  wrote:

> Hi Team,
>
> There seems to be FD leak happening for Samba share, due to which accessing file fails with below error, after accessing certain no of files.
>
> ../source3/smbd/smb2_server.c:2988(smbd_smb2_request_done_ex)
>  smbd_smb2_request_done_ex: idx[1] 
> status[NT_STATUS_TOO_MANY_OPENED_FILES] body[8] dyn[yes:1] at 
> ../source3/smbd/smb2_server.c:3145
>
> This occurs even if we increase smbd's max open files to 65536.
>
> This issue is not seen in samba 4.5.3, but seen with 4.5.11 and 4.5.15
>
> This issue seems to be caused by fix for CVE-2017-2619.
>
> On investigating further below modification in source3/smbd/smb2_query_directory.c for fixing above CVE  may be causing FD leak.
>
> https://github.com/samba-team/samba/commit/47b6b6f8f58efbabd7e4610f51d
> b61dca2bc157c#diff-30edf5566a0d9e2abf214c7f778830df
>
> Line 328: dptr_CloseDir(fsp);
> Do we need to close FD using fd_close() instead of dptr_CloseDir()
>
> I also have opened a bug<https://bugzilla.samba.org/show_bug.cgi?id=13270> for the same on bugzilla.
>
> Simple steps to re  produce it are as below:-
>
>
> *         Map a samba share on windows machine.
> Suppose share is mapped on Z drive.
>
> *         Create a small batch file as below.
> :loop
>
> dir z:
>
> goto loop
>
> *         Analysis it with any crash dump utility depending on platform or wait until smbd hits  NT_STATUS_TOO_MANY_OPENED_FILES error

Hi Arjit,

I think your analysis is correct here (we should be using
fd_close() instead of dptr_CloseDir()). I think the reason it is biting you and not others is that HPUX doesn't support fdopendir(), which would hide the problem (the destructor would then correctly close the underlying fd).

Can you test the following patch (I've also attached to the bug report) ? It shouldn't change anything on systems that support fdopendir(), but should fix the leak for HPUX.

Cheers,

	Jeremy.