"stat open" lseek(fd = -1)

[David Disseldorp]

On Fri, 1 Mar 2019 13:36:49 -0800, Jeremy Allison wrote:

> On Wed, Feb 27, 2019 at 04:52:28PM +0100, David Disseldorp via samba-technical wrote:
> > Hi Samba archaeologists,
> > 
> > Explicit "stat open" tracking was removed way back in 2002 with
> > b9e91d2a8e41a43d7ebb7d7eed807a7d8de9b329. This change instead added a
> > "fd == -1" check to the vfs_default lseek handler, which has remained to
> > this day.
> > Does anybody know whether this lseek(fd = -1) condition can still /
> > could ever be triggered? If so, how?  
...
> So the only question is SMB_VFS_LSEEK still used as we now use pread
> for everything (or we should).
> 
> $ git grep SMB_VFS_LSEEK gives:
> 
> docs-xml/Samba-Developers-Guide/vfs.xml:#define SMB_VFS_LSEEK(fsp, fd, offset, whence) \
> source3/include/vfs_macros.h:#define SMB_VFS_LSEEK(fsp, offset, whence) \
> source3/smbd/reply.c:           if((res = SMB_VFS_LSEEK(fsp,startpos,umode)) == -1) {
> source3/smbd/reply.c:                                   res = SMB_VFS_LSEEK(fsp,0,SEEK_SET);
> source3/smbd/reply.c:           ret = SMB_VFS_LSEEK(fsp2, 0, SEEK_END);
> source3/smbd/smb2_ioctl_filesys.c:              data_off = SMB_VFS_LSEEK(fsp, curr_off, SEEK_DATA);
> source3/smbd/smb2_ioctl_filesys.c:              hole_off = SMB_VFS_LSEEK(fsp, data_off, SEEK_HOLE);
> source3/torture/cmd_vfs.c:      pos = SMB_VFS_LSEEK(vfs->files[fd], offset, whence);
> 
> The calls inside smb2_ioctl_filesys.c are protected
> by:
> 
>        /* READ_DATA permission is required */
>         status = check_access_fsp(fsp, FILE_READ_DATA);
> 
> so they should be OK.
> 
> source3/torture/cmd_vfs.c is merely a test.
> 
> The calls in reply.c come from:
> 
> 1). reply_lseek(), which doesn't check access permissions (so might
> be called on a stat open).
> 
> 2). reply_copy(), which opens src and dest as real, not stat
> files.
> 
> So I think SMB1 with an SMBlseek call is the only possible
> caller here.
> 
> Does that help ?

Yes, very much so - thanks Jeremy!

reply_lseek() bails on check_fsp() failure (triggered on -1 fd), so I
think that covers all callers and allows for removal of the VFS check.
Please see attached.

Cheers, David
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-vfs-drop-lseek-stat-open-checks.patch
Type: text/x-patch
Size: 1825 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190304/5e6d1d2a/0001-vfs-drop-lseek-stat-open-checks.bin>