On 2022/9/1 05:29, Rob Landley wrote: > On 8/31/22 12:00, Weizhao Ouyang wrote: >> On 2022/8/31 18:48, Rob Landley wrote: >>> On 8/31/22 05:35, Rob Landley wrote: >>>> On 8/30/22 10:31, Weizhao Ouyang wrote: >>>>> When getfattx reading overlayfs merged dirs, listxattr will got >>>>> different keys_len with zero size and determined size, then it will >>>>> stuck in this while scope. Update the keys_len after the second >>>>> listxattr to fix it. >>>> Do you have a test case I can use to reproduce this? (I've never used >>>> xattrs >>>> together with overlayfs before.) >> Hi Rob, here is a simple testcase (with overlayfs module): >> mkdir lower upper merged work >> mkdir lower/dir upper/dir >> sudo mount -t overlay overlay >> -olowerdir=lower/,upperdir=upper/,workdir=work/,userxattr merged/ >> toybox getfattr -d merged/dir >> >> Then it will stuck on listxattr. > I don't know what distro you're using, or what host filesystem types you're > mounting together, and my devuan system hasn't got selinux installed: > > root@driftwood:/home/landley# getenforce > bash: getenforce: command not found > > So when I create random files/directories on my laptop it doesn't attach > xattrs > to them by default, and thus I expect returning 0 is probably going to happen > for me even on the overlay? (Or maybe -1. I'm not 100% certain my kernel or > ext4 > filesystem driver are compiled with xattr support, although I _think_ they > are? > I tend to test this sort thing in mkroot where I can control all the > variables, > but that doesn't help me reproduce the issue the first time.) > > I'm just going to assume "Fedora" since that's the one with all the brittle > infrastructure full of overcomplicated sharp edges. Things like selinux and > systemd and glibc symbol versioning tend to be Red Hat's fault...
SELinux is not required for testing, the testcase will trigger an overlayfs feature and attach upper/dir an xttr "*.overlay.origin". I found this bug first on an android device: Android 13 + 5.15 kernel + F2FS Then I tested it on a Ubuntu machine with Ext4 filesystem. >>>> Also, I believe Elliott has a different implementation of getfattr in >>>> android >>>> (for some sort of C++ library reasons, I'd have to check my notes), so >>>> he'll >>>> probably want the test case there too? >>> Would this approach work for you? >> I compiled and tested your patch and it doesn't work, seems have some issues. > I noticed this bug in the patch I sent you: > > $ ./getfattr doesnotexist > getfattr: xrealloc > > Because I'm testing keys_len == -1 not len2. > > But I don't know what else is wrong with it yet, because I haven't got selinux > here so there aren't any xattrs on my files. I'll try to reproduce it under a > Fedora VM. I add the len2 condition, and test case got an ERANGE. >> As for malloc length problem, overlayfs filter out private attrs to hide it, >> which means length decreasing is a possible situation but increasing not, so > Good to know. Thanks. (I wonder what a private attr is? And why overlayfs > DOESN'T filter them out when given a zero length, but does filter them out > when > given a buffer? Producing inconsistent results still sounds like an overlayfs > bug in the kernel...) OverlayFS has some particular xattrs to achieve its feature, so it's private for overlayfs, it shows on upper files not merge/lower files. If overlayfs get a zero buffer_size, it will return after vfs_listxattr and not filter out private xattrs. If we want to change this behavior in the kernel, maybe it will be tricky because of more memory allocation. >> treat the increasing as an error might be a choice, what do you think of the >> patch below: > Your patch looks reasonable, but: > > $ diffstat mine > getfattr.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > $ diffstat yours > getfattr.c | 22 +++++++++++++++------- > 1 file changed, 15 insertions(+), 7 deletions(-) > > The resulting code with your change is 11 lines longer than the resulting code > with my change, plus the loop _does_ adjust for the file changing out from > under > us for other reasons, so I'd like to understand why the loop is failing. > > It sounds like overlayfs is providing different result lengths given different > buffer lengths, and oscillates instead of stabilizing. Which does not fill me > with confidence about the quality of this infratructure. > > Let me study your patch and try this on Fedora. (But first I'm downloading the > Fedora 36 livecd to replace the 34 livecd I've been using, and it says it'll > take 2 hours.) > > Rob I check `man listxattr` examples and found it's similar to my approach: first call to get current size and second call to update buffer_size. IMO, we don't need a loop because twice listxattr is enough to fill the buffer, what's your opinion? Thanks, Weizhao _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
