Re: [BUG] vhost/vringh: __get_user without access_ok leaks kernel memory via vDPA passthrough

["Michael S. Tsirkin" <mst@redhat.com>]

On Sun, Oct 18, 2026 at 11:11:44AM -1000, Linus Torvalds wrote:
> On Sun, 18 Oct 2026 at 10:34, Linus Torvalds
> <torvalds@linuxfoundation.org> wrote:
> >
> > And __get_user() together with access_ok() is actively wrong and
> > stupid. It's slower, bigger, and more bug-prone than just using
> > "get_user()".
> 
> Btw, if you have *multiple* accesses to the same structure, that was a
> historical reason to use access_ok() with multiple __get_user() calls.
> 
> But it's not a useful pattern today - and hasn't been for years -
> because the real cost is not the access_ok(), but the stac/clac that
> surrounds __get_user() on modern hardware.
> 
> If performance is really crticial, the new "unsafe_get_user()" model
> is the way to go.

Thanks! I'll try to find the time to look into converting vhost to that.
The flood of ai found bugs I am dealing with will abate any day now.
Any... day... now...

> Here's an UNTESTED example patch of how to do a conversion. It
> generates much better code than __get_user() ever did. That patch
> looks good to me, but again: I didn't actually *test* it (and the
> reason this signal.c code has never been updated to the modern pattern
> is that it really doesn't matter for performance - I decided to use it
> as an example not because it matters, but because it's a good example
> code-sequence for the legacy model conversion).
> 
> Somebody should test it.
> 
>                      Linus

>  kernel/signal.c | 56 ++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 30 insertions(+), 26 deletions(-)
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index d31ebcb6ed4d..0fd1296e912b 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -4776,12 +4776,12 @@ SYSCALL_DEFINE3(sigaction, int, sig,
>  
>  	if (act) {
>  		old_sigset_t mask;
> -		if (!access_ok(act, sizeof(*act)) ||
> -		    __get_user(new_ka.sa.sa_handler, &act->sa_handler) ||
> -		    __get_user(new_ka.sa.sa_restorer, &act->sa_restorer) ||
> -		    __get_user(new_ka.sa.sa_flags, &act->sa_flags) ||
> -		    __get_user(mask, &act->sa_mask))
> -			return -EFAULT;
> +		scoped_user_read_access(act, Efault) {
> +			unsafe_get_user(new_ka.sa.sa_handler, &act->sa_handler, Efault);
> +			unsafe_get_user(new_ka.sa.sa_restorer, &act->sa_restorer, Efault);
> +			unsafe_get_user(new_ka.sa.sa_flags, &act->sa_flags, Efault);
> +			unsafe_get_user(mask, &act->sa_mask, Efault);
> +		}
>  #ifdef __ARCH_HAS_KA_RESTORER
>  		new_ka.ka_restorer = NULL;
>  #endif
> @@ -4791,15 +4791,18 @@ SYSCALL_DEFINE3(sigaction, int, sig,
>  	ret = do_sigaction(sig, act ? &new_ka : NULL, oact ? &old_ka : NULL);
>  
>  	if (!ret && oact) {
> -		if (!access_ok(oact, sizeof(*oact)) ||
> -		    __put_user(old_ka.sa.sa_handler, &oact->sa_handler) ||
> -		    __put_user(old_ka.sa.sa_restorer, &oact->sa_restorer) ||
> -		    __put_user(old_ka.sa.sa_flags, &oact->sa_flags) ||
> -		    __put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask))
> -			return -EFAULT;
> +		scoped_user_write_access(act, Efault) {
> +			unsafe_put_user(old_ka.sa.sa_handler, &oact->sa_handler, Efault);
> +			unsafe_put_user(old_ka.sa.sa_restorer, &oact->sa_restorer, Efault);
> +			unsafe_put_user(old_ka.sa.sa_flags, &oact->sa_flags, Efault);
> +			unsafe_put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask, Efault);
> +		}
>  	}
>  
>  	return ret;
> +
> +Efault:
> +	return -EFAULT;
>  }
>  #endif
>  #ifdef CONFIG_COMPAT_OLD_SIGACTION
> @@ -4813,12 +4816,12 @@ COMPAT_SYSCALL_DEFINE3(sigaction, int, sig,
>  	compat_uptr_t handler, restorer;
>  
>  	if (act) {
> -		if (!access_ok(act, sizeof(*act)) ||
> -		    __get_user(handler, &act->sa_handler) ||
> -		    __get_user(restorer, &act->sa_restorer) ||
> -		    __get_user(new_ka.sa.sa_flags, &act->sa_flags) ||
> -		    __get_user(mask, &act->sa_mask))
> -			return -EFAULT;
> +		scoped_user_read_access(act, Efault) {
> +			unsafe_get_user(handler, &act->sa_handler, Efault);
> +			unsafe_get_user(restorer, &act->sa_restorer, Efault);
> +			unsafe_get_user(new_ka.sa.sa_flags, &act->sa_flags, Efault);
> +			unsafe_get_user(mask, &act->sa_mask, Efault);
> +		}
>  
>  #ifdef __ARCH_HAS_KA_RESTORER
>  		new_ka.ka_restorer = NULL;
> @@ -4831,16 +4834,17 @@ COMPAT_SYSCALL_DEFINE3(sigaction, int, sig,
>  	ret = do_sigaction(sig, act ? &new_ka : NULL, oact ? &old_ka : NULL);
>  
>  	if (!ret && oact) {
> -		if (!access_ok(oact, sizeof(*oact)) ||
> -		    __put_user(ptr_to_compat(old_ka.sa.sa_handler),
> -			       &oact->sa_handler) ||
> -		    __put_user(ptr_to_compat(old_ka.sa.sa_restorer),
> -			       &oact->sa_restorer) ||
> -		    __put_user(old_ka.sa.sa_flags, &oact->sa_flags) ||
> -		    __put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask))
> -			return -EFAULT;
> +		scoped_user_write_access(act, Efault) {
> +			unsafe_put_user(ptr_to_compat(old_ka.sa.sa_handler), &oact->sa_handler, Efault);
> +			unsafe_put_user(ptr_to_compat(old_ka.sa.sa_restorer), &oact->sa_restorer, Efault);
> +			unsafe_put_user(old_ka.sa.sa_flags, &oact->sa_flags, Efault);
> +			unsafe_put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask, Efault);
> +		}
>  	}
>  	return ret;
> +
> +Efault:
> +	return -EFAULT;
>  }
>  #endif
>