Bug 11606 - vfs_stream_xattr incorrect assumption of null byte in attribute value
vfs_stream_xattr incorrect assumption of null byte in attribute value
Status: NEW
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules
4.3.1
All All
: P5 normal
: ---
Assigned To: Samba QA Contact
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-16 14:44 UTC by Dima Tisnek
Modified: 2017-06-19 14:48 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dima Tisnek 2015-11-16 14:44:19 UTC
At least these two places in ADS over xattr vfs make assume that extended attribute value must be null-terminated:

"source3/modules/vfs_streams_xattr.c" line 491
        /*
         * Darn, xattrs need at least 1 byte
         */
        char null = '\0';


"source3/modules/vfs_streams_xattr.c" line 1024
    length = ea.value.length-1;

    DEBUG(10, ("streams_xattr_pread: get_ea_value returned %d bytes\n",
           (int)length));


However linux extended attribute man pages are clear that zero-length values are in fact allowed:

       setxattr() sets the value of the extended attribute identified by
       name and associated with the given path in the filesystem.  The size
       argument specifies the size (in bytes) of value; a zero-length value
       is permitted.


My use case is that I want to set extended attributes natively on the samba server and have those readable as ADS for windows clients.

Currently if I set 8-byte value, clients see 7-byte value (sans last byte) and I'll probably have to work around samba's vfs module and include a dummy byte at the end.
Comment 1 Jeremy Allison 2015-11-16 18:26:54 UTC
Linux EA's are not the only EA's Samba uses.

We also run on all the *BSD's and also Solaris (and older HPUX and AIX).

It's very likely this restriction might come from another platform than Linux (shock, horror :-). They do exist :-).
 
Adding an extra null byte on Linux isn't the end of the world, and I'm having a hard time thinking of this as a Samba bug.
Comment 2 Michael Adler 2017-06-19 14:48:33 UTC
This affects xattr reads and is a real problem there. The Samba read path forces the final byte to be NULL even if it isn't! It doesn't extend the response with an extra NULL pad. It just overwrites the final byte with NULL.

Produce the following using macOS Sierra as the client and Samba 4.6 on FreeBSD/ZFS server:

On the FreeBSD server on a ZFS backed share:

  > touch asdf
  > setextattr user foo bar asdf
  > getextattr -x user foo asdf
  asdf    62 61 72

the "foo" attribute is 3 bytes, not NULL padded.

On the macOS client:

  > xattr -p foo asdf
  ba
  > xattr -x -p foo asdf
  62 61

FreeBSD does have an "-n" option to setextattr, which NULL pads and produces the expected result on the client. This is not the default though and it seems reasonable for Samba to handle both cases.

You could make a case for not extending attributes when writing from the client, too, especially when the attribute is binary and not a string.

So, the assumption of NULL padding on read is a bug and, perhaps NULL padding should be an option on write instead of forced.