Bug 10247 - vfs_streams_xattr fails to list streams on FreeBSD server
Summary: vfs_streams_xattr fails to list streams on FreeBSD server
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: 4.1.0
Hardware: x64 FreeBSD
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-04 11:00 UTC by Stefan Rompf
Modified: 2013-11-19 12:44 UTC (History)
3 users (show)

See Also:
jra: review+


Attachments
Possible bug fix (722 bytes, patch)
2013-11-04 11:00 UTC, Stefan Rompf
no flags Details
git-am fix for 3.6.next (1.09 KB, patch)
2013-11-12 20:19 UTC, Jeremy Allison
bjacke: review+
Details
Expose geteuid function call (339 bytes, patch)
2013-11-19 12:43 UTC, Timur Bakeyev
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Rompf 2013-11-04 11:00:57 UTC
Created attachment 9364 [details]
Possible bug fix

When creating a file on a share with streams support via vfs_streams_xattr, files stored by an OSX client do not display their properties and resource fork in finder.

Debugging shows that the problem is related to vfs_streams_xattr because vfs_streams_depot works.

Steps to show behaviour on windows:

Use a share like this: 
[test]
        comment = test
        browseable = yes
        writable = yes
        case sensitive = yes
        path = /data/test
        nt acl support = no
        vfs objects = zfsacl streams_xattr
        store dos attributes = yes
        ea support = yes

Using the windows client, create a file test.txt, then
* Storing an alternate stream (echo >test.txt:stream1) works
* Retrieving the alternate stream (type test.txt:stream1) works
* Listing the alternate streams of test.txt with a tool like stream explorer fails. This is also the reason why the OSX client fails, it seems to list the streams before it tries to access them.

Debugging:
The problem seems to be the function bsd_attr_list() in lib/replace/xattr.c. It calls extattr_get_file() for both system and user extattr namespace. The first call for the system namespace fails with EPERM if the user is not root, making bsd_attr_list() fail.

Attached patch to ignore EPERM on system namespace seems to fix the problem for me.
Comment 1 Björn Jacke 2013-11-05 11:08:41 UTC
looks reasonable, but i thought of the following approach also:

when iterating through the extattr namespaces in bsd_attr_list(), skip the EXTATTR_NAMESPACE_SYSTEM namespace when we are not running as root. This would also reduce some overhead.
Comment 2 Björn Jacke 2013-11-08 22:44:42 UTC
this is fixed with 374b2cfde74e0c61f4b2da724b30d0e430596092 in master, which can (and should) be cherry-picked to 4.0 and 4.1.
Comment 3 Jeremy Allison 2013-11-09 04:04:17 UTC
Patch in master - looks good to me. Should apply cleanly to 4.1.next, 4.0.next, 3.6.next.

Jeremy.
Comment 4 Stefan Rompf 2013-11-10 11:51:48 UTC
Works for me, thanks!
Comment 5 Karolin Seeger 2013-11-12 10:41:35 UTC
Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
Does not apply to current v3-6-test.
Is it very important for 3.6?
Comment 6 Björn Jacke 2013-11-12 11:06:25 UTC
for people with *BSD servers and OS X clients it's a nasty bug but it is not a security problem, so 3.6, which is security-fixes-only-mode does not need this.
Comment 7 Karolin Seeger 2013-11-12 11:16:00 UTC
(In reply to comment #6)
> for people with *BSD servers and OS X clients it's a nasty bug but it is not a
> security problem, so 3.6, which is security-fixes-only-mode does not need this.

There will be one last maintenance release of Samba 3.6.
So if it's worth it and someone provides a patch...
Comment 8 Jeremy Allison 2013-11-12 20:19:13 UTC
Created attachment 9410 [details]
git-am fix for 3.6.next

Back-port for 3.6.next.

Jeremy.
Comment 9 Karolin Seeger 2013-11-13 07:57:08 UTC
(In reply to comment #8)
> Created attachment 9410 [details]
> git-am fix for 3.6.next
> 
> Back-port for 3.6.next.
> 
> Jeremy.

Thanks a lot, Jeremy!
Pushed to v3-6-test.
Comment 10 Timur Bakeyev 2013-11-19 12:38:54 UTC
This fix breaks build in case --enable-uid-wrapper or --enable-nss-wrapper is enabled.

The fix is trivial though, see the attachment.
Comment 11 Timur Bakeyev 2013-11-19 12:43:29 UTC
Created attachment 9438 [details]
Expose geteuid function call