patch 1-4 are trivial cleanups .
patch 5-7 makes inline_getxattr() and inline_listxattr() unified as
iter_inline_xattr(), shared_getxattr() and shared_listxattr() unified as
iter_shared_xattr().
patch 8 converts the callback styled xattr_foreach() to two separate
xattr handling fucntions for listxattr and getxattr, i.e.
erofs_listxattr_foreach() and erofs_getxattr_foreach().
It has passed erofs/019 testcase of erofs-utils.
Jingbo Xu (8):
erofs: move several xattr helpers into xattr.c
erofs: rename init_inode_xattrs with erofs_ prefix
erofs: simplify erofs_xattr_generic_get()
erofs: introduce erofs_xattr_iter_fixup_aligned() helper
erofs: unify xattr_iter structures
erofs: make the size of read data stored in buffer_ofs
erofs: unify inline/share xattr iterators for listxattr/getxattr
erofs: use separate xattr parsers for listxattr/getxattr
fs/erofs/xattr.c | 705 ++++++++++++++++++++---------------------------
fs/erofs/xattr.h | 56 ----
2 files changed, 303 insertions(+), 458 deletions(-)
--
2.19.1.6.gb485710b
There are several xattr helpers not used outside xattr.c, thus move them
into xattr.c as a cleanup.
inlinexattr_header_size() has only one caller, and thus make it inlined
into the caller directly.
Signed-off-by: Jingbo Xu <[email protected]>
---
fs/erofs/xattr.c | 73 +++++++++++++++++++++++++++++++++---------------
fs/erofs/xattr.h | 56 -------------------------------------
2 files changed, 51 insertions(+), 78 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 459caa3cd65d..760ec864a39c 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -7,6 +7,19 @@
#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;
@@ -157,7 +170,8 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
struct erofs_inode *const vi = EROFS_I(inode);
unsigned int xattr_header_sz, inline_xattr_ofs;
- xattr_header_sz = inlinexattr_header_size(inode);
+ 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;
@@ -351,20 +365,18 @@ static int inline_getxattr(struct inode *inode, struct getxattr_iter *it)
static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
{
struct erofs_inode *const vi = EROFS_I(inode);
- struct super_block *const sb = inode->i_sb;
- unsigned int i;
+ struct super_block *const sb = it->it.sb;
+ unsigned int i, xsid;
int ret = -ENOATTR;
for (i = 0; i < vi->xattr_shared_count; ++i) {
- erofs_blk_t blkaddr =
- xattrblock_addr(sb, vi->xattr_shared_xattrs[i]);
-
- it->it.ofs = xattrblock_offset(sb, vi->xattr_shared_xattrs[i]);
- it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
- EROFS_KMAP);
+ 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_read_metabuf(&it->it.buf, sb,
+ it->it.blkaddr, EROFS_KMAP);
if (IS_ERR(it->it.kaddr))
return PTR_ERR(it->it.kaddr);
- it->it.blkaddr = blkaddr;
ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
if (ret != -ENOATTR)
@@ -383,9 +395,8 @@ 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)
+static int erofs_getxattr(struct inode *inode, int index, const char *name,
+ void *buffer, size_t buffer_size)
{
int ret;
struct getxattr_iter it;
@@ -473,6 +484,26 @@ const struct xattr_handler *erofs_xattr_handlers[] = {
NULL,
};
+static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
+{
+ static const struct xattr_handler *xattr_handler_map[] = {
+ [EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
+#ifdef CONFIG_EROFS_FS_POSIX_ACL
+ [EROFS_XATTR_INDEX_POSIX_ACL_ACCESS] =
+ &posix_acl_access_xattr_handler,
+ [EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT] =
+ &posix_acl_default_xattr_handler,
+#endif
+ [EROFS_XATTR_INDEX_TRUSTED] = &erofs_xattr_trusted_handler,
+#ifdef CONFIG_EROFS_FS_SECURITY
+ [EROFS_XATTR_INDEX_SECURITY] = &erofs_xattr_security_handler,
+#endif
+ };
+
+ return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
+ xattr_handler_map[idx] : NULL;
+}
+
struct listxattr_iter {
struct xattr_iter it;
@@ -562,20 +593,18 @@ 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 = inode->i_sb;
- unsigned int i;
+ struct super_block *const sb = it->it.sb;
+ unsigned int i, xsid;
int ret = 0;
for (i = 0; i < vi->xattr_shared_count; ++i) {
- erofs_blk_t blkaddr =
- xattrblock_addr(sb, vi->xattr_shared_xattrs[i]);
-
- it->it.ofs = xattrblock_offset(sb, vi->xattr_shared_xattrs[i]);
- it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
- EROFS_KMAP);
+ 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_read_metabuf(&it->it.buf, sb,
+ it->it.blkaddr, EROFS_KMAP);
if (IS_ERR(it->it.kaddr))
return PTR_ERR(it->it.kaddr);
- it->it.blkaddr = blkaddr;
ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
if (ret)
diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
index f7a21aaa9755..cc9ef97dbf8e 100644
--- a/fs/erofs/xattr.h
+++ b/fs/erofs/xattr.h
@@ -13,66 +13,10 @@
/* Attribute not found */
#define ENOATTR ENODATA
-static inline unsigned int inlinexattr_header_size(struct inode *inode)
-{
- return sizeof(struct erofs_xattr_ibody_header) +
- sizeof(u32) * EROFS_I(inode)->xattr_shared_count;
-}
-
-static inline erofs_blk_t xattrblock_addr(struct super_block *sb,
- unsigned int xattr_id)
-{
#ifdef CONFIG_EROFS_FS_XATTR
- return EROFS_SB(sb)->xattr_blkaddr +
- xattr_id * sizeof(__u32) / sb->s_blocksize;
-#else
- return 0;
-#endif
-}
-
-static inline unsigned int xattrblock_offset(struct super_block *sb,
- unsigned int xattr_id)
-{
- return (xattr_id * sizeof(__u32)) % sb->s_blocksize;
-}
-
-#ifdef CONFIG_EROFS_FS_XATTR
-extern const struct xattr_handler erofs_xattr_user_handler;
-extern const struct xattr_handler erofs_xattr_trusted_handler;
-extern const struct xattr_handler erofs_xattr_security_handler;
-
-static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
-{
- static const struct xattr_handler *xattr_handler_map[] = {
- [EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
-#ifdef CONFIG_EROFS_FS_POSIX_ACL
- [EROFS_XATTR_INDEX_POSIX_ACL_ACCESS] =
- &posix_acl_access_xattr_handler,
- [EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT] =
- &posix_acl_default_xattr_handler,
-#endif
- [EROFS_XATTR_INDEX_TRUSTED] = &erofs_xattr_trusted_handler,
-#ifdef CONFIG_EROFS_FS_SECURITY
- [EROFS_XATTR_INDEX_SECURITY] = &erofs_xattr_security_handler,
-#endif
- };
-
- return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
- xattr_handler_map[idx] : NULL;
-}
-
extern const struct xattr_handler *erofs_xattr_handlers[];
-
-int erofs_getxattr(struct inode *, int, const char *, void *, size_t);
ssize_t erofs_listxattr(struct dentry *, char *, size_t);
#else
-static inline int erofs_getxattr(struct inode *inode, int index,
- const char *name, void *buffer,
- size_t buffer_size)
-{
- return -EOPNOTSUPP;
-}
-
#define erofs_listxattr (NULL)
#define erofs_xattr_handlers (NULL)
#endif /* !CONFIG_EROFS_FS_XATTR */
--
2.19.1.6.gb485710b
erofs_xattr_generic_get() won't be called from xattr handlers other than
user/trusted/security xattr handler, and thus there's no need of extra
checking.
Signed-off-by: Jingbo Xu <[email protected]>
---
fs/erofs/xattr.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index ab4517e5ec84..b83331a694f3 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -431,20 +431,9 @@ 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)
{
- struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
-
- switch (handler->flags) {
- case EROFS_XATTR_INDEX_USER:
- if (!test_opt(&sbi->opt, XATTR_USER))
- return -EOPNOTSUPP;
- break;
- case EROFS_XATTR_INDEX_TRUSTED:
- break;
- case EROFS_XATTR_INDEX_SECURITY:
- break;
- default:
- return -EINVAL;
- }
+ if (handler->flags == EROFS_XATTR_INDEX_USER &&
+ !test_opt(&EROFS_I_SB(inode)->opt, XATTR_USER))
+ return -EOPNOTSUPP;
return erofs_getxattr(inode, handler->flags, name, buffer, size);
}
--
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]>
---
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 e58996b039f1..264561ccaa44 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -305,7 +305,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;
}
@@ -338,7 +338,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)
@@ -360,7 +360,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
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]>
---
fs/erofs/xattr.c | 166 ++++++++++++++++++++---------------------------
1 file changed, 69 insertions(+), 97 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index bdd9393145ce..e58996b039f1 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -20,16 +20,22 @@ static inline unsigned int erofs_xattr_blkoff(struct super_block *sb,
return erofs_blkoff(sb, xattr_id * sizeof(__u32));
}
-struct xattr_iter {
+struct erofs_xattr_iter {
struct super_block *sb;
struct erofs_buf buf;
void *kaddr;
-
erofs_blk_t blkaddr;
unsigned int ofs;
+
+ char *buffer;
+ int buffer_size, buffer_ofs;
+
+ int index;
+ struct qstr name;
+ struct dentry *dentry;
};
-static inline int erofs_xattr_iter_fixup(struct xattr_iter *it)
+static inline int erofs_xattr_iter_fixup(struct erofs_xattr_iter *it)
{
if (it->ofs < it->sb->s_blocksize)
return 0;
@@ -42,7 +48,7 @@ static inline int erofs_xattr_iter_fixup(struct xattr_iter *it)
return 0;
}
-static inline int erofs_xattr_iter_fixup_aligned(struct xattr_iter *it)
+static inline int erofs_xattr_iter_fixup_aligned(struct erofs_xattr_iter *it)
{
DBG_BUGON(it->ofs > it->sb->s_blocksize);
return erofs_xattr_iter_fixup(it);
@@ -51,7 +57,7 @@ static inline int erofs_xattr_iter_fixup_aligned(struct xattr_iter *it)
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;
@@ -154,15 +160,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);
@@ -190,7 +196,7 @@ 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.
*/
-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)
{
@@ -281,47 +287,32 @@ 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;
- struct qstr name;
-};
-
-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);
-
return (it->index != entry->e_name_index ||
it->name.len != entry->e_name_len) ? -ENOATTR : 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);
-
return memcmp(buf, it->name.name + processed, len) ? -ENOATTR : 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);
}
@@ -332,41 +323,40 @@ 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;
unsigned int i, xsid;
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_read_metabuf(&it->it.buf, sb,
- it->it.blkaddr, EROFS_KMAP);
- if (IS_ERR(it->it.kaddr))
- return PTR_ERR(it->it.kaddr);
-
- ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
+ it->blkaddr = erofs_xattr_blkaddr(sb, xsid);
+ it->ofs = erofs_xattr_blkoff(sb, xsid);
+ it->kaddr = erofs_read_metabuf(&it->buf, sb, it->blkaddr, EROFS_KMAP);
+ if (IS_ERR(it->kaddr))
+ return PTR_ERR(it->kaddr);
+
+ ret = xattr_foreach(it, &find_xattr_handlers, NULL);
if (ret != -ENOATTR)
break;
}
@@ -387,31 +377,30 @@ static int erofs_getxattr(struct inode *inode, int index, const char *name,
void *buffer, size_t buffer_size)
{
int ret;
- struct getxattr_iter it;
+ struct erofs_xattr_iter it;
if (!name)
return -EINVAL;
+ if (strlen(name) > EROFS_NAME_LEN)
+ return -ERANGE;
ret = erofs_init_inode_xattrs(inode);
if (ret)
return ret;
- it.index = index;
- it.name.len = strlen(name);
- if (it.name.len > EROFS_NAME_LEN)
- return -ERANGE;
-
- it.it.buf = __EROFS_BUF_INITIALIZER;
- it.name.name = name;
-
- it.buffer = buffer;
- it.buffer_size = buffer_size;
+ it = (struct erofs_xattr_iter) {
+ .buf = __EROFS_BUF_INITIALIZER,
+ .sb = inode->i_sb,
+ .name = QSTR_INIT(name, strlen(name)),
+ .index = index,
+ .buffer = buffer,
+ .buffer_size = buffer_size,
+ };
- it.it.sb = inode->i_sb;
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;
}
@@ -481,19 +470,9 @@ static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
xattr_handler_map[idx] : 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 prefix_len;
const char *prefix;
@@ -520,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;
}
@@ -548,42 +521,41 @@ 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;
unsigned int i, xsid;
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_read_metabuf(&it->it.buf, sb,
- it->it.blkaddr, EROFS_KMAP);
- if (IS_ERR(it->it.kaddr))
- return PTR_ERR(it->it.kaddr);
-
- ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
+ it->blkaddr = erofs_xattr_blkaddr(sb, xsid);
+ it->ofs = erofs_xattr_blkoff(sb, xsid);
+ it->kaddr = erofs_read_metabuf(&it->buf, sb, it->blkaddr, EROFS_KMAP);
+ if (IS_ERR(it->kaddr))
+ return PTR_ERR(it->kaddr);
+
+ ret = xattr_foreach(it, &list_xattr_handlers, NULL);
if (ret)
break;
}
@@ -594,7 +566,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)
@@ -602,18 +574,18 @@ ssize_t erofs_listxattr(struct dentry *dentry,
if (ret)
return ret;
- it.it.buf = __EROFS_BUF_INITIALIZER;
- it.dentry = dentry;
- it.buffer = buffer;
- it.buffer_size = buffer_size;
- it.buffer_ofs = 0;
-
- it.it.sb = dentry->d_sb;
+ it = (struct erofs_xattr_iter) {
+ .buf = __EROFS_BUF_INITIALIZER,
+ .sb = dentry->d_sb,
+ .dentry = dentry,
+ .buffer = buffer,
+ .buffer_size = buffer_size,
+ };
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
Make inline_getxattr() and inline_listxattr() unified as
iter_inline_xattr(), shared_getxattr() and shared_listxattr() unified as
iter_shared_xattr().
After the unification, both iter_inline_xattr() and iter_shared_xattr()
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 | 210 ++++++++++++++++++-----------------------------
1 file changed, 81 insertions(+), 129 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 264561ccaa44..196b2eb59e29 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -7,18 +7,8 @@
#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));
-}
+static int erofs_getxattr(struct inode *inode, int index, const char *name,
+ void *buffer, size_t buffer_size);
struct erofs_xattr_iter {
struct super_block *sb;
@@ -33,6 +23,8 @@ struct erofs_xattr_iter {
int index;
struct qstr name;
struct dentry *dentry;
+ struct inode *inode;
+ bool getxattr;
};
static inline int erofs_xattr_iter_fixup(struct erofs_xattr_iter *it)
@@ -168,30 +160,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, inline_xattr_ofs;
-
- 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;
- }
-
- 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_read_metabuf(&it->buf, inode->i_sb, it->blkaddr,
- 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
* `ofs' pointing to the next xattr item rather than an arbitrary position.
@@ -323,46 +291,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;
- unsigned int i, xsid;
- int ret = -ENOATTR;
-
- for (i = 0; i < vi->xattr_shared_count; ++i) {
- xsid = vi->xattr_shared_xattrs[i];
- it->blkaddr = erofs_xattr_blkaddr(sb, xsid);
- it->ofs = erofs_xattr_blkoff(sb, xsid);
- it->kaddr = erofs_read_metabuf(&it->buf, sb, it->blkaddr, 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);
@@ -373,37 +301,6 @@ static bool erofs_xattr_trusted_list(struct dentry *dentry)
return capable(CAP_SYS_ADMIN);
}
-static 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;
- if (strlen(name) > EROFS_NAME_LEN)
- return -ERANGE;
-
- ret = erofs_init_inode_xattrs(inode);
- if (ret)
- return ret;
-
- it = (struct erofs_xattr_iter) {
- .buf = __EROFS_BUF_INITIALIZER,
- .sb = inode->i_sb,
- .name = QSTR_INIT(name, strlen(name)),
- .index = index,
- .buffer = buffer,
- .buffer_size = buffer_size,
- };
-
- 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,45 +418,96 @@ static const struct xattr_iter_handlers list_xattr_handlers = {
.value = NULL
};
-static int inline_listxattr(struct erofs_xattr_iter *it)
+static int erofs_iter_inline_xattr(struct erofs_xattr_iter *it)
{
+ struct erofs_inode *const vi = EROFS_I(it->inode);
+ const struct xattr_iter_handlers *op;
+ unsigned int xattr_header_sz, remaining;
+ erofs_off_t pos;
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;
+ }
+
+ pos = erofs_iloc(it->inode) + vi->inode_isize + xattr_header_sz;
+ it->blkaddr = erofs_blknr(it->sb, pos);
+ it->ofs = erofs_blkoff(it->sb, pos);
+ it->kaddr = erofs_read_metabuf(&it->buf, it->sb, it->blkaddr, EROFS_KMAP);
+ if (IS_ERR(it->kaddr))
+ return PTR_ERR(it->kaddr);
+
+ remaining = vi->xattr_isize - xattr_header_sz;
+ op = it->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 ((it->getxattr && ret != -ENOATTR) || (!it->getxattr && ret))
break;
}
- return ret ? ret : it->buffer_ofs;
+ return ret;
}
-static int shared_listxattr(struct erofs_xattr_iter *it)
+static int erofs_iter_shared_xattr(struct erofs_xattr_iter *it)
{
- struct inode *const inode = d_inode(it->dentry);
- struct erofs_inode *const vi = EROFS_I(inode);
+ struct erofs_inode *const vi = EROFS_I(it->inode);
struct super_block *const sb = it->sb;
+ const struct xattr_iter_handlers *op;
unsigned int i, xsid;
- int ret = 0;
+ int ret = -ENOATTR;
+
+ op = it->getxattr ? &find_xattr_handlers : &list_xattr_handlers;
for (i = 0; i < vi->xattr_shared_count; ++i) {
xsid = vi->xattr_shared_xattrs[i];
- it->blkaddr = erofs_xattr_blkaddr(sb, xsid);
- it->ofs = erofs_xattr_blkoff(sb, xsid);
+ it->blkaddr = EROFS_SB(sb)->xattr_blkaddr +
+ erofs_blknr(sb, xsid * sizeof(__u32));
+ it->ofs = erofs_blkoff(sb, xsid * sizeof(__u32));
it->kaddr = erofs_read_metabuf(&it->buf, sb, it->blkaddr, EROFS_KMAP);
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 ((it->getxattr && ret != -ENOATTR) || (!it->getxattr && ret))
break;
}
- return ret ? ret : it->buffer_ofs;
+ return ret;
+}
+
+static 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;
+ if (strlen(name) > EROFS_NAME_LEN)
+ return -ERANGE;
+
+ ret = erofs_init_inode_xattrs(inode);
+ if (ret)
+ return ret;
+
+ it = (struct erofs_xattr_iter) {
+ .buf = __EROFS_BUF_INITIALIZER,
+ .sb = inode->i_sb,
+ .inode = inode,
+ .name = QSTR_INIT(name, strlen(name)),
+ .index = index,
+ .buffer = buffer,
+ .buffer_size = buffer_size,
+ .getxattr = true,
+ };
+
+ ret = erofs_iter_inline_xattr(&it);
+ if (ret == -ENOATTR)
+ ret = erofs_iter_shared_xattr(&it);
+ erofs_put_metabuf(&it.buf);
+ return ret ? ret : it.buffer_ofs;
}
ssize_t erofs_listxattr(struct dentry *dentry,
@@ -578,15 +526,19 @@ ssize_t erofs_listxattr(struct dentry *dentry,
.buf = __EROFS_BUF_INITIALIZER,
.sb = dentry->d_sb,
.dentry = dentry,
+ .inode = d_inode(dentry),
.buffer = buffer,
.buffer_size = buffer_size,
+ .getxattr = false,
};
- ret = inline_listxattr(&it);
- if (ret >= 0 || ret == -ENOATTR)
- ret = shared_listxattr(&it);
+ ret = erofs_iter_inline_xattr(&it);
+ if (!ret || ret == -ENOATTR)
+ ret = erofs_iter_shared_xattr(&it);
+ if (ret == -ENOATTR)
+ ret = 0;
erofs_put_metabuf(&it.buf);
- return ret;
+ return ret ? ret : it.buffer_ofs;
}
#ifdef CONFIG_EROFS_FS_POSIX_ACL
--
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 for listxattr and getxattr.
Signed-off-by: Jingbo Xu <[email protected]>
---
fs/erofs/xattr.c | 347 ++++++++++++++++++++++-------------------------
1 file changed, 159 insertions(+), 188 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 196b2eb59e29..eac5b7b69691 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -24,6 +24,7 @@ struct erofs_xattr_iter {
struct qstr name;
struct dentry *dentry;
struct inode *inode;
+ unsigned int remaining; /* size of inline xattrs to be iterated */
bool getxattr;
};
@@ -144,153 +145,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
- * `ofs' 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;
- unsigned int value_sz, processed, slice;
- int err;
-
- /* 0. fixup blkaddr, ofs, ipage */
- err = erofs_xattr_iter_fixup(it);
- if (err)
- return err;
-
- /*
- * 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);
- 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->ofs += 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;
- goto out;
- }
-
- /* 2. handle xattr name (ofs will finally be at the end of name) */
- processed = 0;
-
- while (processed < entry.e_name_len) {
- err = erofs_xattr_iter_fixup_aligned(it);
- if (err)
- goto out;
-
- slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
- entry.e_name_len - processed);
-
- /* handle name */
- err = op->name(it, processed, it->kaddr + it->ofs, slice);
- if (err) {
- it->ofs += entry.e_name_len - processed + value_sz;
- goto out;
- }
-
- it->ofs += slice;
- processed += slice;
- }
-
- /* 3. handle xattr value */
- processed = 0;
-
- if (op->alloc_buffer) {
- err = op->alloc_buffer(it, value_sz);
- if (err) {
- it->ofs += value_sz;
- goto out;
- }
- }
-
- while (processed < value_sz) {
- err = erofs_xattr_iter_fixup_aligned(it);
- if (err)
- goto out;
-
- slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
- value_sz - processed);
- op->value(it, processed, it->kaddr + it->ofs, slice);
- it->ofs += slice;
- processed += slice;
- }
-
-out:
- /* xattrs should be 4-byte aligned (on-disk constraint) */
- it->ofs = EROFS_XATTR_ALIGN(it->ofs);
- return err < 0 ? err : 0;
-}
-
-static int xattr_entrymatch(struct erofs_xattr_iter *it,
- struct erofs_xattr_entry *entry)
-{
- return (it->index != entry->e_name_index ||
- it->name.len != entry->e_name_len) ? -ENOATTR : 0;
-}
-
-static int xattr_namematch(struct erofs_xattr_iter *it,
- unsigned int processed, char *buf, unsigned int len)
-{
- return memcmp(buf, it->name.name + processed, len) ? -ENOATTR : 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);
@@ -367,62 +221,178 @@ static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
xattr_handler_map[idx] : NULL;
}
-static int xattr_entrylist(struct erofs_xattr_iter *it,
- struct erofs_xattr_entry *entry)
+/*
+ * 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
+ */
+typedef int (*erofs_xattr_body_handler)(struct erofs_xattr_iter *it,
+ unsigned int processed,
+ char *buf, unsigned int len);
+
+static int erofs_xattr_namematch(struct erofs_xattr_iter *it,
+ unsigned int processed, char *buf, unsigned int len)
+{
+ return memcmp(buf, it->name.name + processed, len) ? -ENOATTR : 0;
+}
+
+static int erofs_xattr_copy(struct erofs_xattr_iter *it,
+ unsigned int unused, char *buf, unsigned int len)
+{
+ memcpy(it->buffer + it->buffer_ofs, buf, len);
+ it->buffer_ofs += len;
+ return 0;
+}
+
+static int erofs_xattr_body(struct erofs_xattr_iter *it, unsigned int len,
+ erofs_xattr_body_handler handler)
+{
+ unsigned int slice, processed = 0;
+
+ while (processed < len) {
+ int err = erofs_xattr_iter_fixup_aligned(it);
+ if (err)
+ return err;
+
+ slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
+ len - processed);
+ err = handler(it, processed, it->kaddr + it->ofs, slice);
+ if (err) {
+ it->ofs += len - processed;
+ return err;
+ }
+
+ it->ofs += slice;
+ processed += slice;
+ }
+ return 0;
+}
+
+static int erofs_xattr_check_entry(struct erofs_xattr_iter *it)
{
- unsigned int prefix_len;
+ int err;
+
+ err = erofs_xattr_iter_fixup(it);
+ if (err)
+ return err;
+
+ if (it->remaining) {
+ struct erofs_xattr_entry *entry = it->kaddr + it->ofs;
+ unsigned int entry_sz = erofs_xattr_entry_size(entry);
+ /* xattr on-disk corruption: xattr entry beyond xattr_isize */
+ if (it->remaining < entry_sz) {
+ DBG_BUGON(1);
+ return -EFSCORRUPTED;
+ }
+ it->remaining -= entry_sz;
+ }
+ return 0;
+}
+
+/*
+ * Regardless of success or failure, erofs_[get|list]xattr_foreach() will end up
+ * with `ofs' pointing to the next xattr item rather than an arbitrary position.
+ */
+static int erofs_listxattr_foreach(struct erofs_xattr_iter *it)
+{
+ struct erofs_xattr_entry entry;
+ const struct xattr_handler *h;
const char *prefix;
+ unsigned int value_sz, prefix_len, name_sz;
+ int err;
- const struct xattr_handler *h =
- erofs_xattr_handler(entry->e_name_index);
+ err = erofs_xattr_check_entry(it);
+ if (err)
+ return err;
- if (!h || (h->list && !h->list(it->dentry)))
- return 1;
+ /* 1. handle xattr entry */
+ entry = *(struct erofs_xattr_entry *)(it->kaddr + it->ofs);
+ it->ofs += sizeof(struct erofs_xattr_entry);
+ value_sz = le16_to_cpu(entry.e_value_size);
+
+ h = erofs_xattr_handler(entry.e_name_index);
+ if (!h || (h->list && !h->list(it->dentry))) {
+ it->ofs += entry.e_name_len + value_sz;
+ goto out;
+ }
prefix = xattr_prefix(h);
prefix_len = strlen(prefix);
+ name_sz = prefix_len + entry.e_name_len + 1;
if (!it->buffer) {
- it->buffer_ofs += prefix_len + entry->e_name_len + 1;
- return 1;
+ it->buffer_ofs += name_sz;
+ it->ofs += entry.e_name_len + value_sz;
+ goto out;
}
-
- if (it->buffer_ofs + prefix_len
- + entry->e_name_len + 1 > it->buffer_size)
+ if (it->buffer_ofs + name_sz > it->buffer_size)
return -ERANGE;
memcpy(it->buffer + it->buffer_ofs, prefix, prefix_len);
it->buffer_ofs += prefix_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 can't be 1 or ENOATTR) */
+ err = erofs_xattr_body(it, entry.e_name_len, erofs_xattr_copy);
+ if (err)
+ return err;
+
+ it->buffer[it->buffer_ofs++] = '\0';
+ it->ofs += value_sz;
+out:
+ it->ofs = EROFS_XATTR_ALIGN(it->ofs);
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 erofs_xattr_entry entry;
+ unsigned int value_sz;
+ int err;
-static const struct xattr_iter_handlers list_xattr_handlers = {
- .entry = xattr_entrylist,
- .name = xattr_namelist,
- .alloc_buffer = xattr_skipvalue,
- .value = NULL
-};
+ err = erofs_xattr_check_entry(it);
+ if (err)
+ return err;
+
+ /* 1. handle xattr entry */
+ entry = *(struct erofs_xattr_entry *)(it->kaddr + it->ofs);
+ it->ofs += sizeof(struct erofs_xattr_entry);
+ value_sz = le16_to_cpu(entry.e_value_size);
+
+ if (it->index != entry.e_name_index || it->name.len != entry.e_name_len) {
+ it->ofs += entry.e_name_len + value_sz;
+ err = -ENOATTR;
+ goto out;
+ }
+
+ /* 2. handle xattr name */
+ err = erofs_xattr_body(it, entry.e_name_len, erofs_xattr_namematch);
+ if (err) {
+ it->ofs += value_sz;
+ goto out;
+ }
+
+ /* 3. handle xattr value */
+ if (!it->buffer) {
+ it->buffer_ofs = value_sz;
+ it->ofs += value_sz;
+ goto out; /* err == 0 */
+ }
+ if (it->buffer_size < value_sz)
+ return -ERANGE;
+
+ /* no need updating ofs on error (err can't be 1 or ENOATTR) */
+ err = erofs_xattr_body(it, value_sz, erofs_xattr_copy);
+out:
+ it->ofs = EROFS_XATTR_ALIGN(it->ofs);
+ return err < 0 ? err : 0;
+}
static int erofs_iter_inline_xattr(struct erofs_xattr_iter *it)
{
struct erofs_inode *const vi = EROFS_I(it->inode);
- const struct xattr_iter_handlers *op;
- unsigned int xattr_header_sz, remaining;
+ unsigned int xattr_header_sz;
erofs_off_t pos;
int ret;
@@ -440,11 +410,12 @@ static int erofs_iter_inline_xattr(struct erofs_xattr_iter *it)
if (IS_ERR(it->kaddr))
return PTR_ERR(it->kaddr);
- remaining = vi->xattr_isize - xattr_header_sz;
- op = it->getxattr ? &find_xattr_handlers : &list_xattr_handlers;
-
- while (remaining) {
- ret = xattr_foreach(it, op, &remaining);
+ it->remaining = vi->xattr_isize - xattr_header_sz;
+ while (it->remaining) {
+ if (it->getxattr)
+ ret = erofs_getxattr_foreach(it);
+ else
+ ret = erofs_listxattr_foreach(it);
if ((it->getxattr && ret != -ENOATTR) || (!it->getxattr && ret))
break;
}
@@ -455,12 +426,9 @@ static int erofs_iter_shared_xattr(struct erofs_xattr_iter *it)
{
struct erofs_inode *const vi = EROFS_I(it->inode);
struct super_block *const sb = it->sb;
- const struct xattr_iter_handlers *op;
unsigned int i, xsid;
int ret = -ENOATTR;
- op = it->getxattr ? &find_xattr_handlers : &list_xattr_handlers;
-
for (i = 0; i < vi->xattr_shared_count; ++i) {
xsid = vi->xattr_shared_xattrs[i];
it->blkaddr = EROFS_SB(sb)->xattr_blkaddr +
@@ -470,7 +438,10 @@ static int erofs_iter_shared_xattr(struct erofs_xattr_iter *it)
if (IS_ERR(it->kaddr))
return PTR_ERR(it->kaddr);
- ret = xattr_foreach(it, op, NULL);
+ if (it->getxattr)
+ ret = erofs_getxattr_foreach(it);
+ else
+ ret = erofs_listxattr_foreach(it);
if ((it->getxattr && ret != -ENOATTR) || (!it->getxattr && ret))
break;
}
--
2.19.1.6.gb485710b
Rename init_inode_xattrs() to erofs_init_inode_xattrs() without logic
change.
Signed-off-by: Jingbo Xu <[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 760ec864a39c..ab4517e5ec84 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -29,7 +29,7 @@ struct xattr_iter {
unsigned int ofs;
};
-static int init_inode_xattrs(struct inode *inode)
+static int erofs_init_inode_xattrs(struct inode *inode)
{
struct erofs_inode *const vi = EROFS_I(inode);
struct xattr_iter it;
@@ -404,7 +404,7 @@ static int erofs_getxattr(struct inode *inode, int index, const char *name,
if (!name)
return -EINVAL;
- ret = init_inode_xattrs(inode);
+ ret = erofs_init_inode_xattrs(inode);
if (ret)
return ret;
@@ -619,7 +619,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
int ret;
struct listxattr_iter it;
- ret = init_inode_xattrs(d_inode(dentry));
+ ret = erofs_init_inode_xattrs(d_inode(dentry));
if (ret == -ENOATTR)
return 0;
if (ret)
--
2.19.1.6.gb485710b
Introduce erofs_xattr_iter_fixup_aligned() helper where
it.ofs <= EROFS_BLKSIZ is mandatory.
Signed-off-by: Jingbo Xu <[email protected]>
---
fs/erofs/xattr.c | 76 ++++++++++++++++++++----------------------------
1 file changed, 32 insertions(+), 44 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index b83331a694f3..bdd9393145ce 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -29,6 +29,25 @@ struct xattr_iter {
unsigned int ofs;
};
+static inline int erofs_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_read_metabuf(&it->buf, it->sb, 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 inline int erofs_xattr_iter_fixup_aligned(struct xattr_iter *it)
+{
+ DBG_BUGON(it->ofs > it->sb->s_blocksize);
+ return erofs_xattr_iter_fixup(it);
+}
+
static int erofs_init_inode_xattrs(struct inode *inode)
{
struct erofs_inode *const vi = EROFS_I(inode);
@@ -80,6 +99,7 @@ static int erofs_init_inode_xattrs(struct inode *inode)
goto out_unlock;
}
+ it.sb = sb;
it.buf = __EROFS_BUF_INITIALIZER;
it.blkaddr = erofs_blknr(sb, erofs_iloc(inode) + vi->inode_isize);
it.ofs = erofs_blkoff(sb, erofs_iloc(inode) + vi->inode_isize);
@@ -105,19 +125,11 @@ static int erofs_init_inode_xattrs(struct inode *inode)
it.ofs += 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_read_metabuf(&it.buf, sb, ++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;
+ ret = erofs_xattr_iter_fixup_aligned(&it);
+ if (ret) {
+ kfree(vi->xattr_shared_xattrs);
+ vi->xattr_shared_xattrs = NULL;
+ goto out_unlock;
}
vi->xattr_shared_xattrs[i] =
le32_to_cpu(*(__le32 *)(it.kaddr + it.ofs));
@@ -150,20 +162,6 @@ 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_read_metabuf(&it->buf, it->sb, 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)
{
@@ -201,7 +199,7 @@ static int xattr_foreach(struct xattr_iter *it,
int err;
/* 0. fixup blkaddr, ofs, ipage */
- err = xattr_iter_fixup(it);
+ err = erofs_xattr_iter_fixup(it);
if (err)
return err;
@@ -236,14 +234,9 @@ static int xattr_foreach(struct xattr_iter *it,
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;
- }
+ err = erofs_xattr_iter_fixup_aligned(it);
+ if (err)
+ goto out;
slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
entry.e_name_len - processed);
@@ -271,14 +264,9 @@ static int xattr_foreach(struct xattr_iter *it,
}
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;
- }
+ err = erofs_xattr_iter_fixup_aligned(it);
+ if (err)
+ goto out;
slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
value_sz - processed);
--
2.19.1.6.gb485710b
On 2023/3/23 08:09, Jingbo Xu wrote:
> There are several xattr helpers not used outside xattr.c, thus move them
> into xattr.c as a cleanup.
>
> inlinexattr_header_size() has only one caller, and thus make it inlined
> into the caller directly.
>
> Signed-off-by: Jingbo Xu <[email protected]>
> ---
> fs/erofs/xattr.c | 73 +++++++++++++++++++++++++++++++++---------------
> fs/erofs/xattr.h | 56 -------------------------------------
> 2 files changed, 51 insertions(+), 78 deletions(-)
>
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index 459caa3cd65d..760ec864a39c 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -7,6 +7,19 @@
> #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;
> @@ -157,7 +170,8 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
> struct erofs_inode *const vi = EROFS_I(inode);
> unsigned int xattr_header_sz, inline_xattr_ofs;
>
> - xattr_header_sz = inlinexattr_header_size(inode);
> + 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;
> @@ -351,20 +365,18 @@ static int inline_getxattr(struct inode *inode, struct getxattr_iter *it)
> static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
> {
> struct erofs_inode *const vi = EROFS_I(inode);
> - struct super_block *const sb = inode->i_sb;
> - unsigned int i;
> + struct super_block *const sb = it->it.sb;
> + unsigned int i, xsid;
> int ret = -ENOATTR;
>
> for (i = 0; i < vi->xattr_shared_count; ++i) {
> - erofs_blk_t blkaddr =
> - xattrblock_addr(sb, vi->xattr_shared_xattrs[i]);
> -
> - it->it.ofs = xattrblock_offset(sb, vi->xattr_shared_xattrs[i]);
> - it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
> - EROFS_KMAP);
> + 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_read_metabuf(&it->it.buf, sb,
> + it->it.blkaddr, EROFS_KMAP);
> if (IS_ERR(it->it.kaddr))
> return PTR_ERR(it->it.kaddr);
> - it->it.blkaddr = blkaddr;
>
> ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
> if (ret != -ENOATTR)
> @@ -383,9 +395,8 @@ 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)
> +static int erofs_getxattr(struct inode *inode, int index, const char *name,
> + void *buffer, size_t buffer_size)
> {
> int ret;
> struct getxattr_iter it;
> @@ -473,6 +484,26 @@ const struct xattr_handler *erofs_xattr_handlers[] = {
> NULL,
> };
>
> +static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
> +{
> + static const struct xattr_handler *xattr_handler_map[] = {
> + [EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
> +#ifdef CONFIG_EROFS_FS_POSIX_ACL
> + [EROFS_XATTR_INDEX_POSIX_ACL_ACCESS] =
> + &posix_acl_access_xattr_handler,
> + [EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT] =
> + &posix_acl_default_xattr_handler,
> +#endif
> + [EROFS_XATTR_INDEX_TRUSTED] = &erofs_xattr_trusted_handler,
> +#ifdef CONFIG_EROFS_FS_SECURITY
> + [EROFS_XATTR_INDEX_SECURITY] = &erofs_xattr_security_handler,
> +#endif
> + };
> +
> + return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
> + xattr_handler_map[idx] : NULL;
> +}
> +
> struct listxattr_iter {
> struct xattr_iter it;
>
> @@ -562,20 +593,18 @@ 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 = inode->i_sb;
> - unsigned int i;
> + struct super_block *const sb = it->it.sb;
> + unsigned int i, xsid;
> int ret = 0;
>
> for (i = 0; i < vi->xattr_shared_count; ++i) {
> - erofs_blk_t blkaddr =
> - xattrblock_addr(sb, vi->xattr_shared_xattrs[i]);
> -
> - it->it.ofs = xattrblock_offset(sb, vi->xattr_shared_xattrs[i]);
> - it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
> - EROFS_KMAP);
> + 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_read_metabuf(&it->it.buf, sb,
> + it->it.blkaddr, EROFS_KMAP);
> if (IS_ERR(it->it.kaddr))
> return PTR_ERR(it->it.kaddr);
> - it->it.blkaddr = blkaddr;
>
> ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
> if (ret)
> diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
> index f7a21aaa9755..cc9ef97dbf8e 100644
> --- a/fs/erofs/xattr.h
> +++ b/fs/erofs/xattr.h
> @@ -13,66 +13,10 @@
> /* Attribute not found */
> #define ENOATTR ENODATA
>
> -static inline unsigned int inlinexattr_header_size(struct inode *inode)
> -{
> - return sizeof(struct erofs_xattr_ibody_header) +
> - sizeof(u32) * EROFS_I(inode)->xattr_shared_count;
> -}
> -
> -static inline erofs_blk_t xattrblock_addr(struct super_block *sb,
> - unsigned int xattr_id)
> -{
> #ifdef CONFIG_EROFS_FS_XATTR
> - return EROFS_SB(sb)->xattr_blkaddr +
> - xattr_id * sizeof(__u32) / sb->s_blocksize;
> -#else
> - return 0;
> -#endif
> -}
> -
> -static inline unsigned int xattrblock_offset(struct super_block *sb,
> - unsigned int xattr_id)
> -{
> - return (xattr_id * sizeof(__u32)) % sb->s_blocksize;
> -}
> -
> -#ifdef CONFIG_EROFS_FS_XATTR
> -extern const struct xattr_handler erofs_xattr_user_handler;
> -extern const struct xattr_handler erofs_xattr_trusted_handler;
> -extern const struct xattr_handler erofs_xattr_security_handler;
> -
> -static inline const struct xattr_handler *erofs_xattr_handler(unsigned int idx)
> -{
> - static const struct xattr_handler *xattr_handler_map[] = {
> - [EROFS_XATTR_INDEX_USER] = &erofs_xattr_user_handler,
> -#ifdef CONFIG_EROFS_FS_POSIX_ACL
> - [EROFS_XATTR_INDEX_POSIX_ACL_ACCESS] =
> - &posix_acl_access_xattr_handler,
> - [EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT] =
> - &posix_acl_default_xattr_handler,
> -#endif
> - [EROFS_XATTR_INDEX_TRUSTED] = &erofs_xattr_trusted_handler,
> -#ifdef CONFIG_EROFS_FS_SECURITY
> - [EROFS_XATTR_INDEX_SECURITY] = &erofs_xattr_security_handler,
> -#endif
> - };
> -
> - return idx && idx < ARRAY_SIZE(xattr_handler_map) ?
> - xattr_handler_map[idx] : NULL;
> -}
Could we drop this? since Christian Brauner also touchs this stuff for
the next cycle, see:
https://lore.kernel.org/r/20230125-fs-acl-remove-generic-xattr-handlers-v3-7-f760cc58967d@kernel.org/
I'd like to wait it for the following cycle after the next cycle to
avoid such cross-tree conflicts since it's just a cleanup.
> -
> extern const struct xattr_handler *erofs_xattr_handlers[];
> -
> -int erofs_getxattr(struct inode *, int, const char *, void *, size_t);
Could we leave it for now? I'm not sure laterly if erofs will
read xattrs itself.
Otherwise it looks good to me.
Thanks,
Gao Xiang
> ssize_t erofs_listxattr(struct dentry *, char *, size_t);
> #else
> -static inline int erofs_getxattr(struct inode *inode, int index,
> - const char *name, void *buffer,
> - size_t buffer_size)
> -{
> - return -EOPNOTSUPP;
> -}
> -
> #define erofs_listxattr (NULL)
> #define erofs_xattr_handlers (NULL)
> #endif /* !CONFIG_EROFS_FS_XATTR */
On 2023/3/23 08:09, Jingbo Xu wrote:
> Rename init_inode_xattrs() to erofs_init_inode_xattrs() without logic
> change.
>
> Signed-off-by: Jingbo Xu <[email protected]>
Reviewed-by: Gao Xiang <[email protected]>
Thanks,
Gao Xiang
> ---
> 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 760ec864a39c..ab4517e5ec84 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -29,7 +29,7 @@ struct xattr_iter {
> unsigned int ofs;
> };
>
> -static int init_inode_xattrs(struct inode *inode)
> +static int erofs_init_inode_xattrs(struct inode *inode)
> {
> struct erofs_inode *const vi = EROFS_I(inode);
> struct xattr_iter it;
> @@ -404,7 +404,7 @@ static int erofs_getxattr(struct inode *inode, int index, const char *name,
> if (!name)
> return -EINVAL;
>
> - ret = init_inode_xattrs(inode);
> + ret = erofs_init_inode_xattrs(inode);
> if (ret)
> return ret;
>
> @@ -619,7 +619,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
> int ret;
> struct listxattr_iter it;
>
> - ret = init_inode_xattrs(d_inode(dentry));
> + ret = erofs_init_inode_xattrs(d_inode(dentry));
> if (ret == -ENOATTR)
> return 0;
> if (ret)
On 2023/3/23 08:09, Jingbo Xu wrote:
> erofs_xattr_generic_get() won't be called from xattr handlers other than
> user/trusted/security xattr handler, and thus there's no need of extra
> checking.
>
> Signed-off-by: Jingbo Xu <[email protected]>
Reviewed-by: Gao Xiang <[email protected]>
Thanks,
Gao Xiang
> ---
> fs/erofs/xattr.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index ab4517e5ec84..b83331a694f3 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -431,20 +431,9 @@ 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)
> {
> - struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
> -
> - switch (handler->flags) {
> - case EROFS_XATTR_INDEX_USER:
> - if (!test_opt(&sbi->opt, XATTR_USER))
> - return -EOPNOTSUPP;
> - break;
> - case EROFS_XATTR_INDEX_TRUSTED:
> - break;
> - case EROFS_XATTR_INDEX_SECURITY:
> - break;
> - default:
> - return -EINVAL;
> - }
> + if (handler->flags == EROFS_XATTR_INDEX_USER &&
> + !test_opt(&EROFS_I_SB(inode)->opt, XATTR_USER))
> + return -EOPNOTSUPP;
>
> return erofs_getxattr(inode, handler->flags, name, buffer, size);
> }