linux-kernel - Re: [RFC PATCH 19/24] erofs: introduce namei alternative to C

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]

Message-ID: <8871d954-4e6d-4e2d-9080-c5950e7ac2c6@linux.alibaba.com>
Date: Tue, 17 Sep 2024 16:08:26 +0800
From: Gao Xiang <hsiangkao@...ux.alibaba.com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: Yiyang Wu <toolmanp@...p.cc>, linux-erofs@...ts.ozlabs.org,
 rust-for-linux@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 19/24] erofs: introduce namei alternative to C



On 2024/9/17 15:44, Al Viro wrote:
> On Tue, Sep 17, 2024 at 08:31:49AM +0100, Al Viro wrote:
> 
>>> After d_splice_alias() and d_add(), rename() could change d_name.  So
>>> either we take d_lock or with rcu_read_lock() to take a snapshot of
>>> d_name in the RCU walk path.  That is my overall understanding.
>>
>> No, it's more complicated than that, sadly.  ->d_name and ->d_parent are
>> the trickiest parts of dentry field stability.
>>
>>> But for EROFS, since we don't have rename, so it doesn't matter.
>>
>> See above.  IF we could guarantee that all filesystem images are valid
>> and will remain so, life would be much simpler.
> 
> In any case, currently it is safe - d_splice_alias() is the last thing
> done by erofs_lookup().  Just don't assume that names can't change in
> there - and the fewer places in filesystem touch ->d_name, the better.
> 
> In practice, for ->lookup() you are safe until after d_splice_alias()
> and for directory-modifying operations you are safe unless you start
> playing insane games with unlocking and relocking the parent directories
> (apparmorfs does; the locking is really obnoxious there).  That covers
> the majority of ->d_name and ->d_parent accesses in filesystem code.
> 
> ->d_hash() and ->d_compare() are separate story; I've posted a text on
> that last year (or this winter - not sure, will check once I get some
> sleep).
> 
> d_path() et.al. are taking care to do the right thing; those (and %pd
> format) can be used safely.
> 
> Anyway, I'm half-asleep at the moment and I'd rather leave writing these
> rules up until tomorrow.  Sorry...

Agreed, thanks for writing so many words on this!

Thanks,
Gao Xiang