[PATCH] parisc: Correct completer in lws start

[PATCH] parisc: Correct completer in lws start

[John David Anglin]

Correct completer in lws start.

The completer in the "or,ev   %r1,%r30,%r30" instruction is reversed, so we are
not clipping the LWS number when we are called from a 32-bit process (W=0). We
need to nulify the depdi instruction when the least-significant bit of %r30 is 1.

Signed-off-by: John David Anglin <dave.anglin@bell.net>
---

diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S
index d2497b339d13..65c88ca7a7ac 100644
--- a/arch/parisc/kernel/syscall.S
+++ b/arch/parisc/kernel/syscall.S
@@ -472,7 +472,7 @@ lws_start:
  	extrd,u	%r1,PSW_W_BIT,1,%r1
  	/* sp must be aligned on 4, so deposit the W bit setting into
  	 * the bottom of sp temporarily */
-	or,ev	%r1,%r30,%r30
+	or,od	%r1,%r30,%r30

  	/* Clip LWS number to a 32-bit value for 32-bit processes */
  	depdi	0, 31, 32, %r20

Re: [PATCH] parisc: Correct completer in lws start

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]

Am Dienstag, 21. Dezember 2021, 19:21:22 CET schrieb John David Anglin:
> Correct completer in lws start.
> 
> The completer in the "or,ev   %r1,%r30,%r30" instruction is reversed, so we
> are not clipping the LWS number when we are called from a 32-bit process
> (W=0). We need to nulify the depdi instruction when the least-significant
> bit of %r30 is 1.

I'm curious: what effect has this bug? Since this is syscall code I guess it 
can somehow be exposed from userspace, but how?

Maybe some sort of explanation like this can even be added to the commit 
message?

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] parisc: Correct completer in lws start

[John David Anglin]

On 2021-12-21 1:29 p.m., Rolf Eike Beer wrote:
> Am Dienstag, 21. Dezember 2021, 19:21:22 CET schrieb John David Anglin:
>> Correct completer in lws start.
>>
>> The completer in the "or,ev   %r1,%r30,%r30" instruction is reversed, so we
>> are not clipping the LWS number when we are called from a 32-bit process
>> (W=0). We need to nulify the depdi instruction when the least-significant
>> bit of %r30 is 1.
> I'm curious: what effect has this bug? Since this is syscall code I guess it
> can somehow be exposed from userspace, but how?
I believe the LWS code will branch to an incorrect location in the kernel if the value is not clipped
on a 64-bit kernel.

#ifdef CONFIG_64BIT
         ssm     PSW_SM_W, %r1
         extrd,u %r1,PSW_W_BIT,1,%r1
         /* sp must be aligned on 4, so deposit the W bit setting into
          * the bottom of sp temporarily */
         or,od   %r1,%r30,%r30

         /* Clip LWS number to a 32-bit value for 32-bit processes */
         depdi   0, 31, 32, %r20
#endif

         /* Is the lws entry number valid? */
         comiclr,>>      __NR_lws_entries, %r20, %r0
         b,n     lws_exit_nosys

         /* Load table start */
         ldil    L%lws_table, %r1
         ldo     R%lws_table(%r1), %r28  /* Scratch use of r28 */
         LDREGX  %r20(%sr2,r28), %r21    /* Scratch use of r21 */

         /* Jump to lws, lws table pointers already relocated */
         be,n    0(%sr2,%r21)

Note that the comiclr instruction only checks the least significant 32 bits, but the LDREGX
instruction uses all 64 bits of %r20.

You can see how gcc setups up %r20 in linux-atomic.c.  On PA 2.0 machines, there are ways
for 32-bit processes to set the most significant 32 bits in a register.  So, a user process could
crash the machine if it deliberately did a LWS call with the upper 32-bits of %r20 nonzero.

Currently, only glibc and gcc generate LWS calls.

Dave

-- 
John David Anglin  dave.anglin@bell.net