Bug 15563 - Attempting to create a file with extended attributes results in an empty file being created if the underlying filesystem does not support user xattrs
Summary: Attempting to create a file with extended attributes results in an empty file...
Status: NEW
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.19.4
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Samba QA Contact
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-01-29 00:00 UTC by Etienne Dechamps
Modified: 2024-02-06 18:38 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Etienne Dechamps 2024-01-29 00:00:36 UTC
If the following conditions are met:

- smb.conf `ea support` is on (which is the case by default)
- Samba is serving from a filesystem that does not support user xattrs (for example, tmpfs)
- A client attempts to create a file with extended attributes (for example, a Windows client copying a file that has the "downloaded from the web" attribute)

Then the following will occur:

- The file creation request will return NT_STATUS_EAS_NOT_SUPPORTED, which makes sense;
- But a zero-length file will be created anyway and left behind, which does not.

In the case of Windows Explorer, this then leads to Explorer being confused and attempting to create the file again without extended attributes, which fails with STATUS_OBJECT_NAME_COLLISION, leading to confusing "The destination already has a file named X" error messages.

This is actually the exact same bug as https://bugzilla.samba.org/show_bug.cgi?id=11589 from 8 years ago. That bug was marked as fixed, but actually the resolution is incomplete - the fix makes Samba behave correctly if `ea support` is off, but Samba still misbehaves if `ea support` is on but the underlying filesystem does not support user xattrs.

The behavior is clearly wrong - Samba should either create the file, or return an error, but it definitely shouldn't do both. Ideally, Samba should check if the underlying filesystem supports user xattrs before creating the file.

One workaround is to set `ea support` to off, in which case Samba behaves correctly. But of course, one then loses extended attribute support on filesystems that support it.
Comment 1 Jeremy Allison 2024-01-31 01:36:24 UTC
This is due to the fact that creation and setting EA's are two separate operations in POSIX, instead of atomic like Windows, so setting the EA fails once the file is created. We can't delete the file on set_ea() error as there is a race condition here:


SMB client                smbd server                         Local process

CreateFile(name +ea) 
                    -> openat(dir, name, O_CREAT)
                       SUCCESS - new file 'name' created.
                       fd points to new file.

                                                              rename(name, name1)
                                                              openat(name, O_CREAT)
                                                              SUCCESS - file 'name' created.

                       fsetxattr(fd, ea)
                       ERROR - file system doesn't support EA's

                       unlinkat(dir, name) <==== WRONG FILE DELETED !
Comment 2 Jeremy Allison 2024-01-31 17:27:42 UTC
If there were an 'unlinkat()' variant that deleted the file pointed to by the file descriptor that would be safe, but without it there is always a race condition.
Comment 3 Etienne Dechamps 2024-01-31 18:44:21 UTC
I agree that this is tricky. In fact, even if there was a way to reliably delete the file afterwards, there would still be edge cases like inotify listeners and the like being incorrectly notified of the appearance of a new file. (As it turns out, I do have inotify listeners on the particular folder I encountered this issue on, and these listeners did get confused and broke because of the empty file.)

I think the ideal solution would be for Samba to somehow know, *before* creating the file, that it won't be possible to set EAs on it afterwards, so that it won't even try. But I don't know of any way to do that off the top of my head, short of something ugly/hacky like "if the filesystem is called 'tmpfs' then assume it will fail".
Comment 4 Ralph Böhme 2024-01-31 20:10:07 UTC
(In reply to Etienne Dechamps from comment #3)
*You* know if beforehand and configure the shares appropriately. :) "ea support" is a per-share option...
Comment 5 Etienne Dechamps 2024-01-31 20:42:24 UTC
(In reply to Ralph Böhme from comment #4)
Well, sure, but there are a couple of problems with this.

First, it requires manual configuration, which is tedious and error-prone. Ideally, Samba should just figure this out automatically and Do The Right Thing.

Second, shares don't map 1:1 to filesystems. In my case, I have a number of mountpoints under the share path - some of these mountpoints support user xattrs, some don't. This means that there is no single valid value for `ea support` for this share - it depends on where exactly the file is being created.
Comment 6 Etienne Dechamps 2024-02-06 12:52:49 UTC
Ok, here's an idea: at first glance it looks like open(O_TMPFILE) could be used to cleanly solve this condundrum. O_TMPFILE makes it possible to create a file without giving it a name, and only give it a name later.

1. Initially, create the file with O_TMPFILE in the target directory.
2. Call fsetxattr() on the resulting fd. If that fails, close the file and return an error.
3. At the very end, call linkat() to give the file a name and make it actually appear in the target directory.

This would make it possible for Samba to detect that xattrs can't be set *before* the file actually appears in the target directory. In other words this makes the setting of xattrs atomic with file creation for all intents and purposes, matching the semantics of the corresponding SMB command.

(Not all filesystems support O_TMPFILE, though tmpfs does. If the filesystem does not support O_TMPFILE, we can fall back to the previous behavior so it won't make things any worse than they already are.)
Comment 7 Jeremy Allison 2024-02-06 18:38:04 UTC
It looks like this would work, but it's completely Linux-specific.

In order to achieve this we'd need VFS changes and a re-working of the CreateWithEA code path.

What I'm saying is don't hold your breath for a quick fix for this.