Re: [RFC] a possible way of reducing the PITA of ->d_name audits

[Al Viro <viro@zeniv.linux.org.uk>]

On Sun, Sep 07, 2025 at 05:47:31PM -0700, Linus Torvalds wrote:
> On Sun, 7 Sept 2025 at 17:06, Al Viro <viro@zeniv.linux.org.uk> wrote:

> I would expect that you *only* do this for the functions where the
> name isn't stable (ie it's called without the parent locked).
> 
> So rmdir/mknod/link/etc would continue to just pass the dentry,
> because the name is stable and filesystems using dentry->d_name is
> perfectly fine.

Absolute majority of those ~800 hits *are* in those or in functions
called only from them.

> Ie we'd end up using take_dentry_name_snapshot().
> 
> Would that be horribly bad?

In some cases we'll have to (trace shite, mostly - racy, but not "the
sky is falling" stuff), but that (and any real crap) drowns in the
false positives.

> Don't a lot of routines already have the parent locked - all the
> normal functions where we actually _modify_ the directory - so the
> name is stable. No?

Yes, it is.

> Then, the other thing we could do is just mark "struct qstr d_name" as
> 'const' in struct dentry, and then the (very very few) places that
> actually modify the name will have to use an alias to do the
> modification.

Yes, and I have that (that's what I mentioned as the first and easy part
of those audits - see #work.qstr, doing exactly that).

> Wouldn't that deal with at least a majority of the worries of people
> playing games?
> 
> This is where you go "Oh, Linus, you sweet summer child", and shake
> your head. You've been walking through the call-chains, I haven't.

The problem is that a regression (e.g. somebody suddenly starting to read
->d_name in their ->open() or ->setattr(), or... - we had such cases) is
always possible and the way the things are it's fucking hard to spot.

Most of the uses *are* done to stable dentries; it's just that we have no
way to tell which ones are like that.  That's exactly the reason why I'm
thinking about such annotations.  The only way to catch regressions is
to grep, go over the list and manually verify that all of those places
still are reachable only from the "safe" methods.  Worse, comparing
the grep output from the previous cycle (modulo line number shifts,
etc.) doesn't help - if you have one in a function 3 levels down from a
method and its caller grows an extra call site, you would see nothing
in the diff anywhere near the places where ->d_name is used or any of
the directory methods.

I'm not so worried about anyone malicious - honest fuckups happen and
AFAICS all of them so far had been of that sort.  I would really like
to be able to find such fuckups without ~8--12 hours of digging;
needs breaks, too, since it's monotonous enough to cause something
similar to highway hypnosis, IME ;-/