Message-ID: <q3thzkbsq6bwur7baoxvxijnlvnobyt6cx4sckonhgdkviwz76@45b6xlzvrtkr>
Date: Fri, 25 Apr 2025 21:38:02 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: linux-bcachefs@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] bcachefs fixes for 6.15-rc4

On Fri, Apr 25, 2025 at 09:35:27AM -0700, Linus Torvalds wrote:
> On Thu, 24 Apr 2025 at 21:52, Kent Overstreet <kent.overstreet@...ux.dev> wrote:
> >
> > And the attitude of "I hate this, so I'm going to partition this off as
> > much as I can and spend as little time as I can on this" has all made
> > this even worse - the dcache stuff is all half baked.
> 
> No. The dcache side is *correct*.
> 
> The thing is, you absolutely cannot make the case-insensitive lookup
> be the fast case.

That's precisely what the dcache code does, and is the source of the
preblems.

Case insensitivy in the dcache caches the case sensitive name the lookup
was done with, not the case insensitive name we do the comparison
against.

That means we end up with extra aliases - and potentially an _unbounded_
number of aliases if someone is screwing around. Given the problems
we've had with negative dentries, that sounds like a DOS attack waiting
to happen.

It also introduces tricky corner cases into the filesystem code; last
bug I was chasing last night was one where without calling
d_prune_aliases(), those aliases would stick around and end up pointing
to an old version of the vfs inode when the file was deleted and
recreated, which was only caught by bcachefs assertions in our
write_inode method. Eesh.

And that's to say nothing of the complications w.r.t. negative dentries.

I would've rather had the dcache itself do normalization in pathwalk,
and only cache the _normalized_ dirent name.

That would've made lookups a tiny bit slower - but like you said, who
cares; we don't need case insensitive lookups to be the fastpath. RCU
pathwalks would've still worked, it wouldn't have been huge.

Maybe the samba people cared, I haven't looked up the original
discussions. But it seems more likely the determining factor was keeping
case insensitivity out of the dcache code.

> Now, if filesystem people were to see the light, and have a proper and
> well-designed case insensitivity, that might change. But I've never
> seen even a *whiff* of that. I have only seen bad code that
> understands neither how UTF-8 works, nor how unicode works (or rather:
> how unicode does *not* work - code that uses the unicode comparison
> functions without a deeper understanding of what the implications
> are).

Since you're not saying anything about how you think filesystems get
this wrong, this is just trash talking. I haven't seen anything that
looks broken about how case insensitivy is handled.

And honestly I don't think the security "concerns" are real concerns
anymore, since we're actively getting away from directories shared by
different users - /tmp - because that's caused _so_ many problems all on
its own.

(But unicode does create problems with e.g. all the different space
characters, because if you do have a shared directory you can now do
sneaky things like drop in a file that appears to have the same name as
another file, and try to get a unsuspecting user to click on the wrong
one. I don't think that's something the filesystem needs to be getting
involved in - that's a "don't use shared directories, idiot" problem,
i.e. your permissions model is broken if you're affected by that.)

> It's in filesystem people who didn't understand - and still don't,
> after decades - that you MUST NOT just blindly follow some external
> case folding table that you don't understand and that can change over
> time.
> 
> The "change overr time" part is particularly vexing to me, because it
> breaks one of the fundamental rules that unicode was *supposed* to
> fix: no locale garbage.

First off, since the unicode folding rules are referenced by the on disk
formats, they are _not_ changing without review by filesystem folks.

Actually - not in the case of bcachefs, since we store both the
normalized and unnormalized d_name.

Like Ted said, updates that simply add folding rules for new unicode
charecters are totally fine. Aside from that, everyone agrees with you
that we don't want locale garbage, literally no one is asking for that.

So I don't know where yuo're getting that from.

In filesystem land, we version things because you'd be BLOODY STUPID not
to - not because we plan on making changes willy-nilly.

Powered by blists - more mailing lists