linux-kernel - Re: [PATCH v5 3/4] fat: add renameat2 RENAME

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 9 Jun 2022 22:36:21 +0200
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
Cc:     linux-kernel@...r.kernel.org, Peter Jones <pjones@...hat.com>,
        Alexander Larsson <alexl@...hat.com>,
        Colin Walters <walters@...bum.org>,
        Alberto Ruiz <aruiz@...hat.com>,
        Muhammad Usama Anjum <usama.anjum@...labora.com>,
        Lennart Poettering <lennart@...ttering.net>,
        Chung-Chiang Cheng <cccheng@...ology.com>,
        Christian Kellner <ckellner@...hat.com>,
        Carlos Maiolino <cmaiolin@...hat.com>
Subject: Re: [PATCH v5 3/4] fat: add renameat2 RENAME_EXCHANGE flag support

Hello OGAWA,

Thanks for your feedback.

On 6/9/22 21:17, OGAWA Hirofumi wrote:
> Javier Martinez Canillas <javierm@...hat.com> writes:
> 
>> +static void vfat_update_nlink(struct inode *dir, struct inode *inode)
>> +{
>> +		if (S_ISDIR(inode->i_mode))
>> +			drop_nlink(dir);
>> +		else
>> +			inc_nlink(dir);
>> +}
> 
> [...]
> 
>> +	vfat_update_dir_metadata(old_dir, &ts);
>> +	/* if directories are not the same, update new_dir as well */
>> +	if (old_dir != new_dir) {
>> +		vfat_update_dir_metadata(new_dir, &ts);
>> +		/* nlink only needs to be updated if the file types differ */
>> +		if (old_inode->i_mode != new_inode->i_mode) {
>> +			vfat_update_nlink(old_dir, old_inode);
>> +			vfat_update_nlink(new_dir, new_inode);
>> +		}
>> +	}
> 
> Looks like unnecessary complex (and comparing raw i_mode, not S_ISDIR(),
> better to change before make dir dirty).  How about this change, it is
> only tested slightly though? Can you review and test?
>

Your change looks good to me and indeed the logic is simpler than in mine.

I've also tested it and AFAICT it works correctly as well. Do you plan to
squash this or should I respin a new revision of the whole patch-set ? If
you want to post it as a follow-up I'm also OK with that.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat