Fedora Account System
Red Hat Associate
Red Hat Customer
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.
Created attachment 458391 [details] Handle NULs in "text" encoded output
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?
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]
(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.
(In reply to comment #2) > 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? 0) I haven't looked at the patch yet, because I'm afraid that I need to think a bit about strings with embedded NULs. Those feel wrong to me. 1) A first reaction: if people want NULs to survive the tricks you use - which they seem not to do, otherwise they would have run into this issue before - they should just use the base64 encoding or the hex encoding. 2) But maybe it is specified somewhere what should be done here, making this discussion moot. Or perhaps there are competing implementations that can give some guidance here.
(In reply to comment #5) > 0) I haven't looked at the patch yet, because I'm afraid that I need to think a > bit about strings with embedded NULs. Those feel wrong to me. s/strings/attributes/ > 1) A first reaction: if people want NULs to survive the tricks you use - which > they seem not to do, otherwise they would have run into this issue before - > they should just use the base64 encoding or the hex encoding. Dump/restore should either honestly fail, or work properly. If it silently looses information with zero exit code, then it needs to be fixed. I see the current behavior, even with your original patch applied, as a bug. > 2) But maybe it is specified somewhere what should be done here, making this > discussion moot. Or perhaps there are competing implementations that can give > some guidance here. Sure, I'll forward the issue upstream. Thanks for your comments.
Created attachment 459025 [details] [PATCH v2] proposed improvement of setfattr.1 man page just fixed two typos...
Created attachment 459444 [details] Trivial typo in setfattr.1 Just something I noticed when looking at this.
(In reply to comment #8) > Just something I noticed when looking at this. By the way, similar patch could be applied to getfattr.1
Comment on attachment 459444 [details] Trivial typo in setfattr.1 >-.SS OPTIONS >+.SH OPTIONS Thanks for review. I don't think it was a typo. ".SS" is a valid macro according to man(7) man page. .SS t Subheading t (like .SH, but used for a subsection inside a section). Its usage is fairly consistent across all attr/acl man pages.
proposed upstream: http://lists.gnu.org/archive/html/acl-devel/2010-11/msg00001.html
pushed upstream: http://git.savannah.gnu.org/cgit/attr.git/commit/?id=7fed444 Btw. I must admit you were right with the .SS/.SH usage -- the getfattr/setfattr man pages were indeed changed in the way you had suggested: http://git.savannah.gnu.org/cgit/attr.git/commit/?id=0a2d62b
fixed in attr-2.4.44-6.fc15
attr-2.4.44-6.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/attr-2.4.44-6.fc14
attr-2.4.44-4.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/attr-2.4.44-4.fc13
attr-2.4.44-4.fc13 has been pushed to the Fedora 13 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update attr'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/attr-2.4.44-4.fc13
attr-2.4.44-6.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report.
attr-2.4.44-4.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.