linux-kernel - Re: [PATCH] futex: improve user space accesses

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ca5559b9-16fe-4808-a28b-b87129b0e6b8@csgroup.eu>
Date: Mon, 9 Dec 2024 09:00:07 +0100
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
 Andreas Schwab <schwab@...ux-m68k.org>
Cc: Josh Poimboeuf <jpoimboe@...nel.org>, LKML
 <linux-kernel@...r.kernel.org>, x86@...nel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH] futex: improve user space accesses



Le 09/12/2024 à 01:32, Linus Torvalds a écrit :
> On Sun, 8 Dec 2024 at 14:54, Andreas Schwab <schwab@...ux-m68k.org> wrote:
>>
>> This breaks userspace on ppc32.  As soon as /init in the initrd is
>> started the kernel hangs (without any messages).
> 
> Funky, funky. Most of the diff is the code movement (and some small
> x86-specific stuff), so for ppc, the only part that should be relevant
> is the futex_get_value_locked().
> 
> And since ppc doesn't do the masked user access thing, so it
> *literally* boils down to just that
> 
>          if (!user_read_access_begin(from, sizeof(*from)))
>                  return -EFAULT;
>          unsafe_get_user(val, from, Efault);
>          user_access_end();
> 
> path.
> 
> Ahh... And now that I write that out, the bug is obvious: it should be using
> 
>          user_read_access_end();
> 
> to match up with the user_read_access_begin().

Yes indeed, especially on book3s/32, which is only able to write-protect 
user accesses. On that platform user_read_access_...() are no-ops.

user_access_end() and user_write_access_end() are similar, and rely on a 
thread var stored by user_access_begin(). When calling that 
user_access_end() without prior call to user_access_begin(), that var 
has value ~0 instead of the address of the user segment being accessed, 
and ~0 is a kernel address so user_access_end() applies some user 
segment flags to a kernel segment which most likely leads to a complete 
mess allthough I'm not able to trigger the hang with QEMU.

> 
> And yeah, ppc is the only platform that has that
> "read-vs-write-vs-both" thing, so this bug is not visible anywhere
> else.
> 
> IOW, does this one-liner fix it for you?
> 
>    --- a/kernel/futex/futex.h
>    +++ b/kernel/futex/futex.h
>    @@ -265,7 +265,7 @@
>          else if (!user_read_access_begin(from, sizeof(*from)))
>                  return -EFAULT;
>          unsafe_get_user(val, from, Efault);
>    -     user_access_end();
>    +     user_read_access_end();
>          *dest = val;
>          return 0;
>     Efault:
> 
> I bet it does, but I'll wait for confirmation before actually
> committing that fix.
> 

You'll need the same change in the Efault leg.

Christophe