Created attachment 3494 [details] test case for unpacking problem Environment: ostro-os master with pseudo 1.7.5 and bsdtar 3.2.1. Probably can be reproduced using OE-core master. Steps: * build pseudo-native and libarchive-native * unpack the attached lib.tar.xz in a test directory as follows: rm -rf pseudo-state rootfs; mkdir rootfs; PSEUDO_PREFIX=<my path>/tmp-glibc/sysroots/x86_64-linux/usr PSEUDO_NOSYMLINKEXP=1 PSEUDO_DISABLED=0 PSEUDO_LOCALSTATEDIR=`pwd`/pseudo-state <my path>/tmp-glibc/sysroots/x86_64-linux/usr/bin/pseudo /bin/sh -c '<my path>/tmp-glibc/sysroots/x86_64-linux/usr/bin/bsdtar -xf /tmp/lib.tar.xz -C rootfs; getfattr -d -m . rootfs/lib/*' 2>&1 | grep -B1 'SMACK64="...*"' Expected result: * all entries in lib should have security.SMACK64="_" and thus nothing should match that grep expression Actual result: # file: rootfs/lib/depmod.d security.SMACK64="__" -- # file: rootfs/lib/modules security.SMACK64="_S" -- # file: rootfs/lib/security security.SMACK64="_K" -- # file: rootfs/lib/udev security.SMACK64="_K" Only directories are affected, but not all of them, and this also does not occur when extracting just the affected directory. Looks like some kind of random corruption problem. It works when running bsdtar as root without pseudo. valgrind reports nothing about bsdtar when running as root.
Created attachment 3495 [details] Simpler test case with just two directories Simpler test case with just two directories: lib/udev lib/depmod.d -> # file: rootfs/lib/depmod.d security.SMACK64="_1"
I'm curious about how the tarball was made. It shouldn't matter, I assume, but I'd like to know what's going on with the security.SMACK64 thing.
The initial lib.tar.xz was created from a real rootfs by running bsdtar inside "bitbake -c devshell". xattrs in that rootfs were correct, and so is the resulting archive (can be verified by unpacking as root). The simplified lib.tar was created by unpacking as root into rootfs2, then creating an archive with bsdtar outside of pseudo with "bsdtar -cnvf /tmp/lib.tar -C rootfs2 lib/udev lib/depmod.d"
Regarding security.SMACK64: that is getting set by some helper class in Ostro when building the rootfs.
Okay. I'll try to have a look at this soonish. This certainly *should* work, at least so far as I understand it; pseudo isn't doing anything special unique to the SMACK64 attribute.
This bothers me, because the code's clearly been wrong for a while, but apparently we've been getting lucky (or unlucky) and it hasn't bitten us. I suspect that, for a while, we've been mostly-harmlessly stashing extra null bytes on the ends of things, and some other change caused them to sometimes not be null. The actual problem is alluded to by one of the comments: I observe that I don't want as many bytes when I'm reading extended attributes as I do with file names, but then I didn't actually update the computed length. Fixing that appears to resolve this. So the update that fixed copying and such appears to have caused the data sent to the server to include a trailing null byte, but the database code assumed that no such byte was included in the character count reported to it. So, "security.SMACK64\0_\0" was reported as 19 bytes (which it technically is), and the database code assumed that the _\0 was the value. Now the database code is told it's got 18 bytes. (Since it's using the length, not the null terminator, because we need to allow binary data.) Fixed upstream in d5a6b5e2.
I've tried the fix on top of 1.8.1 (applies cleanly). Unfortunately it now breaks in a different way: # mkdir foo # setfattr -n security.SMACK64 -v "_" foo # getfattr -d -m . foo foo: security.SMACK64: No such attribute
Also happens when using pseudo git master.
Could it be that some code paths send the "_" value with length 2 and some send it only with length 1? Just a guess...
The difference between bsdtar and my manual setfattr is that bsdtar has a trailing slash on the path name. That leads to a slight difference in pseudo_client_op() such that it does not compute the length of the full buffer correctly. Here's the smoking gun with some extra debug output: rm -rf pseudo-state rootfs; mkdir rootfs; PSEUDO_PREFIX=tmp-glibc/sysroots/x86_64-linux/usr PSEUDO_NOSYMLINKEXP=1 PSEUDO_DISABLED=0 PSEUDO_DEBUG=fxiV PSEUDO_LOCALSTATEDIR=`pwd`/pseudo-state tmp-glibc/sysroots/x86_64-linux/usr/bin/pseudo -v /bin/sh -c 'mkdir rootfs/foo; setfattr -n security.SMACK64 -v "_" rootfs/foo; getfattr -d -m . rootfs/foo; bsdtar -C rootfs -xf /tmp/lib.tar; getfattr -d -m . rootfs/lib/*' ... path buffer len 74 = 55 + 1 + 16 + 2 (stripping slash) combined path buffer at 0x13d9a00 [74 bytes]: 0x000000 2f 66 61 73 74 2f 62 75 69 6c 64 2f 6f 73 74 72 '/fast/build/ostr' 0x000010 6f 2f 69 6e 74 65 6c 2d 63 6f 72 65 69 37 2d 36 'o/intel-corei7-6' 0x000020 34 2f 72 6f 6f 74 66 73 2f 6c 69 62 2f 64 65 70 '4/rootfs/lib/dep' 0x000030 6d 6f 64 2e 64 00 73 65 63 75 72 69 74 79 2e 53 'mod.d.security.S' 0x000040 4d 41 43 4b 36 34 00 5f 00 00 'MACK64._..' Note that there are two trailing nul bytes. Knowing that, a simpler way to test both cases is: rm -rf pseudo-state rootfs; mkdir rootfs; PSEUDO_PREFIX=tmp-glibc/sysroots/x86_64-linux/usr PSEUDO_NOSYMLINKEXP=1 PSEUDO_DISABLED=0 PSEUDO_DEBUG=fxiV PSEUDO_LOCALSTATEDIR=`pwd`/pseudo-state tmp-glibc/sysroots/x86_64-linux/usr/bin/pseudo -v /bin/sh -c 'mkdir rootfs/foo; setfattr -n security.SMACK64 -v "_" rootfs/foo; mkdir rootfs/bar; setfattr -n security.SMACK64 -v "_" rootfs/bar/; getfattr -d -m . rootfs/*' Seebs, to fix the problem one has to revert commit d5a6b5e2 and instead apply a patch that I am going to attach.
Created attachment 3521 [details] fix buffer length calculation
I have a pseudo_1.8.1.bb patch which adds this patch. If accepted upstream, I can also submit that to OE-core.
Comment on attachment 3521 [details] fix buffer length calculation Oh, very nice.
Okay, that does explain a few things, one of them being "why did this show up only on directories, not on files", and one being "why didn't I spot it before, and why didn't my analysis quite fit the history". Patch merged in upstream. Thanks!
(In reply to comment #12) > I have a pseudo_1.8.1.bb patch which adds this patch. If accepted upstream, > I can also submit that to OE-core. Patrick, will you be submitting a recipe update to OE-Core?
"pseudo: include fix for xattr corruption" is in OE-core master.