linux-kernel - Re: [GIT PULL] vfsuid updates for v6.2

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <68e6091d-e665-bcd7-0c19-ccd80635078e@prevas.dk>
Date:   Thu, 15 Dec 2022 08:37:22 +0100
From:   Rasmus Villemoes <rasmus.villemoes@...vas.dk>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Christian Brauner <brauner@...nel.org>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] vfsuid updates for v6.2

On 13/12/2022 04.28, Linus Torvalds wrote:
> On Mon, Dec 12, 2022 at 4:34 AM Christian Brauner <brauner@...nel.org> wrote:
>>
>> This pull request converts all remaining places that still make use of non-type
>> safe idmapping helpers to rely on the new type safe vfs{g,u}id based helpers.
>> Afterwards it removes all the old non-type safe helpers.
> 
> So I've pulled this, but I'm not entirely happy about some of those
> crazy helpers.
> 
> In particular, the whole "ordering" helpers are really not something
> that should be used in general, I feel. I'm talking about
> vfsuid_gt_kuid() and friends - it's an entirely insane operation and
> makes no sense at all.
> 
> Yes, yes, I understand why they exist (those crazy IMA rules), but I
> feel that those functions *really* shouldn't be exposed to anybody
> else.
> 
> IOW, making those insane functions available in <linux/idmapping.h>
> really seems wrong to me. They are crazy special cases, and I think
> they should exist purely in that crazy ima_security file.

Yeah. Aside from assigning any semantics to < or > of [ug]ids, which is
something IMA apparently wants to do, taking the address of a static
inline in the first place is a code smell; that obviously forces the
compiler to emit a copy in the current TU. But the code compares stored
pointers to addresses of those static inlines, which would be completely
broken if this didn't happen to all be contained in a single TU. That's
quite subtle, and probably fowner_op would be better as an enum.

Rasmus