Implementing SMB_VFS_FCNTL in Samba

[Anoop C S]

On Fri, 2019-10-04 at 10:38 +0200, Ralph Boehme via samba-technical
wrote:
> On 10/3/19 3:20 PM, Anoop C S via samba-technical wrote:
> > On Thu, 2019-10-03 at 18:43 +0530, Anoop C S via samba-technical
> > wrote:
> > > OK. This should be it. I hope attached patch covers the missing
> > > part
> > > where recent fcntl() commands are detected during configure. A
> > > pipeline
> > > has been completed successfully for the attached patches.
> > > 
> > > https://gitlab.com/samba-team/devel/samba/pipelines/86263033
> > > 
> > > Reviews are appreciated.
> > 
> > Please ignore the previous version which had a typo in checking
> > HAVE_XX_XX inside vfs_default. Attaching the patches after
> > correction.
> 
> nice addition, thanks!
> 
> One nitpick and one general question.
> 
> Please don't do function calls in if expressions:
> 
> if ((val = SMB_VFS_FCNTL(fsp, F_GETFL, 0)) == -1) {
>     return -1;
> }
> 
> Instead:
> 
> val = SMB_VFS_FCNTL(fsp, F_GETFL, 0);
> if (val == -1) {
>     return -1;
> }

Right. I was also skeptical in putting it that way(frankly speaking it
is a copy of current set_blocking() function). :-)

Please see the new patch set attached.

> Then, I wonder why you make a copy of va_args in the time_audit and
> full_audit VFS modules before calling the NEXT function. Can't we
> just pass the va_list on to the NEXT function?

Two reasons:

* Both SMB_VFS_FCNTL and SMB_VFS_NEXT_FCNTL invoke same
smb_vfs_call_fcntl() function which does a va_start() on the received
variable arguments. Thus if va_list is passed as variable argument to
SMB_VFS_NEXT_FCNTL we again end up calling va_start() on it and a
subsequent va_arg() would not give us the required(/original) argument.
This lead me to do argument extraction before passing it to
SMB_VFS_NEXT_FCNTL. Now here comes the next problem..

* This is the prominent one and I mentioned in one of my previous
responses in this thread. Basically the program(in our case smbd)
crashes when va_arg() is done directly on the received va_list without
duplication. Following blog tries to explain this in some detail:

https://julio.meroh.net/2011/09/using-vacopy-to-safely-pass-ap.html

It may be platform dependent but va_copy() seems to be the right way.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-VFS-Add-SMB_VFS_FCNTL.patch
Type: text/x-patch
Size: 17330 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20191004/4751fd28/0001-s3-VFS-Add-SMB_VFS_FCNTL.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-s3-VFS-Use-SMB_VFS_FCNTL-to-set-fd-flags-in-open_fil.patch
Type: text/x-patch
Size: 2708 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20191004/4751fd28/0002-s3-VFS-Use-SMB_VFS_FCNTL-to-set-fd-flags-in-open_fil.bin>