linux-kernel - Re: [PATCH] stat: don't fail if the major number is >= 256

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: <CAHk-=wgsHK4pDDoEgCyKgGyo-AMGpy1jg2QbstaCR0G-v568yg@mail.gmail.com>
Date:   Mon, 11 Apr 2022 19:37:44 -1000
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Mikulas Patocka <mpatocka@...hat.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH] stat: don't fail if the major number is >= 256

On Mon, Apr 11, 2022 at 7:13 AM Mikulas Patocka <mpatocka@...hat.com> wrote:
>
> Should we perhaps hash the number, take 16 bits of the hash and hope
> than the collision won't happen?

That would "work", but I think it would be incredibly annoying to
users with basically random results.

I think the solution is to just put the bits in the high bits. Yes,
they might be masked off if people use 'MAJOR()' to pick them out, but
the common "compare st_dev and st_ino" model at least works. That's
the one that wants unique numbers.

> For me, the failure happens in cp_compat_stat (I have a 64-bit kernel). In
> struct compat_stat in arch/x86/include/asm/compat.h, st_dev and st_rdev
> are compat_dev_t which is 16-bit. But they are followed by 16-bit
> paddings, so they could be extended.

Ok, that actually looks like a bug.

The compat structure should match the native structure.  Those "u16
__padX" fields seem to be just a symptom of the bug.

The only user of that compat_stat structure is the kernel, so that
should just be fixed.

Of course, who knows what the libraries have done, so user space could
still have screwed up.

> If you have a native 32-bit kernel, it uses 'struct stat' defined at the
> beginning of arch/x86/include/uapi/asm/stat.h that has 32-bit st_dev and
> st_rdev.

Correct. It's literally the compat structure that has no basis in reality.

Or it might be some truly ancient thing, but I really don't think so.

Regardless, if we fill in the high 16 bits of that field, in the
_worst_ case things will act as if your patch had been applied, but in
any sane case you'll have that working "unique st_dev" thing.

I'd love to actually use the better "modern" 64-bit encoding (12+20
bits, or whatever it is we do, too lazy to look it up), but hey, that
historical thing is what it is.

Realistically, nobody uses it. Apparently your OpenWatcom use is also
really just fairly theoretical, not some "this app is used by people
and doesn't work with a filesystem on NVMe".

                 Linus