| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8e69dcec-b038-4941-1157-8d80fd3f1afa@linux.alibaba.com>
Date: Thu, 1 Jun 2023 11:37:00 +0800
From: Gao Xiang <hsiangkao@...ux.alibaba.com>
To: Jingbo Xu <jefflexu@...ux.alibaba.com>, xiang@...nel.org,
chao@...nel.org, huyue2@...lpad.com, linux-erofs@...ts.ozlabs.org
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 6/6] erofs: use separate xattr parsers for
listxattr/getxattr
On 2023/6/1 10:43, 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 for listxattr and getxattr.
>
> Signed-off-by: Jingbo Xu <jefflexu@...ux.alibaba.com>
> ---
...
> +static int erofs_xattr_body(struct erofs_xattr_iter *it,
> + unsigned int len, bool copy)
It might be better to call it as "erofs_xattr_handle_string".
> {
> - unsigned int base_index = entry->e_name_index;
> - unsigned int prefix_len, infix_len = 0;
> + unsigned int slice, processed = 0;
> + void *buf;
> + int err;
> +
> + while (processed < len) {
> + err = erofs_xattr_iter_fixup(it, true);
> + if (err)
> + return err;
> +
> + buf = it->kaddr + it->ofs;
> + slice = min_t(unsigned int, it->sb->s_blocksize - it->ofs,
> + len - processed);
> + if (copy) {
> + memcpy(it->buffer + it->buffer_ofs, buf, slice);
> + it->buffer_ofs += slice;
> + } else if (memcmp(it->name.name + it->infix_len + processed,
> + buf, slice)) {
> + return -ENOATTR;
> + }
> + it->ofs += slice;
> + processed += slice;
> + }
> + return 0;
> +}
> +
> +/*
> + * Wen returning 0 or ENOATTR, erofs_[list|get]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;
> + unsigned int base_index, 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 + it->ofs);
> + it->ofs += 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;
> + goto out;
> infix = pf->prefix->infix;
> infix_len = pf->infix_len;
> base_index = pf->prefix->base_index;
> @@ -392,53 +252,99 @@ static int xattr_entrylist(struct erofs_xattr_iter *it,
>
> prefix = erofs_xattr_prefix(base_index, it->dentry);
> if (!prefix)
> - return 1;
> + goto out;
> prefix_len = strlen(prefix);
>
> if (!it->buffer) {
> - it->buffer_ofs += prefix_len + infix_len +
> - entry->e_name_len + 1;
> - return 1;
> + it->buffer_ofs += prefix_len + infix_len + entry.e_name_len + 1;
> + goto out;
> }
>
> if (it->buffer_ofs + prefix_len + infix_len +
> - + entry->e_name_len + 1 > it->buffer_size)
> + entry.e_name_len + 1 > 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 can't be ENOATTR) */
> + err = erofs_xattr_body(it, entry.e_name_len, true);
> + if (err)
> + return err;
> +
> + it->buffer[it->buffer_ofs++] = '\0';
> + it->ofs += le16_to_cpu(entry.e_value_size);
> + it->ofs = EROFS_XATTR_ALIGN(it->ofs);
> + return 0;
> +out:
> + it->ofs = it->next_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
> -};
> + /* 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);
> +
> + err = -ENOATTR;
> + /* 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(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)
> + goto out;
> +
> + if (it->index != pf->prefix->base_index ||
> + it->name.len != entry.e_name_len + pf->infix_len)
> + goto out;
> +
> + if (memcmp(it->name.name, pf->prefix->infix, pf->infix_len))
> + goto out;
> +
> + it->infix_len = pf->infix_len;
> + } else {
> + if (it->index != entry.e_name_index ||
> + it->name.len != entry.e_name_len)
> + goto out;
please use "} else if {" here
> + it->infix_len = 0;
> + }
> +
Thanks,
Gao Xiang
Powered by blists - more mailing lists