Re: [PATCH v2 5/6] exfat: introduce exfat_chain_advance helper

[Chi Zhiling <chizhiling@163.com>]

On 4/3/26 11:33 AM, Yuezhang.Mo@sony.com wrote:
>> On 4/3/26 10:34 AM, Yuezhang.Mo@sony.com wrote:
>>>> From: Chi Zhiling <chizhiling@kylinos.cn>
>>>>
>>>> Introduce exfat_chain_advance() to walk a exfat_chain structure by a
>>>> given step, updating both ->dir and ->size fields atomically. This
>>>> helper handles both ALLOC_NO_FAT_CHAIN and ALLOC_FAT_CHAIN modes with
>>>> proper boundary checking.
>>>>
>>>> Signed-off-by: Chi Zhiling <chizhiling@kylinos.cn>
>>>> Tested-by: syzbot@syzkaller.appspotmail.com
>>>>
>>>> ---
>>>> fs/exfat/exfat_fs.h | 17 +++++++++++++++++
>>>>    1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
>>>> index 530459ab9acc..1035d20ba563 100644
>>>> --- a/fs/exfat/exfat_fs.h
>>>> +++ b/fs/exfat/exfat_fs.h
>>>> @@ -552,6 +552,23 @@ int exfat_read_volume_label(struct super_block *sb,
>>>> int exfat_write_volume_label(struct super_block *sb,
>>>>                                struct exfat_uni_name *label);
>>>>
>>>> +static inline int exfat_chain_advance(struct super_block *sb,
>>>> +               struct exfat_chain *chain, unsigned int step)
>>>> +{
>>>> +       if (chain->size >= step)
>>>> +               chain->size -= step;
>>>
>>> It would be better to not change *chain if return an error.
>>
>> Okay,
>>
>> v3:
>>
>> static inline int exfat_chain_advance(struct super_block *sb,
>>                   struct exfat_chain *chain, unsigned int step)
>> {
>>           if (chain->size < step)
>>                   return -EIO;
>>
>>           chain->size -= step;
>>
>>           if (exfat_fat_walk(sb, &chain->dir, step, chain->flags))
>>                   return -EIO;
>>
>>           if (chain->size == 0 && chain->flags == ALLOC_NO_FAT_CHAIN)
>>                   chain->dir = EXFAT_EOF_CLUSTER;
>>
>>           return 0;
>> }
>>
> 
> chain->dir should also not be changed.
> 
> static inline int exfat_chain_advance(struct super_block *sb,
>                   struct exfat_chain *chain, unsigned int step)
> {
>           unsigned int clu = chain->dir;
> 
>           if (chain->size < step)
>                   return -EIO;
> 
>           if (exfat_fat_walk(sb, &clu, step, chain->flags))
>                   return -EIO;
> 
>           chain->size -= step;
> 
>           if (chain->size == 0 && chain->flags == ALLOC_NO_FAT_CHAIN)
>                   chain->dir = EXFAT_EOF_CLUSTER;
>           else
>                   chain->dir = clu;
> 
>           return 0;
> }

Alright, I got it, this is great.

I’ll update it in v3.


Thanks,

> 
>>
>> Thanks,
>>>
>>>> +       else
>>>> +               return -EIO;
>>>> +
>>>> +       if (exfat_fat_walk(sb, &chain->dir, step, chain->flags))
>>>> +               return -EIO;
>>>> +
>>>> +       if (chain->size == 0 && chain->flags == ALLOC_NO_FAT_CHAIN)
>>>> +               chain->dir = EXFAT_EOF_CLUSTER;
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>>    /* inode.c */
>>>>    extern const struct inode_operations exfat_file_inode_operations;
>>>>    void exfat_sync_inode(struct inode *inode);
>>>> --
>>>> 2.43.0
>>>
>>>