[PATCH v2] fat: fix parent link count when replacing a sibling directory

[PATCH v2] fat: fix parent link count when replacing a sibling directory

[Hungyu Lin]

vfat_rename() and do_msdos_rename() adjust parent link counts only when
updating the dotdot entry for a move between different parents. Replacing
an existing empty directory in the same parent also removes a
subdirectory, but leaves the parent's link count unchanged.

For example, renaming one of two sibling directories over the other
leaves the parent with four links instead of three. stat and ls report
the stale count, and find -links 3 does not match the parent.

Drop the parent's link count for this case after the source entry has
been removed successfully. Leave cross-parent renames, regular files
and the existing error paths unchanged.

As in rmdir, check that the parent's link count is at least three before
decrementing it. Report a filesystem error if corrupted directory entries
have left the count too low, rather than reducing it below two.

Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
Changes in v2:
- Check the parent link count before decrementing it and report low
  counts with fat_fs_error(), matching rmdir(), as noted by Sashiko.
- Retain Hirofumi's Acked-by from v1.

v1: https://lore.kernel.org/r/20261028211626.26103-1-dennylin0707@gmail.com
Review: https://sashiko.dev/#/patchset/20261028211626.26103-1-dennylin0707@gmail.com
 fs/fat/namei_msdos.c | 8 ++++++++
 fs/fat/namei_vfat.c  | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
index c7500c2859e0..97289c9d9261 100644
--- a/fs/fat/namei_msdos.c
+++ b/fs/fat/namei_msdos.c
@@ -565,6 +565,14 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 		(void)sync_inode_metadata(old_dir, 1);
 
 	if (new_inode) {
+		if (is_dir && old_dir == new_dir) {
+			if (old_dir->i_nlink >= 3)
+				drop_nlink(old_dir);
+			else
+				fat_fs_error(old_dir->i_sb,
+					     "parent dir link count too low (%u)",
+					     old_dir->i_nlink);
+		}
 		drop_nlink(new_inode);
 		if (is_dir)
 			drop_nlink(new_inode);
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index ff7652bf0410..c4d5628c05fa 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -1011,6 +1011,14 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 	vfat_update_dir_metadata(old_dir, &ts);
 
 	if (new_inode) {
+		if (is_dir && old_dir == new_dir) {
+			if (old_dir->i_nlink >= 3)
+				drop_nlink(old_dir);
+			else
+				fat_fs_error(old_dir->i_sb,
+					     "parent dir link count too low (%u)",
+					     old_dir->i_nlink);
+		}
 		drop_nlink(new_inode);
 		if (is_dir)
 			drop_nlink(new_inode);
-- 
2.43.0

Re: [PATCH v2] fat: fix parent link count when replacing a sibling directory

[OGAWA Hirofumi]

Hungyu Lin <dennylin0707@gmail.com> writes:

> vfat_rename() and do_msdos_rename() adjust parent link counts only when
> updating the dotdot entry for a move between different parents. Replacing
> an existing empty directory in the same parent also removes a
> subdirectory, but leaves the parent's link count unchanged.
>
> For example, renaming one of two sibling directories over the other
> leaves the parent with four links instead of three. stat and ls report
> the stale count, and find -links 3 does not match the parent.
>
> Drop the parent's link count for this case after the source entry has
> been removed successfully. Leave cross-parent renames, regular files
> and the existing error paths unchanged.
>
> As in rmdir, check that the parent's link count is at least three before
> decrementing it. Report a filesystem error if corrupted directory entries
> have left the count too low, rather than reducing it below two.
>
> Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---
> Changes in v2:
> - Check the parent link count before decrementing it and report low
>   counts with fat_fs_error(), matching rmdir(), as noted by Sashiko.
> - Retain Hirofumi's Acked-by from v1.
>
> v1: https://lore.kernel.org/r/20261028211626.26103-1-dennylin0707@gmail.com
> Review: https://sashiko.dev/#/patchset/20261028211626.26103-1-dennylin0707@gmail.com
>  fs/fat/namei_msdos.c | 8 ++++++++
>  fs/fat/namei_vfat.c  | 8 ++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
> index c7500c2859e0..97289c9d9261 100644
> --- a/fs/fat/namei_msdos.c
> +++ b/fs/fat/namei_msdos.c
> @@ -565,6 +565,14 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
>  		(void)sync_inode_metadata(old_dir, 1);
>  
>  	if (new_inode) {
> +		if (is_dir && old_dir == new_dir) {
> +			if (old_dir->i_nlink >= 3)
> +				drop_nlink(old_dir);
> +			else
> +				fat_fs_error(old_dir->i_sb,
> +					     "parent dir link count too low (%u)",
> +					     old_dir->i_nlink);
> +		}
>  		drop_nlink(new_inode);
>  		if (is_dir)
>  			drop_nlink(new_inode);
> diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
> index ff7652bf0410..c4d5628c05fa 100644
> --- a/fs/fat/namei_vfat.c
> +++ b/fs/fat/namei_vfat.c
> @@ -1011,6 +1011,14 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
>  	vfat_update_dir_metadata(old_dir, &ts);
>  
>  	if (new_inode) {
> +		if (is_dir && old_dir == new_dir) {
> +			if (old_dir->i_nlink >= 3)
> +				drop_nlink(old_dir);
> +			else
> +				fat_fs_error(old_dir->i_sb,
> +					     "parent dir link count too low (%u)",
> +					     old_dir->i_nlink);
> +		}
>  		drop_nlink(new_inode);
>  		if (is_dir)
>  			drop_nlink(new_inode);

Thanks. Hm, how about move those same code to fat module

	fat_dir_drop_nlink()
        EXPORT_SYMBOL_GPL(fat_dir_drop_nlink)

as such function for 3 places?
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH v2] fat: fix parent link count when replacing a sibling directory

[OGAWA Hirofumi]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> Hungyu Lin <dennylin0707@gmail.com> writes:
>
>> vfat_rename() and do_msdos_rename() adjust parent link counts only when
>> updating the dotdot entry for a move between different parents. Replacing
>> an existing empty directory in the same parent also removes a
>> subdirectory, but leaves the parent's link count unchanged.
>>
>> For example, renaming one of two sibling directories over the other
>> leaves the parent with four links instead of three. stat and ls report
>> the stale count, and find -links 3 does not match the parent.
>>
>> Drop the parent's link count for this case after the source entry has
>> been removed successfully. Leave cross-parent renames, regular files
>> and the existing error paths unchanged.
>>
>> As in rmdir, check that the parent's link count is at least three before
>> decrementing it. Report a filesystem error if corrupted directory entries
>> have left the count too low, rather than reducing it below two.
>>
>> Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
>> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
>> ---
>> Changes in v2:
>> - Check the parent link count before decrementing it and report low
>>   counts with fat_fs_error(), matching rmdir(), as noted by Sashiko.
>> - Retain Hirofumi's Acked-by from v1.
>>
>> v1: https://lore.kernel.org/r/20261028211626.26103-1-dennylin0707@gmail.com
>> Review: https://sashiko.dev/#/patchset/20261028211626.26103-1-dennylin0707@gmail.com
>>  fs/fat/namei_msdos.c | 8 ++++++++
>>  fs/fat/namei_vfat.c  | 8 ++++++++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
>> index c7500c2859e0..97289c9d9261 100644
>> --- a/fs/fat/namei_msdos.c
>> +++ b/fs/fat/namei_msdos.c
>> @@ -565,6 +565,14 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
>>  		(void)sync_inode_metadata(old_dir, 1);
>>  
>>  	if (new_inode) {
>> +		if (is_dir && old_dir == new_dir) {
>> +			if (old_dir->i_nlink >= 3)
>> +				drop_nlink(old_dir);
>> +			else
>> +				fat_fs_error(old_dir->i_sb,
>> +					     "parent dir link count too low (%u)",
>> +					     old_dir->i_nlink);
>> +		}
>>  		drop_nlink(new_inode);
>>  		if (is_dir)
>>  			drop_nlink(new_inode);
>> diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
>> index ff7652bf0410..c4d5628c05fa 100644
>> --- a/fs/fat/namei_vfat.c
>> +++ b/fs/fat/namei_vfat.c
>> @@ -1011,6 +1011,14 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
>>  	vfat_update_dir_metadata(old_dir, &ts);
>>  
>>  	if (new_inode) {
>> +		if (is_dir && old_dir == new_dir) {
>> +			if (old_dir->i_nlink >= 3)
>> +				drop_nlink(old_dir);
>> +			else
>> +				fat_fs_error(old_dir->i_sb,
>> +					     "parent dir link count too low (%u)",
>> +					     old_dir->i_nlink);
>> +		}
>>  		drop_nlink(new_inode);
>>  		if (is_dir)
>>  			drop_nlink(new_inode);
>
> Thanks. Hm, how about move those same code to fat module
>
> 	fat_dir_drop_nlink()
>         EXPORT_SYMBOL_GPL(fat_dir_drop_nlink)
>
> as such function for 3 places?

Ouch, it should be 4 places.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>