Re: [PATCH] btrfs: test create a bunch of files with name hash collision

Re: [PATCH] btrfs: test create a bunch of files with name hash collision

[Christoph Hellwig]

On Thu, Feb 26, 2026 at 02:34:37PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Test that if we create a high number of files with a name that results in
> a hash collision, the filesystem is not turned to RO due to a transaction
> abort. This could be exploited by malicious users to disrupt a system.

Umm, file systems must handle an unlimited number of name collisions.
While going read-only is of course really bad, just rejecting them
can also pretty easily break things.

Also it seems like part of this test is generic, and only the subvolume
creation part is btrfs-specific?

Re: [PATCH] btrfs: test create a bunch of files with name hash collision

[Filipe Manana]

On Fri, Mar 6, 2026 at 2:35 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Feb 26, 2026 at 02:34:37PM +0000, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Test that if we create a high number of files with a name that results in
> > a hash collision, the filesystem is not turned to RO due to a transaction
> > abort. This could be exploited by malicious users to disrupt a system.
>
> Umm, file systems must handle an unlimited number of name collisions.
> While going read-only is of course really bad, just rejecting them
> can also pretty easily break things.

I don't think in practice we get a large enough number of names with a
crc32c hash collision in btrfs.
Never heard yet of any users reporting problems with that.

The fix related to this test is more to prevent a bad intentioned user
from turning the fs into read-only mode (this is actually a regression
introduced a few years ago).

Adding support for an unlimited number of collisions is simply not
easily doable, it would require an on-disk format change (new key
type, item, etc, update btrfs-progs, etc).
The motivation for that is very low, as I'm not aware of users ever complaining.

>
> Also it seems like part of this test is generic, and only the subvolume
> creation part is btrfs-specific?

Specifying a 4K leaf size at mkfs, all the comments, and verifying
that we can't create a file beyond the limit, are also btrfs specific.

>

Re: [PATCH] btrfs: test create a bunch of files with name hash collision

[Christoph Hellwig]

On Fri, Mar 06, 2026 at 02:55:23PM +0000, Filipe Manana wrote:
> > Umm, file systems must handle an unlimited number of name collisions.
> > While going read-only is of course really bad, just rejecting them
> > can also pretty easily break things.
> 
> I don't think in practice we get a large enough number of names with a
> crc32c hash collision in btrfs.

That's not the point.  Posix and Linux file systems don't have an
error code or defined condition for adding this file name would cause
a hash collision and we were to lazy to deal with it.

> Adding support for an unlimited number of collisions is simply not
> easily doable, it would require an on-disk format change (new key
> type, item, etc, update btrfs-progs, etc).
> The motivation for that is very low, as I'm not aware of users ever complaining.

Well, how would they know this happened?  The users only sees the file
system acting weird in completely unpredictable ways.

Re: [PATCH] btrfs: test create a bunch of files with name hash collision

[Filipe Manana]

On Tue, Mar 10, 2026 at 1:13 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Fri, Mar 06, 2026 at 02:55:23PM +0000, Filipe Manana wrote:
> > > Umm, file systems must handle an unlimited number of name collisions.
> > > While going read-only is of course really bad, just rejecting them
> > > can also pretty easily break things.
> >
> > I don't think in practice we get a large enough number of names with a
> > crc32c hash collision in btrfs.
>
> That's not the point.  Posix and Linux file systems don't have an
> error code or defined condition for adding this file name would cause
> a hash collision and we were to lazy to deal with it.

No one is disagreeing with you Christoph.

What I was saying is that giving support for an unlimited number of
collisions would require an update to the on-disk format, which is a
lot of work as it implies metadata changes, feature incompatibility
bits, updating the user space tools, etc.
Given the work that would be needed and the fact no users have
complained about this (to my knowledge at least), the motivation for
such big changes isn't much.

>
> > Adding support for an unlimited number of collisions is simply not
> > easily doable, it would require an on-disk format change (new key
> > type, item, etc, update btrfs-progs, etc).
> > The motivation for that is very low, as I'm not aware of users ever complaining.
>
> Well, how would they know this happened?  The users only sees the file
> system acting weird in completely unpredictable ways.

Because no one reported -EOVERFLOW errors when creating files. I
haven't seen it in more than 12 years at least - sure it doesn't mean
there isn't someone who hasn't hit it, but we don't know about it.

What this test is doing is to verify we don't turn the fs into RO mode
after -EOVERFLOW - which is actually a regression we had since 2022,
and could be used as a sort of denial of service.
This is a useful test for us btrfs developers to make sure we don't
regress on that.

You may disagree with not allowing an unlimited number of hash
collisions, but that is another topic and doesn't invalidate this test
(and that's part of the reason why it's a btrfs specific test).


>