Bug 651119

+++ This bug was initially created as a clone of Bug #650539 +++

Description of problem:
There can be NULs in the output of getfattr when using "text" encoding. getfattr should check whether it tries to print a string with an embedded NUL, ie, tries to print a truncated string.

Version-Release number of selected component (if applicable):
attr-2.4.44-5.fc15

How reproducible:
Always

Steps to Reproduce:
1. getfattr -e text -d -m - /usr/bin/getfattr
2.
3.
  
Actual results:
getfattr: Removing leading '/' from absolute path names
# file: usr/bin/getfattr
security.selinux="system_u:object_r:bin_t:s0

Expected results:
getfattr: Removing leading '/' from absolute path names
# file: usr/bin/getfattr
security.selinux="system_u:object_r:bin_t:s0"

Additional info:
The only, entirely trivial, difference is that currently, getfattr doesn't print the closing double quote in the example above.

Apparently, all SELinux extended attributes have a NUL append. See the 0x00 at the end of this output:
getfattr -e hex -d -m - /usr/bin/getfattr
getfattr: Removing leading '/' from absolute path names
# file: usr/bin/getfattr
security.selinux=0x73797374656d5f753a6f626a6563745f723a62696e5f743a733000

I'll try to attach a trivial patch to fix this shortly.

--- Additional comment from pebolle on 2010-11-06 23:45:24 CET ---

Created attachment 458391 [details]
Handle NULs in "text" encoded output

--- Additional comment from kdudka on 2010-11-08 19:56:35 CET ---

Created attachment 458849 [details]
extended version of the patch that handles embedded NULLs, too

Thank you for the bug report and the patch!  It's indeed broken and your patch solves the problem.  It stops on the first occurrence of '\0'.  For some reason 'setfattr --restore' then appends the trailing zero automatically while operating in 'text' mode, and everything works fine in the case of SELinux attributes.

However, the encoding is still broken when embedded NULLs are involved:

$ touch foo
$ setfattr -n user.bar -v 'foo\000bar' foo
$ getfattr --dump -e hex foo
# file: foo
user.bar=0x666f6f00626172

$ getfattr --dump -e text foo | tee dump
# file: foo
user.bar="foo"

$ setfattr --restore dump
$ ./getfattr/.libs/getfattr --dump -e hex foo
# file: foo
user.bar=0x666f6f

It means we are still loosing information at this point ^^^.  We should probably extend your patch this way:

$ setfattr -n user.bar -v 'foo\000bar' foo
$ getfattr --dump -e hex foo
# file: foo
user.bar=0x666f6f00626172

$ getfattr --dump -e text foo | tee dump
# file: foo
user.bar="foo\000bar"

$ setfattr --restore dump
$ ./getfattr/.libs/getfattr --dump -e hex foo
# file: foo
user.bar=0x666f6f00626172

Could you please have a look at the updated patch?

--- Additional comment from kdudka on 2010-11-08 20:29:18 CET ---

Actually, it reminded me an outstanding documentation bug (bug 587516), which is closely related to this one.  Paul, could you please have a look at the following man page fix and check if the wording is clear to you?  Thanks in advance!

attachment #458858 [details]

--- Additional comment from pebolle on 2010-11-08 21:45:53 CET ---

(In reply to comment #3)
> Paul, could you please have a look at the
> following man page fix and check if the wording is clear to you?

One of these days, I hope. Please prod me if you get nervous.