[Toybox] [PATCH] getfattr.c: fix overlayfs merged dir stuck

[Weizhao Ouyang]

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.

>> 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.

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
treat the increasing as an error might be a choice, what do you think of the
patch below:

diff --git a/toys/pending/getfattr.c b/toys/pending/getfattr.c
index 255e194b..ebeafb6b 100644
--- a/toys/pending/getfattr.c
+++ b/toys/pending/getfattr.c
@@ -33,7 +33,7 @@ static void do_getfattr(char *file)
   ssize_t (*getter)(const char *, const char *, void *, size_t) = getxattr;
   ssize_t (*lister)(const char *, char *, size_t) = listxattr;
   char **sorted_keys;
-  ssize_t keys_len;
+  ssize_t keys_len, len2;
   char *keys, *key;
   int i, key_count;
 
@@ -43,14 +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);
+  len2 = lister(file, keys, keys_len);
+  if (len2 < 0 || len2 > keys_len) {
     free(keys);
+    perror_exit("listxattr failed");
+  } else if (len2 == 0) {
+    free(keys);
+    return;
   }
-
-  if (keys_len == 0) return;
+  keys_len = len2;
 
   // Sort the keys.

   for (key = keys, key_count = 0; key-keys < keys_len; key += strlen(key)+1)