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

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

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>