iustin / pyxattr

A python module for accessing filesystem Extended Attributes
https://pyxattr.k1024.org/
GNU Lesser General Public License v2.1
31 stars 14 forks source link

xattr_get did not allocate a large enough buffer #6

Closed m0zes closed 9 years ago

m0zes commented 9 years ago

the getxattr library call requires a size option for the length of the buffer. We only allocated enough memory to handle the exact length of the value of the extended attribute. You must allocate an extra byte for the null terminator on the end of the string.

$ getfattr -n ceph.dir.rbytes /var/cephfs/test

ceph.dir.rbytes="222142325238"

current head:

xattr.get('/var/cephfs/test', 'ceph.dir.rbytes') b'22214232523\x00'

with my fix:

xattr.get('/var/cephfs/test', 'ceph.dir.rbytes') b'222142325238'

iustin commented 9 years ago

Actually, I believe this is a bug in the behaviour of the ceph filesystem; the man page says clearly (emphasis mine):

An extended attribute name is a simple NULL-terminated string. The name includes a namespace prefix - there may be several, disjoint namespaces associated with an indi‐ vidual inode. The value of an extended attribute is a chunk of arbitrary textual or binary data of specified length.

Note that the name is a NULL-terminated string, but the buffer is binary data of the specified length. The fact that ceph null-terminates the string but returns the not-null-terminated size is a bug in the ceph client. Looking at their client code, if I read correctly, they indeed check that size < xattr->val_len, instead of <=.

Unfortunately, I don't have a ceph installation to verify this (and it seems non-trivial to setup a cluster), but since you seem that you do - I'd recommend comparing strace output for a ext4/xfs filesystem and ceph, and if indeed they null-terminate wrongly, file a bug with upstream ceph. I'm reluctant to merge this since it seems to be a workaround for a broken filesystem, rather than an issue with pyxattr itself.

For the record:

    $ setfattr -n user.foo -v 'bar\0a' file2
    $ getfattr -e text -n user.foo file2
    # file: file2
    user.foo="bar\000a"

So yes, regular filesystems treat the value as binary data, not C strings.