[PATCH] Fix streams_xattr (and fruit) with kernel oplocks

[Richard Sharpe]

On Thu, Jul 27, 2017 at 11:15 AM, Ralph Böhme via samba-technical
<samba-technical at lists.samba.org> wrote:
> Hi folks,
>
> attached find a fix for bug 12791: when kernel oplocks are enabled any VFS
> module that does an open() on the underlying base file when processing a client
> "stream open" request is doomed to cause all sort of havoc as it may trigger a
> kernel oplock break which triggers panics and errors in the oplock break
> handlers and the async open handling.
>
> The basic idea of the fix is to *not* open() the underlying file for stream
> related ops if it's not really needed and can't be replaced with a path-name
> based equivalent. Luckily, in streams_xattr and fruit all calls can be replaced
> with path-name based equivalents -- problem solved.
>
> As the open() implementation in the VFS module has to return *something* that is
> not (int)-1 and is ideally unique per open (although that's probably not really
> required, but read on) I've opted to return a pipe fd, this satisfies both
> conditions.
>
> The patchset has been running at a large customer site for 4 weeks and so far
> the previous daily crashes are gone. It also just passed another private
> autobuild.
>
> Please review & push if happy. Thanks!

I have read through the patch and it looks OK, although I will need to
read it carefully one more time before I can approve it.

However, did you consider O_PATH? Since this seems to be Linux
specific (kernel oplocks) that might have avoided the need for a pipe,
although I will have to check the kernel code to see of an open with
O_PATH will trigger a kernel oplock break. Any attempt to use an FD
opened with O_PATH for anything serious will result in EBADF ...

Also, there seems to be some funny space vs tab issue here:

+static int streams_xattr_fchown(vfs_handle_struct *handle, files_struct *fsp,
+                               uid_t uid, gid_t gid)
+{
+        struct stream_io *sio =
+               (struct stream_io *)VFS_FETCH_FSP_EXTENSION(handle, fsp);
+
+       if (sio == NULL) {
+               return SMB_VFS_NEXT_FCHOWN(handle, fsp, uid, gid);
+       }
+
+       return 0;
+}
+

and all the others in vfs_streams_xattr.c. The struct stream_io like
seems to have spaces at the beginning while the rest have tabs.

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)