Since 4.14 open_file() in source3/smbd/open.c has been restructured to use a function reopen_from_procfd(). This results in (when writing to an existing file) the following sequence of system calls:
fd = openat (AT_FDCWD, filename, O_RDONLY|O_NOFOLLOW);
new_fd = openat(AT_FDCWD, "/proc/self/fd/<fd>", O_RDWR|O_CREAT|O_NONBLOCK, 0744);
This works fine on Linux, but on NetBSD (and it seems FreeBSD and OpenBSD) the second openat() fails with an EEXIST.
The issue can be demonstrated by using notepad to edit an existing writable file on a samba share. Modify the file, at the point you try to "Save" you get a dialog box saying the file exists. The same works perfectly with 4.13.x.
Some discussion of the issue can be seen here:
The second openat() isn't specifying O_EXCL, so EEXIST would seem to be a kernel bug here.
The description of open error return codes says:
O_CREAT and O_EXCL are set, and the named file exists.
(In reply to Jeremy Allison from comment #1)
> The second openat() isn't specifying O_EXCL, so EEXIST would seem to be a
> kernel bug here.
You might consider that, but if so its a bug on all the BSD's
Yep, looks like a generic kernel bug in the *BSD's that they don't follow POSIX I'm afraid. You should log a bug with the OS's affected.
(In reply to Jeremy Allison from comment #3)
What if we just filter out the O_CREAT in this case? We know we're opening an existing file so we don't need to pass O_CREAT. Easy fix for the BSDs and not really a big heritage to carry along.
Yes, that might work. Worries me a bit though that the semantics around opening /proc/XX/fd's in the BSD's is not well defined though.
Having said that it's not well defined in Linux either, Linux is just the 800lb gorilla here.
Created attachment 17130 [details]
patch to drop O_CREAT from fd opening
Is the attached patch what has been suggested here? Any side-effects I do not see (and I don't see that well around those sources)?
So far, the NetBSD packages are still stuck at samba 4.13 because of this.
OpenBSD is unaffected by this because it doesn't have /proc at all.
On illumos, reopening files from /proc doesn't seem to work due to permission problems. Samba is able to open /proc/self/pid/n readonly, but O_RW with or without O_CREAT fails, I think because the kernel checks the permissions on /proc as well as the actual file, and the /proc stuff is owned by root and samba is at that point running as the connected user.
Unfortunately, and there's not much we can do about this - Linux is the most popular platform for Samba. So we write to the Linux /proc semantics, and use a non-proc fallback for other platforms.
I'm happy to entertain fixes for non-Linux platforms to allow them to get the security benefits of /proc, but to be honest it would be much easier for them to conform to how Linux supports /proc. Yes I know this sucks, and is a new form of "Windows everywhere". But that's the world Samba has to live in.
As I said, if you can make our /proc code work with non-Linux /proc semantics I'm happy to look at patches.
So what about my little suggested patch attached to this bug?
(In reply to Thomas Orgis from comment #10)
Yes, that actually looks like a good fix to me.