[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aMwHHkaSECBDjuir@localhost.localdomain>
Date: Thu, 18 Sep 2025 09:20:30 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Thomas Gleixner <tglx@...utronix.de>
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 16-Sep-2025 06:33:13 PM, Thomas Gleixner wrote:
> User space access regions are tedious and require similar code patterns all
> over the place:
>
> if (!user_read_access_begin(from, sizeof(*from)))
> return -EFAULT;
> unsafe_get_user(val, from, Efault);
> user_read_access_end();
> return 0;
> Efault:
> user_read_access_end();
> return -EFAULT;
>
> This got worse with the recend addition of masked user access, which
> optimizes the speculation prevention:
>
> if (can_do_masked_user_access())
> from = masked_user_read_access_begin((from));
> else if (!user_read_access_begin(from, sizeof(*from)))
> return -EFAULT;
> unsafe_get_user(val, from, Efault);
> user_read_access_end();
> return 0;
> Efault:
> user_read_access_end();
> return -EFAULT;
>
> There have been issues with using the wrong user_*_access_end() variant in
> the error path and other typical Copy&Pasta problems, e.g. using the wrong
> fault label in the user accessor which ends up using the wrong accesss end
> variant.
>
> 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,
- Having the scoped code within another macro parameter.
I would rather expect something like this to mimick our expectations
in C:
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.
- len is the length of the range.
- 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.
- The scope is presented similarly to a "for ()" loop scope.
Now I have no clue whether preprocessor limitations prevent achieving
this somehow, or if it would end up generating poor assembler.
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Powered by blists - more mailing lists