linux-kernel - Re: [PATCH 5/5] powerpc: Implement masked user 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250622205109.02fd2ecb@pumpkin>
Date: Sun, 22 Jun 2025 20:51:09 +0100
From: David Laight <david.laight.linux@...il.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Christophe Leroy <christophe.leroy@...roup.eu>, Michael Ellerman
 <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>, Naveen N Rao
 <naveen@...nel.org>, Madhavan Srinivasan <maddy@...ux.ibm.com>, Alexander
 Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, Jan
 Kara <jack@...e.cz>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar
 <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>, Darren Hart
 <dvhart@...radead.org>, Davidlohr Bueso <dave@...olabs.net>, Andre Almeida
 <andrealmeid@...lia.com>, Andrew Morton <akpm@...ux-foundation.org>, Dave
 Hansen <dave.hansen@...ux.intel.com>, linux-kernel@...r.kernel.org,
 linuxppc-dev@...ts.ozlabs.org, linux-fsdevel@...r.kernel.org,
 linux-mm@...ck.org
Subject: Re: [PATCH 5/5] powerpc: Implement masked user access

On Sun, 22 Jun 2025 10:40:00 -0700
Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Sun, 22 Jun 2025 at 10:13, David Laight <david.laight.linux@...il.com> wrote:
> >
> > Not checking the size is slightly orthogonal.
> > It really just depends on the accesses being 'reasonably sequential'.
> > That is probably always true since access_ok() covers a single copy.  
> 
> It is probably true in practice, but yeah, it's worth thinking about.
> Particularly for various user level structure accesses, we do end up
> often accessing the members individually and thus potentially out of
> order, but as you say "reasonable sequential" is still true: the
> accesses are within a reasonably small offset of each other.

I found one that did ptr[4] followed by ptr[0].
Which was potentially problematic if changed to use 'masked' accesses
before you changed the code to use cmov. 

> 
> And when we have potentially very big accesses with large offsets from
> the beginning (ie things like read/write() calls), we do them
> sequentially.
> 
> There *might* be odd ioctls and such that get offsets from user space,
> though. So any conversion to using 'masked_user_access_begin()' needs
> to have at least *some* thought and not be just a mindless conversion
> from access_ok().

True - but the ioctl (like) code is more likely to be using copy_to/from_user()
on the offsetted address rather than trying to be too clever.

> 
> We have this same issue in access_ok() itself, and on x86-64 that does
> 
>   static inline bool __access_ok(const void __user *ptr, unsigned long size)
>   {
>         if (__builtin_constant_p(size <= PAGE_SIZE) && size <= PAGE_SIZE) {
>                 return valid_user_address(ptr);
>         .. do the more careful one that actually uses the 'size' ...
> 
> so it turns access_ok() itself into just a simple single-ended
> comparison with the starting address for small sizes, because we know
> it's ok to overflow by a bit (because of how valid_user_address()
> works on x86).

IIRC there is a comment just below that the says the size could (probably)
just be ignored.
Given how few access_ok() there ought to be, checking them shouldn't be
a problem.
But I get either io_uring or bpf does something strange and unexpected
that is probably a bug waiting to be found.

Remembers some very strange code that has two iovec[] for reading data
from a second process.
I think I failed to find all the access_ok() tests.
IIRC it isn't used by anything 'important' and could be nuked on
security grounds.

	David

> 
>            Linus