linux-kernel - Re: [patch V2 3/6] uaccess: Provide scoped masked user access regions

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: <87bjn6959j.ffs@tglx>
Date: Fri, 19 Sep 2025 11:10:48 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Linus Torvalds
 <torvalds@...ux-foundation.org>, Peter Zijlstra <peterz@...radead.org>,
 Christophe Leroy <christophe.leroy@...roup.eu>, kernel test robot
 <lkp@...el.com>, Russell King <linux@...linux.org.uk>,
 linux-arm-kernel@...ts.infradead.org, Nathan Chancellor
 <nathan@...nel.org>, Darren Hart <dvhart@...radead.org>, Davidlohr Bueso
 <dave@...olabs.net>, André Almeida
 <andrealmeid@...lia.com>,
 x86@...nel.org, Alexander Viro <viro@...iv.linux.org.uk>, Christian
 Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 linux-fsdevel@...r.kernel.org
Subject: Re: [patch V2 3/6] uaccess: Provide scoped masked user access regions

On Thu, Sep 18 2025 at 09:20, Mathieu Desnoyers wrote:
> On 16-Sep-2025 06:33:13 PM, Thomas Gleixner wrote:
>> These patterns beg for scopes with automatic cleanup. The resulting outcome
>> is:
>>     	scoped_masked_user_read_access(from, return -EFAULT,
>> 		scoped_get_user(val, from); );
>> 	return 0;
>
> I find a few aspects of the proposed API odd:
>
> - Explicitly implementing the error label within a macro parameter,

Which error label are you talking about?

> - Having the scoped code within another macro parameter.

Macros are limited and the whole construct requires a closed scope to
work and to keep the local variables and the jump label local.

> I would rather expect something like this to mimick our expectations
> in C:

I obviously would love to do it more C style as everyone else.
If you can come up with a way to do that in full C I'm all ears :)

> int func(void __user *ptr, size_t len, char *val1, char *val2)
> {
>         int ret;
>
>         scoped_masked_user_read_access(ptr, len, ret) {
>                 scoped_get_user(val1, ptr[0]);
>                 scoped_get_user(val2, ptr[0]);
>         }
>         return ret;
> }
>
> Where:
>
> - ptr is the pointer at the beginning of the range where the userspace
>   access will be done.

That's the case with the proposed interface already, no?

> - len is the length of the range.

The majority of use cases does not need an explicit size because the
size is determined by the data type. So not forcing everyone to write
scope(ptr, sizeof(*ptr), ..) is a good thing, no?

Adding a sized interface on top for the others is straight forward
enough.

> - ret is a variable used as output (set to -EFAULT on error, 0 on
>   success). If the user needs to do something cleverer than
>   get a -EFAULT on error, they can open-code it rather than use
>   the scoped helper.

Just write  "ret = WHATEVER" instead of "return WHATEVER".

> - The scope is presented similarly to a "for ()" loop scope.

It is a loop scope and you can exit it with either return or break.

> Now I have no clue whether preprocessor limitations prevent achieving
> this somehow, or if it would end up generating poor assembler.

There is no real good way to implement it so that the scope local
requirements are fulfilled. The only alternative would be to close the
scope with a bracket after the code:

      scope(....)
        foo(); }

or for multiline:

      scope(....) {
           ....
      ))

The lonely extra bracket screws up reading even more than the code
within the macro parameter because it's imbalanced. It also makes
tooling from editors to code checkers mightily unhappy.

Thanks,

        tglx