changes since v7:
- patch 1: add Reviewed-by tag
- patch 4: s/erofs_iter_inline_xattr/erofs_xattr_iter_inline
s/erofs_iter_shared_xattr/erofs_xattr_iter_shared
- patch 5: polish commit message; tweak overly long lines
changes since v6:
- patch 1: s/__u32/__le32 in calculation of it->pos (Gao Xiang)
- patch 4: polish the commit message (Gao Xiang)
- patch 5: s/erofs_xattr_handle_string/erofs_xattr_copy_to_buffer (Gao
Xiang)
changes since v5:
- patch 1: newly added into this series, in preparation for the
following cleanup
- patch 5: rename erofs_xattr_body() to erofs_xattr_handle_string()
changes since v4:
- patch 1: make conversions from erofs_read_metabuf() in xattr.c
to "erofs_init_metabuf() + erofs_bread()" a separate patch
- patch 6: add "bool copy" function parameter to erofs_xattr_body(), and
thus make erofs_xattr_namematch() and erofs_xattr_copy() inlined
inside erofs_xattr_body()
changes since v3:
- patch 1: make a unified erofs_xattr_iter_fixup() API with newly
introduced "bool nospan" argument; call erofs_init_metabuf() and
erofs_bread() separately instead of erofs_read_metabuf()
- patch 2: avoid duplicated strlen() calculation in erofs_getxattr(); no
need zeroing other fields when initializing 'struct erofs_xattr_iter'
- patch 4: don't explode 'struct erofs_xattr_iter' with inode/getxattr
fields; instead pass inode/getxattr parameters through function
parameters of erofs_iter_[inline|shared]_xattr()
- patch 5: don't explode 'struct erofs_xattr_iter' with remaining field;
instead calculate and check the remaining inside
erofs_iter_inline_xattr()
changes since v2:
- rebase to v6.4-rc2
- passes xattr tests (erofs/019,020,021) of erofs-utils [1]
[1] https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/log/?h=experimental-tests
v7: https://lore.kernel.org/all/[email protected]/
v6: https://lore.kernel.org/all/[email protected]/
v5: https://lore.kernel.org/all/[email protected]/
v4: https://lore.kernel.org/all/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/
Jingbo Xu (5):
erofs: use absolute position in xattr iterator
erofs: unify xattr_iter structures
erofs: make the size of read data stored in buffer_ofs
erofs: unify inline/shared xattr iterators for listxattr/getxattr
erofs: use separate xattr parsers for listxattr/getxattr
fs/erofs/xattr.c | 663 +++++++++++++++++------------------------------
1 file changed, 235 insertions(+), 428 deletions(-)
--
2.19.1.6.gb485710b
Unify xattr_iter/listxattr_iter/getxattr_iter structures into
erofs_xattr_iter structure.
This is in preparation for the following further cleanup.
Signed-off-by: Jingbo Xu <[email protected]>
Reviewed-by: Gao Xiang <[email protected]>
---
fs/erofs/xattr.c | 146 ++++++++++++++++++++---------------------------
1 file changed, 62 insertions(+), 84 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 4c11d4f4cf07..b2802121e3aa 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -7,17 +7,27 @@
#include <linux/security.h>
#include "xattr.h"
-struct xattr_iter {
+struct erofs_xattr_iter {
struct super_block *sb;
struct erofs_buf buf;
erofs_off_t pos;
void *kaddr;
+
+ char *buffer;
+ int buffer_size, buffer_ofs;
+
+ /* getxattr */
+ int index, infix_len;
+ struct qstr name;
+
+ /* listxattr */
+ struct dentry *dentry;
};
static int erofs_init_inode_xattrs(struct inode *inode)
{
struct erofs_inode *const vi = EROFS_I(inode);
- struct xattr_iter it;
+ struct erofs_xattr_iter it;
unsigned int i;
struct erofs_xattr_ibody_header *ih;
struct super_block *sb = inode->i_sb;
@@ -121,15 +131,15 @@ static int erofs_init_inode_xattrs(struct inode *inode)
* and need to be handled
*/
struct xattr_iter_handlers {
- int (*entry)(struct xattr_iter *_it, struct erofs_xattr_entry *entry);
- int (*name)(struct xattr_iter *_it, unsigned int processed, char *buf,
+ 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 xattr_iter *_it, unsigned int value_sz);
- void (*value)(struct xattr_iter *_it, unsigned int processed, char *buf,
+ 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);
};
-static int inline_xattr_iter_begin(struct xattr_iter *it,
+static int inline_xattr_iter_begin(struct erofs_xattr_iter *it,
struct inode *inode)
{
struct erofs_inode *const vi = EROFS_I(inode);
@@ -154,7 +164,7 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
* 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 xattr_iter *it,
+static int xattr_foreach(struct erofs_xattr_iter *it,
const struct xattr_iter_handlers *op,
unsigned int *tlimit)
{
@@ -257,18 +267,10 @@ static int xattr_foreach(struct xattr_iter *it,
return err < 0 ? err : 0;
}
-struct getxattr_iter {
- struct xattr_iter it;
-
- char *buffer;
- int buffer_size, index, infix_len;
- struct qstr name;
-};
-
-static int erofs_xattr_long_entrymatch(struct getxattr_iter *it,
+static int erofs_xattr_long_entrymatch(struct erofs_xattr_iter *it,
struct erofs_xattr_entry *entry)
{
- struct erofs_sb_info *sbi = EROFS_SB(it->it.sb);
+ 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);
@@ -286,11 +288,9 @@ static int erofs_xattr_long_entrymatch(struct getxattr_iter *it,
return 0;
}
-static int xattr_entrymatch(struct xattr_iter *_it,
+static int xattr_entrymatch(struct erofs_xattr_iter *it,
struct erofs_xattr_entry *entry)
{
- struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);
-
/* 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);
@@ -302,32 +302,27 @@ static int xattr_entrymatch(struct xattr_iter *_it,
return 0;
}
-static int xattr_namematch(struct xattr_iter *_it,
+static int xattr_namematch(struct erofs_xattr_iter *it,
unsigned int processed, char *buf, unsigned int len)
{
- struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);
-
if (memcmp(buf, it->name.name + it->infix_len + processed, len))
return -ENOATTR;
return 0;
}
-static int xattr_checkbuffer(struct xattr_iter *_it,
+static int xattr_checkbuffer(struct erofs_xattr_iter *it,
unsigned int value_sz)
{
- struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);
int err = it->buffer_size < value_sz ? -ERANGE : 0;
it->buffer_size = value_sz;
return !it->buffer ? 1 : err;
}
-static void xattr_copyvalue(struct xattr_iter *_it,
+static void xattr_copyvalue(struct erofs_xattr_iter *it,
unsigned int processed,
char *buf, unsigned int len)
{
- struct getxattr_iter *it = container_of(_it, struct getxattr_iter, it);
-
memcpy(it->buffer + processed, buf, len);
}
@@ -338,41 +333,41 @@ static const struct xattr_iter_handlers find_xattr_handlers = {
.value = xattr_copyvalue
};
-static int inline_getxattr(struct inode *inode, struct getxattr_iter *it)
+static int inline_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
{
int ret;
unsigned int remaining;
- ret = inline_xattr_iter_begin(&it->it, inode);
+ ret = inline_xattr_iter_begin(it, inode);
if (ret < 0)
return ret;
remaining = ret;
while (remaining) {
- ret = xattr_foreach(&it->it, &find_xattr_handlers, &remaining);
+ ret = xattr_foreach(it, &find_xattr_handlers, &remaining);
if (ret != -ENOATTR)
break;
}
return ret ? ret : it->buffer_size;
}
-static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
+static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
{
struct erofs_inode *const vi = EROFS_I(inode);
- struct super_block *const sb = it->it.sb;
+ struct super_block *const sb = it->sb;
struct erofs_sb_info *sbi = EROFS_SB(sb);
unsigned int i;
int ret = -ENOATTR;
for (i = 0; i < vi->xattr_shared_count; ++i) {
- it->it.pos = erofs_pos(sb, sbi->xattr_blkaddr) +
+ it->pos = erofs_pos(sb, sbi->xattr_blkaddr) +
vi->xattr_shared_xattrs[i] * sizeof(__le32);
- it->it.kaddr = erofs_bread(&it->it.buf,
- erofs_blknr(sb, it->it.pos), EROFS_KMAP);
- if (IS_ERR(it->it.kaddr))
- return PTR_ERR(it->it.kaddr);
+ it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos),
+ EROFS_KMAP);
+ if (IS_ERR(it->kaddr))
+ return PTR_ERR(it->kaddr);
- ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
+ ret = xattr_foreach(it, &find_xattr_handlers, NULL);
if (ret != -ENOATTR)
break;
}
@@ -394,7 +389,7 @@ int erofs_getxattr(struct inode *inode, int index,
void *buffer, size_t buffer_size)
{
int ret;
- struct getxattr_iter it;
+ struct erofs_xattr_iter it;
if (!name)
return -EINVAL;
@@ -404,22 +399,21 @@ int erofs_getxattr(struct inode *inode, int index,
return ret;
it.index = index;
- it.name.len = strlen(name);
+ it.name = (struct qstr)QSTR_INIT(name, strlen(name));
if (it.name.len > EROFS_NAME_LEN)
return -ERANGE;
- it.it.sb = inode->i_sb;
- it.it.buf = __EROFS_BUF_INITIALIZER;
- erofs_init_metabuf(&it.it.buf, it.it.sb);
- it.name.name = name;
-
+ it.sb = inode->i_sb;
+ it.buf = __EROFS_BUF_INITIALIZER;
+ erofs_init_metabuf(&it.buf, it.sb);
it.buffer = buffer;
it.buffer_size = buffer_size;
+ it.buffer_ofs = 0;
ret = inline_getxattr(inode, &it);
if (ret == -ENOATTR)
ret = shared_getxattr(inode, &it);
- erofs_put_metabuf(&it.it.buf);
+ erofs_put_metabuf(&it.buf);
return ret;
}
@@ -465,25 +459,15 @@ const struct xattr_handler *erofs_xattr_handlers[] = {
NULL,
};
-struct listxattr_iter {
- struct xattr_iter it;
-
- struct dentry *dentry;
- char *buffer;
- int buffer_size, buffer_ofs;
-};
-
-static int xattr_entrylist(struct xattr_iter *_it,
+static int xattr_entrylist(struct erofs_xattr_iter *it,
struct erofs_xattr_entry *entry)
{
- struct listxattr_iter *it =
- container_of(_it, struct listxattr_iter, it);
unsigned int base_index = entry->e_name_index;
unsigned int prefix_len, infix_len = 0;
const char *prefix, *infix = NULL;
if (entry->e_name_index & EROFS_XATTR_LONG_PREFIX) {
- struct erofs_sb_info *sbi = EROFS_SB(_it->sb);
+ 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);
@@ -515,23 +499,17 @@ static int xattr_entrylist(struct xattr_iter *_it,
return 0;
}
-static int xattr_namelist(struct xattr_iter *_it,
+static int xattr_namelist(struct erofs_xattr_iter *it,
unsigned int processed, char *buf, unsigned int len)
{
- struct listxattr_iter *it =
- container_of(_it, struct listxattr_iter, it);
-
memcpy(it->buffer + it->buffer_ofs, buf, len);
it->buffer_ofs += len;
return 0;
}
-static int xattr_skipvalue(struct xattr_iter *_it,
+static int xattr_skipvalue(struct erofs_xattr_iter *it,
unsigned int value_sz)
{
- struct listxattr_iter *it =
- container_of(_it, struct listxattr_iter, it);
-
it->buffer[it->buffer_ofs++] = '\0';
return 1;
}
@@ -543,42 +521,42 @@ static const struct xattr_iter_handlers list_xattr_handlers = {
.value = NULL
};
-static int inline_listxattr(struct listxattr_iter *it)
+static int inline_listxattr(struct erofs_xattr_iter *it)
{
int ret;
unsigned int remaining;
- ret = inline_xattr_iter_begin(&it->it, d_inode(it->dentry));
+ ret = inline_xattr_iter_begin(it, d_inode(it->dentry));
if (ret < 0)
return ret;
remaining = ret;
while (remaining) {
- ret = xattr_foreach(&it->it, &list_xattr_handlers, &remaining);
+ ret = xattr_foreach(it, &list_xattr_handlers, &remaining);
if (ret)
break;
}
return ret ? ret : it->buffer_ofs;
}
-static int shared_listxattr(struct listxattr_iter *it)
+static int shared_listxattr(struct erofs_xattr_iter *it)
{
struct inode *const inode = d_inode(it->dentry);
struct erofs_inode *const vi = EROFS_I(inode);
- struct super_block *const sb = it->it.sb;
+ struct super_block *const sb = it->sb;
struct erofs_sb_info *sbi = EROFS_SB(sb);
unsigned int i;
int ret = 0;
for (i = 0; i < vi->xattr_shared_count; ++i) {
- it->it.pos = erofs_pos(sb, sbi->xattr_blkaddr) +
+ it->pos = erofs_pos(sb, sbi->xattr_blkaddr) +
vi->xattr_shared_xattrs[i] * sizeof(__le32);
- it->it.kaddr = erofs_bread(&it->it.buf,
- erofs_blknr(sb, it->it.pos), EROFS_KMAP);
- if (IS_ERR(it->it.kaddr))
- return PTR_ERR(it->it.kaddr);
+ it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos),
+ EROFS_KMAP);
+ if (IS_ERR(it->kaddr))
+ return PTR_ERR(it->kaddr);
- ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
+ ret = xattr_foreach(it, &list_xattr_handlers, NULL);
if (ret)
break;
}
@@ -589,7 +567,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
char *buffer, size_t buffer_size)
{
int ret;
- struct listxattr_iter it;
+ struct erofs_xattr_iter it;
ret = erofs_init_inode_xattrs(d_inode(dentry));
if (ret == -ENOATTR)
@@ -597,9 +575,9 @@ ssize_t erofs_listxattr(struct dentry *dentry,
if (ret)
return ret;
- it.it.sb = dentry->d_sb;
- it.it.buf = __EROFS_BUF_INITIALIZER;
- erofs_init_metabuf(&it.it.buf, it.it.sb);
+ it.sb = dentry->d_sb;
+ it.buf = __EROFS_BUF_INITIALIZER;
+ erofs_init_metabuf(&it.buf, it.sb);
it.dentry = dentry;
it.buffer = buffer;
it.buffer_size = buffer_size;
@@ -608,7 +586,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
ret = inline_listxattr(&it);
if (ret >= 0 || ret == -ENOATTR)
ret = shared_listxattr(&it);
- erofs_put_metabuf(&it.it.buf);
+ erofs_put_metabuf(&it.buf);
return ret;
}
--
2.19.1.6.gb485710b
Since now xattr_iter structures have been unified, make the size of the
read data stored in buffer_ofs. Don't bother reusing buffer_size for
this use, which may be confusing.
This is in preparation for the following further cleanup.
Signed-off-by: Jingbo Xu <[email protected]>
Reviewed-by: Gao Xiang <[email protected]>
---
fs/erofs/xattr.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index b2802121e3aa..8a114c7b6c66 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -315,7 +315,7 @@ static int xattr_checkbuffer(struct erofs_xattr_iter *it,
{
int err = it->buffer_size < value_sz ? -ERANGE : 0;
- it->buffer_size = value_sz;
+ it->buffer_ofs = value_sz;
return !it->buffer ? 1 : err;
}
@@ -348,7 +348,7 @@ static int inline_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
if (ret != -ENOATTR)
break;
}
- return ret ? ret : it->buffer_size;
+ return ret ? ret : it->buffer_ofs;
}
static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
@@ -371,7 +371,7 @@ static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
if (ret != -ENOATTR)
break;
}
- return ret ? ret : it->buffer_size;
+ return ret ? ret : it->buffer_ofs;
}
static bool erofs_xattr_user_list(struct dentry *dentry)
--
2.19.1.6.gb485710b
Replace blkaddr/ofs with pos in 'struct erofs_xattr_iter'.
After erofs_bread() is introduced to replace raw page cache APIs for
metadata I/Os handling, xattr_iter_fixup() is no longer needed anymore.
In addition, it is also unnecessary to check if the iterated position is
span over the block boundary as absolute offset is used instead of
blkaddr + offset pairs.
Signed-off-by: Jingbo Xu <[email protected]>
Reviewed-by: Gao Xiang <[email protected]>
---
fs/erofs/xattr.c | 162 +++++++++++++++++++----------------------------
1 file changed, 65 insertions(+), 97 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index d9e041d27a35..4c11d4f4cf07 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -7,26 +7,11 @@
#include <linux/security.h>
#include "xattr.h"
-static inline erofs_blk_t erofs_xattr_blkaddr(struct super_block *sb,
- unsigned int xattr_id)
-{
- return EROFS_SB(sb)->xattr_blkaddr +
- erofs_blknr(sb, xattr_id * sizeof(__u32));
-}
-
-static inline unsigned int erofs_xattr_blkoff(struct super_block *sb,
- unsigned int xattr_id)
-{
- return erofs_blkoff(sb, xattr_id * sizeof(__u32));
-}
-
struct xattr_iter {
struct super_block *sb;
struct erofs_buf buf;
+ erofs_off_t pos;
void *kaddr;
-
- erofs_blk_t blkaddr;
- unsigned int ofs;
};
static int erofs_init_inode_xattrs(struct inode *inode)
@@ -82,17 +67,16 @@ static int erofs_init_inode_xattrs(struct inode *inode)
it.buf = __EROFS_BUF_INITIALIZER;
erofs_init_metabuf(&it.buf, sb);
- it.blkaddr = erofs_blknr(sb, erofs_iloc(inode) + vi->inode_isize);
- it.ofs = erofs_blkoff(sb, erofs_iloc(inode) + vi->inode_isize);
+ it.pos = erofs_iloc(inode) + vi->inode_isize;
/* read in shared xattr array (non-atomic, see kmalloc below) */
- it.kaddr = erofs_bread(&it.buf, it.blkaddr, EROFS_KMAP);
+ it.kaddr = erofs_bread(&it.buf, erofs_blknr(sb, it.pos), EROFS_KMAP);
if (IS_ERR(it.kaddr)) {
ret = PTR_ERR(it.kaddr);
goto out_unlock;
}
- ih = (struct erofs_xattr_ibody_header *)(it.kaddr + it.ofs);
+ ih = it.kaddr + erofs_blkoff(sb, it.pos);
vi->xattr_shared_count = ih->h_shared_count;
vi->xattr_shared_xattrs = kmalloc_array(vi->xattr_shared_count,
sizeof(uint), GFP_KERNEL);
@@ -103,25 +87,20 @@ static int erofs_init_inode_xattrs(struct inode *inode)
}
/* let's skip ibody header */
- it.ofs += sizeof(struct erofs_xattr_ibody_header);
+ it.pos += sizeof(struct erofs_xattr_ibody_header);
for (i = 0; i < vi->xattr_shared_count; ++i) {
- if (it.ofs >= sb->s_blocksize) {
- /* cannot be unaligned */
- DBG_BUGON(it.ofs != sb->s_blocksize);
-
- it.kaddr = erofs_bread(&it.buf, ++it.blkaddr, EROFS_KMAP);
- if (IS_ERR(it.kaddr)) {
- kfree(vi->xattr_shared_xattrs);
- vi->xattr_shared_xattrs = NULL;
- ret = PTR_ERR(it.kaddr);
- goto out_unlock;
- }
- it.ofs = 0;
+ it.kaddr = erofs_bread(&it.buf, erofs_blknr(sb, it.pos),
+ EROFS_KMAP);
+ if (IS_ERR(it.kaddr)) {
+ kfree(vi->xattr_shared_xattrs);
+ vi->xattr_shared_xattrs = NULL;
+ ret = PTR_ERR(it.kaddr);
+ goto out_unlock;
}
- vi->xattr_shared_xattrs[i] =
- le32_to_cpu(*(__le32 *)(it.kaddr + it.ofs));
- it.ofs += sizeof(__le32);
+ vi->xattr_shared_xattrs[i] = le32_to_cpu(*(__le32 *)
+ (it.kaddr + erofs_blkoff(sb, it.pos)));
+ it.pos += sizeof(__le32);
}
erofs_put_metabuf(&it.buf);
@@ -150,24 +129,11 @@ struct xattr_iter_handlers {
unsigned int len);
};
-static inline int xattr_iter_fixup(struct xattr_iter *it)
-{
- if (it->ofs < it->sb->s_blocksize)
- return 0;
-
- it->blkaddr += erofs_blknr(it->sb, it->ofs);
- it->kaddr = erofs_bread(&it->buf, it->blkaddr, EROFS_KMAP);
- if (IS_ERR(it->kaddr))
- return PTR_ERR(it->kaddr);
- it->ofs = erofs_blkoff(it->sb, it->ofs);
- return 0;
-}
-
static int inline_xattr_iter_begin(struct xattr_iter *it,
struct inode *inode)
{
struct erofs_inode *const vi = EROFS_I(inode);
- unsigned int xattr_header_sz, inline_xattr_ofs;
+ unsigned int xattr_header_sz;
xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) +
sizeof(u32) * vi->xattr_shared_count;
@@ -176,11 +142,9 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
return -ENOATTR;
}
- inline_xattr_ofs = vi->inode_isize + xattr_header_sz;
-
- it->blkaddr = erofs_blknr(it->sb, erofs_iloc(inode) + inline_xattr_ofs);
- it->ofs = erofs_blkoff(it->sb, erofs_iloc(inode) + inline_xattr_ofs);
- it->kaddr = erofs_bread(&it->buf, it->blkaddr, EROFS_KMAP);
+ 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);
return vi->xattr_isize - xattr_header_sz;
@@ -188,27 +152,29 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
/*
* Regardless of success or failure, `xattr_foreach' will end up with
- * `ofs' pointing to the next xattr item rather than an arbitrary position.
+ * `pos' pointing to the next xattr item rather than an arbitrary position.
*/
static int xattr_foreach(struct 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, ofs, ipage */
- err = xattr_iter_fixup(it);
- if (err)
- return 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 + it->ofs);
+ entry = *(struct erofs_xattr_entry *)
+ (it->kaddr + erofs_blkoff(sb, it->pos));
if (tlimit) {
unsigned int entry_sz = erofs_xattr_entry_size(&entry);
@@ -220,40 +186,40 @@ static int xattr_foreach(struct xattr_iter *it,
*tlimit -= entry_sz;
}
- it->ofs += sizeof(struct erofs_xattr_entry);
+ 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->ofs += entry.e_name_len + value_sz;
+ it->pos += entry.e_name_len + value_sz;
goto out;
}
- /* 2. handle xattr name (ofs will finally be at the end of name) */
+ /* 2. handle xattr name (pos will finally be at the end of name) */
processed = 0;
while (processed < entry.e_name_len) {
- if (it->ofs >= it->sb->s_blocksize) {
- DBG_BUGON(it->ofs > it->sb->s_blocksize);
-
- err = xattr_iter_fixup(it);
- if (err)
- goto out;
- it->ofs = 0;
+ 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, it->sb->s_blocksize - it->ofs,
+ 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 + it->ofs, slice);
+ err = op->name(it, processed,
+ it->kaddr + erofs_blkoff(sb, it->pos), slice);
if (err) {
- it->ofs += entry.e_name_len - processed + value_sz;
+ it->pos += entry.e_name_len - processed + value_sz;
goto out;
}
- it->ofs += slice;
+ it->pos += slice;
processed += slice;
}
@@ -263,31 +229,31 @@ static int xattr_foreach(struct xattr_iter *it,
if (op->alloc_buffer) {
err = op->alloc_buffer(it, value_sz);
if (err) {
- it->ofs += value_sz;
+ it->pos += value_sz;
goto out;
}
}
while (processed < value_sz) {
- if (it->ofs >= it->sb->s_blocksize) {
- DBG_BUGON(it->ofs > it->sb->s_blocksize);
-
- err = xattr_iter_fixup(it);
- if (err)
- goto out;
- it->ofs = 0;
+ 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, it->sb->s_blocksize - it->ofs,
+ slice = min_t(unsigned int,
+ sb->s_blocksize - erofs_blkoff(sb, it->pos),
value_sz - processed);
- op->value(it, processed, it->kaddr + it->ofs, slice);
- it->ofs += slice;
+ 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->ofs = EROFS_XATTR_ALIGN(it->ofs);
+ it->pos = EROFS_XATTR_ALIGN(it->pos);
return err < 0 ? err : 0;
}
@@ -394,14 +360,15 @@ static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
{
struct erofs_inode *const vi = EROFS_I(inode);
struct super_block *const sb = it->it.sb;
- unsigned int i, xsid;
+ struct erofs_sb_info *sbi = EROFS_SB(sb);
+ unsigned int i;
int ret = -ENOATTR;
for (i = 0; i < vi->xattr_shared_count; ++i) {
- xsid = vi->xattr_shared_xattrs[i];
- it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid);
- it->it.ofs = erofs_xattr_blkoff(sb, xsid);
- it->it.kaddr = erofs_bread(&it->it.buf, it->it.blkaddr, EROFS_KMAP);
+ it->it.pos = erofs_pos(sb, sbi->xattr_blkaddr) +
+ vi->xattr_shared_xattrs[i] * sizeof(__le32);
+ it->it.kaddr = erofs_bread(&it->it.buf,
+ erofs_blknr(sb, it->it.pos), EROFS_KMAP);
if (IS_ERR(it->it.kaddr))
return PTR_ERR(it->it.kaddr);
@@ -599,14 +566,15 @@ static int shared_listxattr(struct listxattr_iter *it)
struct inode *const inode = d_inode(it->dentry);
struct erofs_inode *const vi = EROFS_I(inode);
struct super_block *const sb = it->it.sb;
- unsigned int i, xsid;
+ struct erofs_sb_info *sbi = EROFS_SB(sb);
+ unsigned int i;
int ret = 0;
for (i = 0; i < vi->xattr_shared_count; ++i) {
- xsid = vi->xattr_shared_xattrs[i];
- it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid);
- it->it.ofs = erofs_xattr_blkoff(sb, xsid);
- it->it.kaddr = erofs_bread(&it->it.buf, it->it.blkaddr, EROFS_KMAP);
+ it->it.pos = erofs_pos(sb, sbi->xattr_blkaddr) +
+ vi->xattr_shared_xattrs[i] * sizeof(__le32);
+ it->it.kaddr = erofs_bread(&it->it.buf,
+ erofs_blknr(sb, it->it.pos), EROFS_KMAP);
if (IS_ERR(it->it.kaddr))
return PTR_ERR(it->it.kaddr);
--
2.19.1.6.gb485710b
Make inline_{list,get}xattr() as well as inline_xattr_iter_begin()
unified as erofs_xattr_iter_inline(), and shared_{list,get}xattr()
unified as erofs_xattr_iter_shared().
After these changes, both erofs_xattr_iter_{inline,shared}() return 0 on
success, and negative error on failure.
One thing worth noting is that, the logic of returning it->buffer_ofs
when there's no shared xattrs in shared_listxattr() is moved to
erofs_listxattr() to make the unification possible. The only difference
is that, semantically the old behavior will return ENOATTR rather than
it->buffer_ofs if ENOATTR encountered when listxattr is parsing upon a
specific shared xattr, while now the new behavior will return
it->buffer_ofs in this case. This is not an issue, as listxattr upon a
specific xattr won't return ENOATTR.
Signed-off-by: Jingbo Xu <[email protected]>
---
fs/erofs/xattr.c | 188 ++++++++++++++++++-----------------------------
1 file changed, 73 insertions(+), 115 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 8a114c7b6c66..c3cb01a8b446 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -139,27 +139,6 @@ struct xattr_iter_handlers {
unsigned int len);
};
-static int inline_xattr_iter_begin(struct erofs_xattr_iter *it,
- struct inode *inode)
-{
- struct erofs_inode *const vi = EROFS_I(inode);
- unsigned int xattr_header_sz;
-
- xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) +
- sizeof(u32) * vi->xattr_shared_count;
- if (xattr_header_sz >= vi->xattr_isize) {
- DBG_BUGON(xattr_header_sz > vi->xattr_isize);
- 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);
- return vi->xattr_isize - xattr_header_sz;
-}
-
/*
* Regardless of success or failure, `xattr_foreach' will end up with
* `pos' pointing to the next xattr item rather than an arbitrary position.
@@ -333,47 +312,6 @@ static const struct xattr_iter_handlers find_xattr_handlers = {
.value = xattr_copyvalue
};
-static int inline_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
-{
- int ret;
- unsigned int remaining;
-
- ret = inline_xattr_iter_begin(it, inode);
- if (ret < 0)
- return ret;
-
- remaining = ret;
- while (remaining) {
- ret = xattr_foreach(it, &find_xattr_handlers, &remaining);
- if (ret != -ENOATTR)
- break;
- }
- return ret ? ret : it->buffer_ofs;
-}
-
-static int shared_getxattr(struct inode *inode, struct erofs_xattr_iter *it)
-{
- struct erofs_inode *const vi = EROFS_I(inode);
- struct super_block *const sb = it->sb;
- struct erofs_sb_info *sbi = EROFS_SB(sb);
- unsigned int i;
- int ret = -ENOATTR;
-
- for (i = 0; i < vi->xattr_shared_count; ++i) {
- it->pos = erofs_pos(sb, sbi->xattr_blkaddr) +
- vi->xattr_shared_xattrs[i] * sizeof(__le32);
- it->kaddr = erofs_bread(&it->buf, erofs_blknr(sb, it->pos),
- EROFS_KMAP);
- if (IS_ERR(it->kaddr))
- return PTR_ERR(it->kaddr);
-
- ret = xattr_foreach(it, &find_xattr_handlers, NULL);
- if (ret != -ENOATTR)
- break;
- }
- return ret ? ret : it->buffer_ofs;
-}
-
static bool erofs_xattr_user_list(struct dentry *dentry)
{
return test_opt(&EROFS_SB(dentry->d_sb)->opt, XATTR_USER);
@@ -384,39 +322,6 @@ static bool erofs_xattr_trusted_list(struct dentry *dentry)
return capable(CAP_SYS_ADMIN);
}
-int erofs_getxattr(struct inode *inode, int index,
- const char *name,
- void *buffer, size_t buffer_size)
-{
- int ret;
- struct erofs_xattr_iter it;
-
- if (!name)
- return -EINVAL;
-
- ret = erofs_init_inode_xattrs(inode);
- if (ret)
- return ret;
-
- it.index = index;
- it.name = (struct qstr)QSTR_INIT(name, strlen(name));
- if (it.name.len > EROFS_NAME_LEN)
- return -ERANGE;
-
- it.sb = inode->i_sb;
- it.buf = __EROFS_BUF_INITIALIZER;
- erofs_init_metabuf(&it.buf, it.sb);
- it.buffer = buffer;
- it.buffer_size = buffer_size;
- it.buffer_ofs = 0;
-
- ret = inline_getxattr(inode, &it);
- if (ret == -ENOATTR)
- ret = shared_getxattr(inode, &it);
- erofs_put_metabuf(&it.buf);
- return ret;
-}
-
static int erofs_xattr_generic_get(const struct xattr_handler *handler,
struct dentry *unused, struct inode *inode,
const char *name, void *buffer, size_t size)
@@ -521,32 +426,49 @@ static const struct xattr_iter_handlers list_xattr_handlers = {
.value = NULL
};
-static int inline_listxattr(struct erofs_xattr_iter *it)
+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;
int ret;
- unsigned int remaining;
- ret = inline_xattr_iter_begin(it, d_inode(it->dentry));
- if (ret < 0)
- return ret;
+ xattr_header_sz = sizeof(struct erofs_xattr_ibody_header) +
+ sizeof(u32) * vi->xattr_shared_count;
+ if (xattr_header_sz >= vi->xattr_isize) {
+ DBG_BUGON(xattr_header_sz > vi->xattr_isize);
+ 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;
- remaining = ret;
while (remaining) {
- ret = xattr_foreach(it, &list_xattr_handlers, &remaining);
- if (ret)
+ ret = xattr_foreach(it, op, &remaining);
+ if ((getxattr && ret != -ENOATTR) || (!getxattr && ret))
break;
}
- return ret ? ret : it->buffer_ofs;
+ return ret;
}
-static int shared_listxattr(struct erofs_xattr_iter *it)
+static int erofs_xattr_iter_shared(struct erofs_xattr_iter *it,
+ struct inode *inode, bool getxattr)
{
- struct inode *const inode = d_inode(it->dentry);
struct erofs_inode *const vi = EROFS_I(inode);
struct super_block *const sb = it->sb;
struct erofs_sb_info *sbi = EROFS_SB(sb);
unsigned int i;
- int ret = 0;
+ 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) +
@@ -556,11 +478,44 @@ static int shared_listxattr(struct erofs_xattr_iter *it)
if (IS_ERR(it->kaddr))
return PTR_ERR(it->kaddr);
- ret = xattr_foreach(it, &list_xattr_handlers, NULL);
- if (ret)
+ ret = xattr_foreach(it, op, NULL);
+ if ((getxattr && ret != -ENOATTR) || (!getxattr && ret))
break;
}
- return ret ? ret : it->buffer_ofs;
+ return ret;
+}
+
+int erofs_getxattr(struct inode *inode, int index,
+ const char *name,
+ void *buffer, size_t buffer_size)
+{
+ int ret;
+ struct erofs_xattr_iter it;
+
+ if (!name)
+ return -EINVAL;
+
+ ret = erofs_init_inode_xattrs(inode);
+ if (ret)
+ return ret;
+
+ it.index = index;
+ it.name = (struct qstr)QSTR_INIT(name, strlen(name));
+ if (it.name.len > EROFS_NAME_LEN)
+ return -ERANGE;
+
+ it.sb = inode->i_sb;
+ it.buf = __EROFS_BUF_INITIALIZER;
+ erofs_init_metabuf(&it.buf, it.sb);
+ it.buffer = buffer;
+ it.buffer_size = buffer_size;
+ it.buffer_ofs = 0;
+
+ ret = erofs_xattr_iter_inline(&it, inode, true);
+ if (ret == -ENOATTR)
+ ret = erofs_xattr_iter_shared(&it, inode, true);
+ erofs_put_metabuf(&it.buf);
+ return ret ? ret : it.buffer_ofs;
}
ssize_t erofs_listxattr(struct dentry *dentry,
@@ -568,8 +523,9 @@ ssize_t erofs_listxattr(struct dentry *dentry,
{
int ret;
struct erofs_xattr_iter it;
+ struct inode *inode = d_inode(dentry);
- ret = erofs_init_inode_xattrs(d_inode(dentry));
+ ret = erofs_init_inode_xattrs(inode);
if (ret == -ENOATTR)
return 0;
if (ret)
@@ -583,11 +539,13 @@ ssize_t erofs_listxattr(struct dentry *dentry,
it.buffer_size = buffer_size;
it.buffer_ofs = 0;
- ret = inline_listxattr(&it);
- if (ret >= 0 || ret == -ENOATTR)
- ret = shared_listxattr(&it);
+ ret = erofs_xattr_iter_inline(&it, inode, false);
+ if (!ret || ret == -ENOATTR)
+ ret = erofs_xattr_iter_shared(&it, inode, false);
+ if (ret == -ENOATTR)
+ ret = 0;
erofs_put_metabuf(&it.buf);
- return ret;
+ return ret ? ret : it.buffer_ofs;
}
void erofs_xattr_prefixes_cleanup(struct super_block *sb)
--
2.19.1.6.gb485710b
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 <[email protected]>
---
fs/erofs/xattr.c | 371 +++++++++++++++++------------------------------
1 file changed, 137 insertions(+), 234 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index c3cb01a8b446..4376f654474d 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -123,195 +123,6 @@ static int erofs_init_inode_xattrs(struct inode *inode)
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;
}
--
2.19.1.6.gb485710b
On 2023/6/13 15:41, Jingbo Xu wrote:
...
> +
> +int erofs_getxattr(struct inode *inode, int index,
> + const char *name,
> + void *buffer, size_t buffer_size)
I changed these line above into:
int erofs_getxattr(struct inode *inode, int index, const char *name,
void *buffer, size_t buffer_size)
> +{
> + int ret;
...
> ssize_t erofs_listxattr(struct dentry *dentry,
> @@ -568,8 +523,9 @@ ssize_t erofs_listxattr(struct dentry *dentry,
I also move erofs_listxattr definition into a line (since it's just
80 chars).
Otherwise it looks good to me,
Reviewed-by: Gao Xiang <[email protected]>
Thanks,
Gao Xiang
On 2023/6/13 15:41, Jingbo Xu wrote:
> 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 <[email protected]>
Reviewed-by: Gao Xiang <[email protected]>
Thanks,
Gao Xiang