2022-02-15 09:55:20

by Baokun Li

[permalink] [raw]
Subject: [PATCH -next] ubifs: rename_whiteout: correct old_dir size computing

When renaming the whiteout file, the old whiteout file is not deleted.
Therefore, we add the old dentry size to the old dir like XFS.
Otherwise, an error may be reported due to `fscki->calc_sz != fscki->size`
in check_indes.

Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
Reported-by: Zhihao Cheng <[email protected]>
Signed-off-by: Baokun Li <[email protected]>
---
fs/ubifs/dir.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index ae082a0be2a3..86151889548e 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1402,6 +1402,9 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
iput(whiteout);
goto out_release;
}
+
+ /* Add the old_dentry size to the old_dir size. */
+ old_sz -= CALC_DENT_SIZE(fname_len(&old_nm));
}

lock_4_inodes(old_dir, new_dir, new_inode, whiteout);
--
2.31.1


2022-03-10 14:35:43

by Baokun Li

[permalink] [raw]
Subject: Re: [PATCH -next] ubifs: rename_whiteout: correct old_dir size computing

A gentle ping, sorry for the noise.


在 2022/2/15 12:07, Baokun Li 写道:
> When renaming the whiteout file, the old whiteout file is not deleted.
> Therefore, we add the old dentry size to the old dir like XFS.
> Otherwise, an error may be reported due to `fscki->calc_sz != fscki->size`
> in check_indes.
>
> Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
> Reported-by: Zhihao Cheng <[email protected]>
> Signed-off-by: Baokun Li <[email protected]>
> ---
> fs/ubifs/dir.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index ae082a0be2a3..86151889548e 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -1402,6 +1402,9 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
> iput(whiteout);
> goto out_release;
> }
> +
> + /* Add the old_dentry size to the old_dir size. */
> + old_sz -= CALC_DENT_SIZE(fname_len(&old_nm));
> }
>
> lock_4_inodes(old_dir, new_dir, new_inode, whiteout);

--
With Best Regards,
Baokun Li

2022-03-17 05:28:17

by Baokun Li

[permalink] [raw]
Subject: Re: [PATCH -next] ubifs: rename_whiteout: correct old_dir size computing

在 2022/3/17 6:00, Richard Weinberger 写道:
> ----- Ursprüngliche Mail -----
>> Von: "libaokun" <[email protected]>
>> An: "richard" <[email protected]>, "linux-mtd" <[email protected]>, "linux-kernel"
>> <[email protected]>
>> CC: "yukuai3" <[email protected]>, "chengzhihao1" <[email protected]>, "libaokun" <[email protected]>
>> Gesendet: Donnerstag, 10. März 2022 09:32:57
>> Betreff: Re: [PATCH -next] ubifs: rename_whiteout: correct old_dir size computing
>> A gentle ping, sorry for the noise.
> I have to say sorry for the day.
> Thanks for your patience with me.
>
Please don't feel bad about it.
Glad to hear from you! Thanks for your kindly reply!
>> 在 2022/2/15 12:07, Baokun Li 写道:
>>> When renaming the whiteout file, the old whiteout file is not deleted.
>>> Therefore, we add the old dentry size to the old dir like XFS.
>>> Otherwise, an error may be reported due to `fscki->calc_sz != fscki->size`
>>> in check_indes.
>>>
>>> Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
>>> Reported-by: Zhihao Cheng <[email protected]>
>>> Signed-off-by: Baokun Li <[email protected]>
>>> ---
>>> fs/ubifs/dir.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
>>> index ae082a0be2a3..86151889548e 100644
>>> --- a/fs/ubifs/dir.c
>>> +++ b/fs/ubifs/dir.c
>>> @@ -1402,6 +1402,9 @@ static int do_rename(struct inode *old_dir, struct dentry
>>> *old_dentry,
>>> iput(whiteout);
>>> goto out_release;
>>> }
>>> +
>>> + /* Add the old_dentry size to the old_dir size. */
>>> + old_sz -= CALC_DENT_SIZE(fname_len(&old_nm));
> So you basically reset old_sz back to 0?
Yes, reset old_sz to 0 here.
Since the old file remains in old_dir after whiteout_rename,
nothing has changed, we just add new_sz to the new folder.

Here, old_sz is the value to be subtracted from old_dir->i_size,
which is initialized to `CALC_DENT_SIZE(fname_len(&old_nm));` .
In RENAME_WHITEOUT, because the old file is not deleted,
we need to subtract old_dent_size from old_sz after RENAME_WHITEOUT is
complete, that is, set old_sz to 0.

we can easily reproduce this problem with the following script:
 ```C
// test_ubifs_whiteout.c
 #include <stdlib.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <sys/syscall.h>
 #include <linux/fs.h>

 int main(int argc, char *argv[])
 {
     system("touch /root/temp/file");
     syscall(SYS_renameat2, AT_FDCWD, "/root/temp/file", AT_FDCWD,
"/root/temp/target", RENAME_WHITEOUT);
     return 0;
 }
 ```

```shell
#!/bin/bash
set -e
# ubifs_whiteout_reproducer.sh

TMP=/root/temp
umount $TMP 2>/dev/null || true
mkdir -p $TMP

modprobe -r ubifs 2>/dev/null || true
for i in $(seq 0 1)
do
    ubidetach -p /dev/mtd$i 2>/dev/null || true
done
modprobe -r ubi 2>/dev/null || true
modprobe -r nandsim 2>/dev/null || true
modprobe nandsim id_bytes="0xec,0xa1,0x00,0x15"
modprobe ubi mtd="0,512"
ubimkvol -N vol_a -m -n 0 /dev/ubi0
modprobe ubifs
mount -t ubifs /dev/ubi0_0 $TMP

./test_ubifs_whiteout
umount /dev/ubi0_0
echo 1 > /sys/kernel/debug/ubifs/chk_fs
mount -t ubifs /dev/ubi0_0 $TMP
```
> Thanks,
> //richard
> .

--
With Best Regards,
Baokun Li

2022-03-17 06:14:34

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH -next] ubifs: rename_whiteout: correct old_dir size computing

----- Ursprüngliche Mail -----
> Von: "libaokun" <[email protected]>
> An: "richard" <[email protected]>, "linux-mtd" <[email protected]>, "linux-kernel"
> <[email protected]>
> CC: "yukuai3" <[email protected]>, "chengzhihao1" <[email protected]>, "libaokun" <[email protected]>
> Gesendet: Donnerstag, 10. März 2022 09:32:57
> Betreff: Re: [PATCH -next] ubifs: rename_whiteout: correct old_dir size computing

> A gentle ping, sorry for the noise.

I have to say sorry for the day.
Thanks for your patience with me.

>
> 在 2022/2/15 12:07, Baokun Li 写道:
>> When renaming the whiteout file, the old whiteout file is not deleted.
>> Therefore, we add the old dentry size to the old dir like XFS.
>> Otherwise, an error may be reported due to `fscki->calc_sz != fscki->size`
>> in check_indes.
>>
>> Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
>> Reported-by: Zhihao Cheng <[email protected]>
>> Signed-off-by: Baokun Li <[email protected]>
>> ---
>> fs/ubifs/dir.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
>> index ae082a0be2a3..86151889548e 100644
>> --- a/fs/ubifs/dir.c
>> +++ b/fs/ubifs/dir.c
>> @@ -1402,6 +1402,9 @@ static int do_rename(struct inode *old_dir, struct dentry
>> *old_dentry,
>> iput(whiteout);
>> goto out_release;
>> }
>> +
>> + /* Add the old_dentry size to the old_dir size. */
>> + old_sz -= CALC_DENT_SIZE(fname_len(&old_nm));

So you basically reset old_sz back to 0?

Thanks,
//richard

2022-03-17 23:32:43

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH -next] ubifs: rename_whiteout: correct old_dir size computing

----- Ursprüngliche Mail -----
> Von: "libaokun" <[email protected]>
>>>> +
>>>> + /* Add the old_dentry size to the old_dir size. */
>>>> + old_sz -= CALC_DENT_SIZE(fname_len(&old_nm));
>> So you basically reset old_sz back to 0?
> Yes, reset old_sz to 0 here.

Okay, makes sense. Patch applied.

Thanks,
//richard

2022-03-18 15:13:05

by Baokun Li

[permalink] [raw]
Subject: Re: [PATCH -next] ubifs: rename_whiteout: correct old_dir size computing

在 2022/3/18 6:35, Richard Weinberger 写道:
> ----- Ursprüngliche Mail -----
>> Von: "libaokun" <[email protected]>
>>>>> +
>>>>> + /* Add the old_dentry size to the old_dir size. */
>>>>> + old_sz -= CALC_DENT_SIZE(fname_len(&old_nm));
>>> So you basically reset old_sz back to 0?
>> Yes, reset old_sz to 0 here.
> Okay, makes sense. Patch applied.
>
> Thanks,
> //richard
> .

Thank you for your review!

--
With Best Regards,
Baokun Li