Created attachment 8366 [details]
Client network capture when error occurs

No, bug #9196 is SMB2-only.

I'll try and take a look at these logs over the holidays.

Jeremy.

Any news on this one? I just upgraded to 4.0.3 and this bug is still present:

> CIFS VFS: server 10.0.0.1 of type Samba 4.0.3 returned unexpected error on SMB posix open, disabling posix open support. Check if server update available.

Ok, it's a client bug in the Linux kernel cifsfs.

In the network capture look at packet 2222:

In the data section of the posix_open we have:

Offset           Value (converted from little-endian to native byte order)
0000             0x02
0004             0x21

The 0x2 at offset 0 == REQUEST_OPLOCK

However, at offset 4 we have the wire_open_mode. 0x21 maps to:

#define SMB_O_EXCL                       0x20
#define SMB_O_RDONLY                      0x1

But sending O_EXCL on its own without SMB_O_CREAT makes no sense, so the Samba server code says:

        /* First take care of O_CREAT|O_EXCL interactions. */
        switch (wire_open_mode & (SMB_O_CREAT | SMB_O_EXCL)) {
                case (SMB_O_CREAT | SMB_O_EXCL):
                        /* File exists fail. File not exist create. */
                        create_disp = FILE_CREATE;
                        break;
                case SMB_O_CREAT:
                        /* File exists open. File not exist create. */
                        create_disp = FILE_OPEN_IF;
                        break;
                case 0:
                        /* File exists open. File not exist fail. */
                        create_disp = FILE_OPEN;
                        break;
                case SMB_O_EXCL:
                        /* O_EXCL on its own without O_CREAT is undefined. */
                default:
                        DEBUG(5,("smb_posix_open: invalid create mode 0x%x\n",
                                (unsigned int)wire_open_mode ));
                        return NT_STATUS_INVALID_PARAMETER;
        }

Note in the SMB_O_EXCL case we fall through to the error NT_STATUS_INVALID_PARAMETER case, which is exactly what we see in the reply packet 2223.

I'm going to re-assign this bug to Jeff and Steve as they are the client maintainers.

Jeremy.

Probably, it's time to think about removing that hack that disables POSIX open support when we get back an EINVAL.

In any case, the code that sets these flags just mirrors what the application sends:

        if ((flags & O_ACCMODE) == O_RDONLY)
                posix_flags = SMB_O_RDONLY;
[...]

        if (flags & O_CREAT)
                posix_flags |= SMB_O_CREAT;
        if (flags & O_EXCL)
                posix_flags |= SMB_O_EXCL;

...could it be that the userland app is sending down these nonsensical flag combinations? I'd be hesitant to second-guess what the application is passing.

Also, on Linux at least, if I pass down a flag combination of O_RDONLY|O_EXCL when open a file on a local fs, the O_EXCL is just ignored:

    open("test", O_RDONLY|O_EXCL)           = 3

Jeremy, perhaps samba is being just a little too strict here? If the goal is to mirror local fs semantics, then it probably ought to just ignore a bare SMB_O_EXCL flag. Maybe that should read:

                case SMB_O_EXCL:
                case 0:
                        /* File exists open. File not exist fail. */
                        create_disp = FILE_OPEN;
                        break;

...with a nice comment explaining why that's being done?

What would be nice though is to also verify that the userland app (libreoffice?) is actually doing this and have them fix it too. I imagine it would cause problems on some platforms, even if Linux is generally tolerant of it.

I'm sure the application is just passing down O_EXCL to you. The problem is without O_CREAT that is undefined. From man 2 open.

       O_EXCL Ensure that this call creates the file: if this flag  is  speci‐
              fied  in  conjunction with O_CREAT, and pathname already exists,
              then open() will fail.

              When these two flags are specified, symbolic links are not  fol‐
              lowed: if pathname is a symbolic link, then open() fails regard‐
              less of where the symbolic link points to.

              In general, the behavior of O_EXCL is undefined if  it  is  used
              without  O_CREAT.   There  is  one  exception:  on Linux 2.6 and
              later, O_EXCL can be used without O_CREAT if pathname refers  to
              a  block  device.   If  the block device is in use by the system
              (e.g., mounted), open() fails with the error EBUSY.

You can't break applications, so my advice would be to just silently strip out O_EXCL if passed in without O_CREAT before sending it to the server.

Jeremy.

So yeah, I can fix this in the server. But I also recommend you fix this in the client as well. You should never be sending a bare O_EXCL on the wire (IMHO).

Jeremy.

(In reply to comment #6)
> Also, on Linux at least, if I pass down a flag combination of O_RDONLY|O_EXCL
> when open a file on a local fs, the O_EXCL is just ignored:
> 
>     open("test", O_RDONLY|O_EXCL)           = 3
> 
> Jeremy, perhaps samba is being just a little too strict here? If the goal is to
> mirror local fs semantics, then it probably ought to just ignore a bare
> SMB_O_EXCL flag. Maybe that should read:
> 
>                 case SMB_O_EXCL:
>                 case 0:
>                         /* File exists open. File not exist fail. */
>                         create_disp = FILE_OPEN;
>                         break;
> 
> ...with a nice comment explaining why that's being done?
> 
> What would be nice though is to also verify that the userland app
> (libreoffice?) is actually doing this and have them fix it too. I imagine it
> would cause problems on some platforms, even if Linux is generally tolerant of
> it.

I tried this on libreoffice 3.4.6 to Samba 4.0.1-pre and don't see the hang

well, I can reproduce it if I write a C program to force an open with O_EXCL | O_RDNOLY.

You get different errors if the file doesn't exist vs. if it does exits, but it looks like it is similar whether the file is local or remote (at least on 3.8-rc6) ... other than we disable posix support incorrectly (when the client pases us the strange flags) and the server returns the error.  We should fix the client to not turn off posix just because an app passed a bad flag combination.

Since the fix for the Samba 3.3 (and earlier bug) in 64cc2c63694a03393985ffc8b178e72f52dd8a06 shows that we have to check for EINVAL on posix open, and Samba 3.3 and earlier is not that rare...then I think we are probably better off masking the flag if there is a common app that sends it so we don't run into this on Samba 4.

What is the version of libreoffice (was wondering if they fixed it)?

and had anyone tried this on Samba 3.4, 3.5 or 3.6?

Samba 3.6 returned the same error

Created attachment 8541 [details]
this patch masks the flag ... but I think it is too aggressive a warning and makes it looks like a kernel bug rather than an app bug.

Created attachment 8542 [details]
2nd try at kernel patch to warn on illegal app behavior (and ignoring O_EXCL)

I like this patch better.  Although it doesn't include details about the app name, at least it warns once in a way that implies the app needs to be fixed not that there is a kernel bug.

Created attachment 8544 [details]
git-am fileserver patch.

Can I get a review of this fileserver patch ?

Once you've +1'ed it I'll get it into master/4.0.next/3.6.next.

Cheers,

Jeremy.

Comment on attachment 8544 [details]
git-am fileserver patch.

Looks good to me.

(In reply to comment #11)
> What is the version of libreoffice (was wondering if they fixed it)?
Right now I am using LibreOffice 3.6.2 (Ubuntu 12.10) and this version also triggers this error. But only when the $HOME folder (or actually ~/.config/libreoffice/) is CIFS-mounted. Opening documents with LibreOffice from a CIFS share with a local $HOME does not trigger the posix_open error, so it must be caused by an operation within ~/.config/libreoffice/.

There are actually even more applications causing these hangs on SMB posix open, but with LibreOffice this was 100% reproducible for me. It is sometimes hard to find the actual culprit with a CIFS-mounted $HOME - maybe a few Samba Devs should consider this setup for their personal use, as it might uncover more bugs related to Samba and the Linux CIFS client ;)

Anyway, it is nice that you are looking into this issue.

Patch applies cleanly to 3.6.next and 4.0.next - re-assigning to Karolin for inclusion.

Jeremy.

Comment on attachment 8542 [details]
2nd try at kernel patch to warn on illegal app behavior (and ignoring O_EXCL)

It doesn't tell you anything about the application that caused that error to fire though which makes it pretty useless for tracking it down. I'd probably just make it ignore the O_EXCL when O_CREAT isn't set, and leave out the cERROR message.

At the same time, it's probably a good idea to think about deprecating the "broken_posix_open" workaround. Changing the client's behavior on an EINVAL return has always been a bit dodgy. There are plenty of ways to get that sort of error back that don't involve a broken server.

Pushed to v3-6-test and autobuild-v4-0-test.

Pushed to v4-0-test.
Closing out bug report.

Thanks!