erofs: use separate xattr parsers for listxattr/getxattr
authorJingbo Xu <jefflexu@linux.alibaba.com>
Tue, 13 Jun 2023 07:41:14 +0000 (15:41 +0800)
committerGao Xiang <hsiangkao@linux.alibaba.com>
Sun, 18 Jun 2023 04:10:54 +0000 (12:10 +0800)
There's a callback styled xattr parser, i.e. xattr_foreach(), which is
shared among listxattr and getxattr.

Convert it to two separate xattr parsers to serve listxattr and getxattr
for better readability.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>
Link: https://lore.kernel.org/r/20230613074114.120115-6-jefflexu@linux.alibaba.com
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
fs/erofs/xattr.c

index abbf8e0bcdac06453a356bffc21cd0d4c444d892..40178b6e0688caf4d2fc5eca4e462eecebd295d9 100644 (file)
@@ -123,195 +123,6 @@ out_unlock:
        return ret;
 }
 
-/*
- * the general idea for these return values is
- * if    0 is returned, go on processing the current xattr;
- *       1 (> 0) is returned, skip this round to process the next xattr;
- *    -err (< 0) is returned, an error (maybe ENOXATTR) occurred
- *                            and need to be handled
- */
-struct xattr_iter_handlers {
-       int (*entry)(struct erofs_xattr_iter *it, struct erofs_xattr_entry *entry);
-       int (*name)(struct erofs_xattr_iter *it, unsigned int processed, char *buf,
-                   unsigned int len);
-       int (*alloc_buffer)(struct erofs_xattr_iter *it, unsigned int value_sz);
-       void (*value)(struct erofs_xattr_iter *it, unsigned int processed, char *buf,
-                     unsigned int len);
-};
-
-/*
- * Regardless of success or failure, `xattr_foreach' will end up with
- * `pos' pointing to the next xattr item rather than an arbitrary position.
- */
-static int xattr_foreach(struct erofs_xattr_iter *it,
-                        const struct xattr_iter_handlers *op,
-                        unsigned int *tlimit)
-{
-       struct erofs_xattr_entry entry;
-       struct super_block *sb = it->sb;
-       unsigned int value_sz, processed, slice;
-       int err;
-
-       /* 0. fixup blkaddr, pos */
-       it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos), EROFS_KMAP);
-       if (IS_ERR(it->kaddr))
-               return PTR_ERR(it->kaddr);
-
-       /*
-        * 1. read xattr entry to the memory,
-        *    since we do EROFS_XATTR_ALIGN
-        *    therefore entry should be in the page
-        */
-       entry = *(struct erofs_xattr_entry *)
-               (it->kaddr + erofs_blkoff(sb, it->pos));
-       if (tlimit) {
-               unsigned int entry_sz = erofs_xattr_entry_size(&entry);
-
-               /* xattr on-disk corruption: xattr entry beyond xattr_isize */
-               if (*tlimit < entry_sz) {
-                       DBG_BUGON(1);
-                       return -EFSCORRUPTED;
-               }
-               *tlimit -= entry_sz;
-       }
-
-       it->pos += sizeof(struct erofs_xattr_entry);
-       value_sz = le16_to_cpu(entry.e_value_size);
-
-       /* handle entry */
-       err = op->entry(it, &entry);
-       if (err) {
-               it->pos += entry.e_name_len + value_sz;
-               goto out;
-       }
-
-       /* 2. handle xattr name (pos will finally be at the end of name) */
-       processed = 0;
-
-       while (processed < entry.e_name_len) {
-               it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos),
-                                       EROFS_KMAP);
-               if (IS_ERR(it->kaddr)) {
-                       err = PTR_ERR(it->kaddr);
-                       goto out;
-               }
-
-               slice = min_t(unsigned int,
-                             sb->s_blocksize - erofs_blkoff(sb, it->pos),
-                             entry.e_name_len - processed);
-
-               /* handle name */
-               err = op->name(it, processed,
-                              it->kaddr + erofs_blkoff(sb, it->pos), slice);
-               if (err) {
-                       it->pos += entry.e_name_len - processed + value_sz;
-                       goto out;
-               }
-
-               it->pos += slice;
-               processed += slice;
-       }
-
-       /* 3. handle xattr value */
-       processed = 0;
-
-       if (op->alloc_buffer) {
-               err = op->alloc_buffer(it, value_sz);
-               if (err) {
-                       it->pos += value_sz;
-                       goto out;
-               }
-       }
-
-       while (processed < value_sz) {
-               it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos),
-                                       EROFS_KMAP);
-               if (IS_ERR(it->kaddr)) {
-                       err = PTR_ERR(it->kaddr);
-                       goto out;
-               }
-
-               slice = min_t(unsigned int,
-                             sb->s_blocksize - erofs_blkoff(sb, it->pos),
-                             value_sz - processed);
-               op->value(it, processed, it->kaddr + erofs_blkoff(sb, it->pos),
-                         slice);
-               it->pos += slice;
-               processed += slice;
-       }
-
-out:
-       /* xattrs should be 4-byte aligned (on-disk constraint) */
-       it->pos = EROFS_XATTR_ALIGN(it->pos);
-       return err < 0 ? err : 0;
-}
-
-static int erofs_xattr_long_entrymatch(struct erofs_xattr_iter *it,
-                                      struct erofs_xattr_entry *entry)
-{
-       struct erofs_sb_info *sbi = EROFS_SB(it->sb);
-       struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes +
-               (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK);
-
-       if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
-               return -ENOATTR;
-
-       if (it->index != pf->prefix->base_index ||
-           it->name.len != entry->e_name_len + pf->infix_len)
-               return -ENOATTR;
-
-       if (memcmp(it->name.name, pf->prefix->infix, pf->infix_len))
-               return -ENOATTR;
-
-       it->infix_len = pf->infix_len;
-       return 0;
-}
-
-static int xattr_entrymatch(struct erofs_xattr_iter *it,
-                           struct erofs_xattr_entry *entry)
-{
-       /* should also match the infix for long name prefixes */
-       if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX)
-               return erofs_xattr_long_entrymatch(it, entry);
-
-       if (it->index != entry->e_name_index ||
-           it->name.len != entry->e_name_len)
-               return -ENOATTR;
-       it->infix_len = 0;
-       return 0;
-}
-
-static int xattr_namematch(struct erofs_xattr_iter *it,
-                          unsigned int processed, char *buf, unsigned int len)
-{
-       if (memcmp(buf, it->name.name + it->infix_len + processed, len))
-               return -ENOATTR;
-       return 0;
-}
-
-static int xattr_checkbuffer(struct erofs_xattr_iter *it,
-                            unsigned int value_sz)
-{
-       int err = it->buffer_size < value_sz ? -ERANGE : 0;
-
-       it->buffer_ofs = value_sz;
-       return !it->buffer ? 1 : err;
-}
-
-static void xattr_copyvalue(struct erofs_xattr_iter *it,
-                           unsigned int processed,
-                           char *buf, unsigned int len)
-{
-       memcpy(it->buffer + processed, buf, len);
-}
-
-static const struct xattr_iter_handlers find_xattr_handlers = {
-       .entry = xattr_entrymatch,
-       .name = xattr_namematch,
-       .alloc_buffer = xattr_checkbuffer,
-       .value = xattr_copyvalue
-};
-
 static bool erofs_xattr_user_list(struct dentry *dentry)
 {
        return test_opt(&EROFS_SB(dentry->d_sb)->opt, XATTR_USER);
@@ -364,20 +175,49 @@ const struct xattr_handler *erofs_xattr_handlers[] = {
        NULL,
 };
 
-static int xattr_entrylist(struct erofs_xattr_iter *it,
-                          struct erofs_xattr_entry *entry)
+static int erofs_xattr_copy_to_buffer(struct erofs_xattr_iter *it,
+                                     unsigned int len)
+{
+       unsigned int slice, processed;
+       struct super_block *sb = it->sb;
+       void *src;
+
+       for (processed = 0; processed < len; processed += slice) {
+               it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos),
+                                       EROFS_KMAP);
+               if (IS_ERR(it->kaddr))
+                       return PTR_ERR(it->kaddr);
+
+               src = it->kaddr + erofs_blkoff(sb, it->pos);
+               slice = min_t(unsigned int, sb->s_blocksize -
+                               erofs_blkoff(sb, it->pos), len - processed);
+               memcpy(it->buffer + it->buffer_ofs, src, slice);
+               it->buffer_ofs += slice;
+               it->pos += slice;
+       }
+       return 0;
+}
+
+static int erofs_listxattr_foreach(struct erofs_xattr_iter *it)
 {
-       unsigned int base_index = entry->e_name_index;
-       unsigned int prefix_len, infix_len = 0;
+       struct erofs_xattr_entry entry;
+       unsigned int base_index, name_total, prefix_len, infix_len = 0;
        const char *prefix, *infix = NULL;
+       int err;
 
-       if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) {
+       /* 1. handle xattr entry */
+       entry = *(struct erofs_xattr_entry *)
+                       (it->kaddr + erofs_blkoff(it->sb, it->pos));
+       it->pos += sizeof(struct erofs_xattr_entry);
+
+       base_index = entry.e_name_index;
+       if (entry.e_name_index & EROFS_XATTR_LONG_PREFIX) {
                struct erofs_sb_info *sbi = EROFS_SB(it->sb);
                struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes +
-                       (entry->e_name_index & EROFS_XATTR_LONG_PREFIX_MASK);
+                       (entry.e_name_index & EROFS_XATTR_LONG_PREFIX_MASK);
 
                if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
-                       return 1;
+                       return 0;
                infix = pf->prefix->infix;
                infix_len = pf->infix_len;
                base_index = pf->prefix->base_index;
@@ -385,53 +225,102 @@ static int xattr_entrylist(struct erofs_xattr_iter *it,
 
        prefix = erofs_xattr_prefix(base_index, it->dentry);
        if (!prefix)
-               return 1;
+               return 0;
        prefix_len = strlen(prefix);
+       name_total = prefix_len + infix_len + entry.e_name_len + 1;
 
        if (!it->buffer) {
-               it->buffer_ofs += prefix_len + infix_len +
-                                       entry->e_name_len + 1;
-               return 1;
+               it->buffer_ofs += name_total;
+               return 0;
        }
 
-       if (it->buffer_ofs + prefix_len + infix_len +
-               + entry->e_name_len + 1 > it->buffer_size)
+       if (it->buffer_ofs + name_total > it->buffer_size)
                return -ERANGE;
 
        memcpy(it->buffer + it->buffer_ofs, prefix, prefix_len);
        memcpy(it->buffer + it->buffer_ofs + prefix_len, infix, infix_len);
        it->buffer_ofs += prefix_len + infix_len;
-       return 0;
-}
 
-static int xattr_namelist(struct erofs_xattr_iter *it,
-                         unsigned int processed, char *buf, unsigned int len)
-{
-       memcpy(it->buffer + it->buffer_ofs, buf, len);
-       it->buffer_ofs += len;
+       /* 2. handle xattr name */
+       err = erofs_xattr_copy_to_buffer(it, entry.e_name_len);
+       if (err)
+               return err;
+
+       it->buffer[it->buffer_ofs++] = '\0';
        return 0;
 }
 
-static int xattr_skipvalue(struct erofs_xattr_iter *it,
-                          unsigned int value_sz)
+static int erofs_getxattr_foreach(struct erofs_xattr_iter *it)
 {
-       it->buffer[it->buffer_ofs++] = '\0';
-       return 1;
-}
+       struct super_block *sb = it->sb;
+       struct erofs_xattr_entry entry;
+       unsigned int slice, processed, value_sz;
 
-static const struct xattr_iter_handlers list_xattr_handlers = {
-       .entry = xattr_entrylist,
-       .name = xattr_namelist,
-       .alloc_buffer = xattr_skipvalue,
-       .value = NULL
-};
+       /* 1. handle xattr entry */
+       entry = *(struct erofs_xattr_entry *)
+                       (it->kaddr + erofs_blkoff(sb, it->pos));
+       it->pos += sizeof(struct erofs_xattr_entry);
+       value_sz = le16_to_cpu(entry.e_value_size);
+
+       /* should also match the infix for long name prefixes */
+       if (entry.e_name_index & EROFS_XATTR_LONG_PREFIX) {
+               struct erofs_sb_info *sbi = EROFS_SB(sb);
+               struct erofs_xattr_prefix_item *pf = sbi->xattr_prefixes +
+                       (entry.e_name_index & EROFS_XATTR_LONG_PREFIX_MASK);
+
+               if (pf >= sbi->xattr_prefixes + sbi->xattr_prefix_count)
+                       return -ENOATTR;
+
+               if (it->index != pf->prefix->base_index ||
+                   it->name.len != entry.e_name_len + pf->infix_len)
+                       return -ENOATTR;
+
+               if (memcmp(it->name.name, pf->prefix->infix, pf->infix_len))
+                       return -ENOATTR;
+
+               it->infix_len = pf->infix_len;
+       } else {
+               if (it->index != entry.e_name_index ||
+                   it->name.len != entry.e_name_len)
+                       return -ENOATTR;
+
+               it->infix_len = 0;
+       }
+
+       /* 2. handle xattr name */
+       for (processed = 0; processed < entry.e_name_len; processed += slice) {
+               it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos),
+                                       EROFS_KMAP);
+               if (IS_ERR(it->kaddr))
+                       return PTR_ERR(it->kaddr);
+
+               slice = min_t(unsigned int,
+                               sb->s_blocksize - erofs_blkoff(sb, it->pos),
+                               entry.e_name_len - processed);
+               if (memcmp(it->name.name + it->infix_len + processed,
+                          it->kaddr + erofs_blkoff(sb, it->pos), slice))
+                       return -ENOATTR;
+               it->pos += slice;
+       }
+
+       /* 3. handle xattr value */
+       if (!it->buffer) {
+               it->buffer_ofs = value_sz;
+               return 0;
+       }
+
+       if (it->buffer_size < value_sz)
+               return -ERANGE;
+
+       return erofs_xattr_copy_to_buffer(it, value_sz);
+}
 
 static int erofs_xattr_iter_inline(struct erofs_xattr_iter *it,
                                   struct inode *inode, bool getxattr)
 {
        struct erofs_inode *const vi = EROFS_I(inode);
-       const struct xattr_iter_handlers *op;
-       unsigned int xattr_header_sz, remaining;
+       unsigned int xattr_header_sz, remaining, entry_sz;
+       erofs_off_t next_pos;
        int ret;
 
        xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) +
@@ -441,19 +330,33 @@ static int erofs_xattr_iter_inline(struct erofs_xattr_iter *it,
                return -ENOATTR;
        }
 
-       it->pos = erofs_iloc(inode) + vi->inode_isize + xattr_header_sz;
-       it->kaddr = erofs_bread(&it->buf, erofs_blknr(it->sb, it->pos),
-                               EROFS_KMAP);
-       if (IS_ERR(it->kaddr))
-               return PTR_ERR(it->kaddr);
-
        remaining = vi->xattr_isize - xattr_header_sz;
-       op = getxattr ? &find_xattr_handlers : &list_xattr_handlers;
+       it->pos = erofs_iloc(inode) + vi->inode_isize + xattr_header_sz;
 
        while (remaining) {
-               ret = xattr_foreach(it, op, &remaining);
+               it->kaddr = erofs_bread(&it->buf, erofs_blknr(it->sb, it->pos),
+                                       EROFS_KMAP);
+               if (IS_ERR(it->kaddr))
+                       return PTR_ERR(it->kaddr);
+
+               entry_sz = erofs_xattr_entry_size(it->kaddr +
+                               erofs_blkoff(it->sb, it->pos));
+               /* xattr on-disk corruption: xattr entry beyond xattr_isize */
+               if (remaining < entry_sz) {
+                       DBG_BUGON(1);
+                       return -EFSCORRUPTED;
+               }
+               remaining -= entry_sz;
+               next_pos = it->pos + entry_sz;
+
+               if (getxattr)
+                       ret = erofs_getxattr_foreach(it);
+               else
+                       ret = erofs_listxattr_foreach(it);
                if ((getxattr && ret != -ENOATTR) || (!getxattr && ret))
                        break;
+
+               it->pos = next_pos;
        }
        return ret;
 }
@@ -465,11 +368,8 @@ static int erofs_xattr_iter_shared(struct erofs_xattr_iter *it,
        struct super_block *const sb = it->sb;
        struct erofs_sb_info *sbi = EROFS_SB(sb);
        unsigned int i;
-       const struct xattr_iter_handlers *op;
        int ret = -ENOATTR;
 
-       op = getxattr ? &find_xattr_handlers : &list_xattr_handlers;
-
        for (i = 0; i < vi->xattr_shared_count; ++i) {
                it->pos = erofs_pos(sb, sbi->xattr_blkaddr) +
                                vi->xattr_shared_xattrs[i] * sizeof(__le32);
@@ -478,7 +378,10 @@ static int erofs_xattr_iter_shared(struct erofs_xattr_iter *it,
                if (IS_ERR(it->kaddr))
                        return PTR_ERR(it->kaddr);
 
-               ret = xattr_foreach(it, op, NULL);
+               if (getxattr)
+                       ret = erofs_getxattr_foreach(it);
+               else
+                       ret = erofs_listxattr_foreach(it);
                if ((getxattr && ret != -ENOATTR) || (!getxattr && ret))
                        break;
        }