Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RESEND PATCH v2] hfsplus: prevent negative dentries when
 casefolded
To:     Viacheslav Dubeyko <slava@dubeyko.com>
Cc:     Linux FS Devel <linux-fsdevel@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Andrew Morton <akpm@linux-foundation.org>,
        gustavoars@kernel.org, gregkh@linuxfoundation.org,
        keescook@chromium.org, mszeredi@redhat.com, shepjeng@gmail.com
References: <20210716073635.1613671-1-cccheng@synology.com>
 <02B9566C-A78E-42FB-924B-A503E4BC6D2F@dubeyko.com>
From:   Chung-Chiang Cheng <cccheng@synology.com>
Message-ID: <a2c84cfa-b6ed-c86c-0bb1-d05087c141d7@synology.com>
Date:   Mon, 19 Jul 2021 17:03:45 +0800
MIME-Version: 1.0
In-Reply-To: <02B9566C-A78E-42FB-924B-A503E4BC6D2F@dubeyko.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk

This function revalidates dentries without blocking and storing to the
dentry. As the document mentioned [1], I think it's safe in rcu-walk
mode. I also found jfs_ci_revalidate() takes the same approach.

         d_revalidate may be called in rcu-walk mode (flags & LOOKUP_RCU).
         If in rcu-walk mode, the filesystem must revalidate the dentry 
without
         blocking or storing to the dentry, d_parent and d_inode should 
not be
         used without care (because they can change and, in d_inode 
case, even
         become NULL under us


[1] https://www.kernel.org/doc/Documentation/filesystems/vfs.txt

Thanks,
C.C.Cheng

>> +
>> +int hfsplus_revalidate_dentry(struct dentry *dentry, unsigned int flags)
>> +{
> What’s about this code?
>
> If (flags & LOOKUP_RCU)
>     return -ECHILD;
>
> Do we really need to miss it here?
>
> Thanks,
> Slava.
>
>
>> +	/*
>> +	 * dentries are always valid when disabling casefold.
>> +	 */
>> +	if (!test_bit(HFSPLUS_SB_CASEFOLD, &HFSPLUS_SB(dentry->d_sb)->flags))
>> +		return 1;
>> +
>> +	/*
>> +	 * Positive dentries are valid when enabling casefold.
>> +	 *
>> +	 * Note, rename() to existing directory entry will have ->d_inode, and
>> +	 * will use existing name which isn't specified name by user.
>> +	 *
>> +	 * We may be able to drop this positive dentry here. But dropping
>> +	 * positive dentry isn't good idea. So it's unsupported like
>> +	 * rename("filename", "FILENAME") for now.
>> +	 */
>> +	if (d_really_is_positive(dentry))
>> +		return 1;
>> +
>> +	/*
>> +	 * Drop the negative dentry, in order to make sure to use the case
>> +	 * sensitive name which is specified by user if this is for creation.
>> +	 */
>> +	if (flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET))
>> +		return 0;
>> +
>> +	return 1;
>> +}
>> -- 
>> 2.25.1
>>