linux-kernel - Re: [patch 0/4] uaccess: Provide and use helpers for user masked access

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] [day] [month] [year] [list]

Message-ID: <9b9b414d-0782-4bcf-aaac-386db96843bc@t-8ch.de>
Date: Tue, 19 Aug 2025 06:44:05 +0200
From: Thomas Weißschuh <linux@...ssschuh.net>
To: David Laight <david.laight.linux@...il.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, 
	LKML <linux-kernel@...r.kernel.org>, Linus Torvalds <torvalds@...ux-foundation.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Peter Zijlstra <peterz@...radead.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 0/4] uaccess: Provide and use helpers for user masked
 access

On 2025-08-18 22:21:06+0100, David Laight wrote:
> On Sun, 17 Aug 2025 14:49:43 +0100
> David Laight <david.laight.linux@...il.com> wrote:

(...)

> Would something like this work (to avoid the hidden update)?
> 
> #define user_read_begin(uaddr, size, error_code) ({	\
> 	typeof(uaddr) __uaddr;				\
> 	if (can_do_masked_user_access())		\
> 		__uaddr = masked_user_access_begin(uaddr);\
> 	else if (user_read_access_begin(uaddr, size))	\
> 		__uaddr = uaddr;			\
> 	else {						\
> 		error_code;				\
> 	}						\
> 	__uaddr;					\
> })
> 
> With typical use being either:
> 	uaddr = user_read_begin(uaddr, sizeof (*uaddr), return -EFAULT);
> or:
> 	uaddr = user_read_begin(uaddr, sizeof (*uaddr), goto bad_uaddr);
> 
> One problem is I don't think you can easily enforce the assignment.
> Ideally you'd want something that made the compiler think that 'uaddr' was unset.
> It could be done for in a debug/diagnostic compile by adding 'uaddr = NULL'
> at the bottom of the #define and COMPILE_ASSERT(!staticically_true(uaddr == NULL))
> inside unsafe_get/put_user().

To enforce some assignment, but not to the exact same variable as the argument,
you can wrap user_read_begin() in a function marked as __must_check.


#define __user_read_begin(uaddr, size, error_code) ({	\
	/* See above */
})

static __always_inline void __must_check __user *__user_read_check(void __user *val)
{
	return val;
}

#define user_read_begin(uaddr, size, error_code)	\
	((typeof(uaddr))__user_read_check(__user_read_begin(uaddr, size, error_code)))


Ignoring the return value gives:

error: ignoring return value of ‘__user_read_check’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
 1629 |         ((typeof(uaddr))__user_read_check(__user_read_begin(uaddr, size, error_code)))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: in expansion of macro ‘user_read_begin’
 1635 |         user_read_begin(uaddr, sizeof (*uaddr), return -EFAULT);
      |         ^~~~~~~~~~~~~~~


Thomas