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 | 79 +++++++++++++++++++++---------------------------
1 file changed, 35 insertions(+), 44 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index bbfe7ce170d2..b79be2a556ba 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -29,6 +29,28 @@ 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)
+{
+ if (it->ofs > it->sb->s_blocksize) {
+ DBG_BUGON(1);
+ return -EFSCORRUPTED;
+ }
+ 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 +102,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 +128,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 +165,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 +202,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 +237,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 +267,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
Hi,
On 2023/5/18 10:45, Jingbo Xu wrote:
> 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 | 79 +++++++++++++++++++++---------------------------
> 1 file changed, 35 insertions(+), 44 deletions(-)
>
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index bbfe7ce170d2..b79be2a556ba 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -29,6 +29,28 @@ 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);
could we use a new buf interface to init_metabuf at once?
> + 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)
Since we're doing cleanup, this name sounds confusing to me
since here the meaning is actually "we don't allow pos >
blksize", IOWs, any pos <= blksize is allowed here, so
'aligned' is not accurate.
Could we think out a better one?
Thanks,
Gao Xiang
On 5/29/23 3:41 PM, Gao Xiang wrote:
> Hi,
>
> On 2023/5/18 10:45, Jingbo Xu wrote:
>> 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 | 79 +++++++++++++++++++++---------------------------
>> 1 file changed, 35 insertions(+), 44 deletions(-)
>>
>> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
>> index bbfe7ce170d2..b79be2a556ba 100644
>> --- a/fs/erofs/xattr.c
>> +++ b/fs/erofs/xattr.c
>> @@ -29,6 +29,28 @@ 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);
>
> could we use a new buf interface to init_metabuf at once?
As discussed offline, I think the following unified API is preferred:
```
int erofs_xattr_iter_fixup(struct xattr_iter *it, bool nospan)
{
if (it->ofs < it->sb->s_blocksize)
return 0;
if (nospan && it->ofs != it->sb->s_blocksize) {
DBG_BUGON(1);
return -EFSCORRUPTED;
}
...
}
```
--
Thanks,
Jingbo
On 2023/5/30 11:42, Jingbo Xu wrote:
>
>
> On 5/29/23 3:41 PM, Gao Xiang wrote:
>> Hi,
>>
>> On 2023/5/18 10:45, Jingbo Xu wrote:
>>> 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 | 79 +++++++++++++++++++++---------------------------
>>> 1 file changed, 35 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
>>> index bbfe7ce170d2..b79be2a556ba 100644
>>> --- a/fs/erofs/xattr.c
>>> +++ b/fs/erofs/xattr.c
>>> @@ -29,6 +29,28 @@ 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);
>>
>> could we use a new buf interface to init_metabuf at once?
>
> As discussed offline, I think the following unified API is preferred:
>
> ```
> int erofs_xattr_iter_fixup(struct xattr_iter *it, bool nospan)
>
> {
> if (it->ofs < it->sb->s_blocksize)
> return 0;
>
> if (nospan && it->ofs != it->sb->s_blocksize) {
> DBG_BUGON(1);
> return -EFSCORRUPTED;
> }
>
> ...
> }
> ```
Yeah, could you send the next version for this?
Thanks,
Gao Xiang
>