All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Christophe Leroy" <christophe.leroy@csgroup.eu>,
	"kernel test robot" <lkp@intel.com>,
	"Russell King" <linux@armlinux.org.uk>,
	linux-arm-kernel@lists.infradead.org,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Darren Hart" <dvhart@infradead.org>,
	"Davidlohr Bueso" <dave@stgolabs.net>,
	"André Almeida" <andrealmeid@igalia.com>,
	x86@kernel.org, "Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	"Jan Kara" <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [patch V2 3/6] uaccess: Provide scoped masked user access regions
Date: Thu, 18 Sep 2025 09:20:30 -0400	[thread overview]
Message-ID: <aMwHHkaSECBDjuir@localhost.localdomain> (raw)
In-Reply-To: <20250916163252.164475057@linutronix.de>

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


  reply	other threads:[~2025-09-18 13:20 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-16 16:33 [patch V2 0/6] uaccess: Provide and use scopes for user masked access Thomas Gleixner
2025-09-16 16:33 ` [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword() Thomas Gleixner
2025-09-16 21:26   ` Russell King (Oracle)
2025-09-17  5:48     ` Thomas Gleixner
2025-09-17  9:41       ` Russell King (Oracle)
2025-09-17 12:35         ` Christophe Leroy
2025-09-17 13:55         ` Thomas Gleixner
2025-09-17 15:17           ` Russell King (Oracle)
2025-09-17 17:14             ` Nathan Chancellor
2025-09-17 17:34               ` Russell King (Oracle)
2025-09-17 19:25                 ` Thomas Gleixner
2025-09-17 18:44             ` Thomas Gleixner
2025-09-19 18:27   ` [patch V2a " Thomas Gleixner
2025-09-16 16:33 ` [patch V2 2/6] kbuild: Disable asm goto on clang < 17 Thomas Gleixner
2025-09-16 18:44   ` Nathan Chancellor
2025-09-16 20:43     ` Thomas Gleixner
2025-09-16 20:56       ` [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17 Thomas Gleixner
2025-09-16 21:50         ` Nathan Chancellor
2025-09-24  7:36         ` [tip: core/urgent] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < 17 tip-bot2 for Thomas Gleixner
2025-09-29  9:38         ` [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17 Geert Uytterhoeven
2025-09-29 10:08           ` Peter Zijlstra
2025-09-29 10:58             ` Geert Uytterhoeven
2025-09-29 11:04               ` Peter Zijlstra
2025-09-29 11:10                 ` Geert Uytterhoeven
2025-09-29 15:53                   ` Linus Torvalds
2025-10-02 18:47                     ` David Laight
2025-09-29 22:05                 ` Thomas Gleixner
2025-09-16 16:33 ` [patch V2 3/6] uaccess: Provide scoped masked user access regions Thomas Gleixner
2025-09-18 13:20   ` Mathieu Desnoyers [this message]
2025-09-19  9:10     ` Thomas Gleixner
2025-09-16 16:33 ` [patch V2 4/6] futex: Convert to scoped masked user access Thomas Gleixner
2025-09-16 16:33 ` [patch V2 5/6] x86/futex: " Thomas Gleixner
2025-09-16 16:33 ` [patch V2 6/6] select: " Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aMwHHkaSECBDjuir@localhost.localdomain \
    --to=mathieu.desnoyers@efficios.com \
    --cc=andrealmeid@igalia.com \
    --cc=brauner@kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=dave@stgolabs.net \
    --cc=dvhart@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=lkp@intel.com \
    --cc=nathan@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.