On Wed, Aug 31, 2022 at 3:27 AM Rob Landley <r...@landley.net> 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.) > > 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? > no, i sent you this implementation :-) i think you're thinking of modprobe, which is parallelized and used as a library inside init directly? > > Signed-off-by: Weizhao Ouyang <o451686...@gmail.com> > > --- > > toys/pending/getfattr.c | 19 +++++++++++++------ > > 1 file changed, 13 insertions(+), 6 deletions(-) > > > > diff --git a/toys/pending/getfattr.c b/toys/pending/getfattr.c > > index bf2c04c8..aa2c3958 100644 > > --- a/toys/pending/getfattr.c > > +++ b/toys/pending/getfattr.c > > @@ -43,15 +43,22 @@ static void do_getfattr(char *file) > > } > > > > // Collect the keys. > > - while ((keys_len = lister(file, NULL, 0))) { > > - if (keys_len == -1) perror_msg("listxattr failed"); > > - keys = xmalloc(keys_len); > > - if (lister(file, keys, keys_len) == keys_len) break; > > + keys_len = lister(file, NULL, 0); > > + if (keys_len < 0) > > + perror_exit("listxattr failed"); > > + else if (keys_len == 0) > > + return; > > + > > + keys = xmalloc(keys_len); > > + keys_len = lister(file, keys, keys_len); > > + if (keys_len < 0) { > > free(keys); > > + perror_exit("listxattr failed"); > > + } else if (keys_len == 0) { > > + free(keys); > > + return; > > } > > > > - if (keys_len == 0) return; > > - > > // Sort the keys. > > for (key = keys, key_count = 0; key-keys < keys_len; key += > strlen(key)+1) > > key_count++; > > Ok, in THEORY the reason for the original loop is that the size could > change out > from under us if the filesystem is modified while we're reading it. You're > saying it loops endlessly, because "listxattr will got different keys_len > with > zero size and determined size". > > Your new unrolled version just _sets_ keys_len to whatever the second one > returns. So on overlayfs, listxattr(file, NULL, 0) and listattr(file, > keys, len) > return different values for the same file. > > This sounds like a bug in overlayfs you're working around, but if we have > to do > that to work with deployed reality fine. The problem is, if the new > keys_len is > LONGER than the previous one, then we didn't malloc() enough space for it, > and > "sort the keys" below will fall off the end of the buffer. > +David Anderson <dvan...@google.com> --- are you aware of this behavior? (sounds like a bug to me too, but you know more about fs stuff than i do...) > I'd really like a test case so I can see this in action... > > Thanks, > > Rob > _______________________________________________ > Toybox mailing list > Toybox@lists.landley.net > http://lists.landley.net/listinfo.cgi/toybox-landley.net >
_______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net