linux-kernel - Re: [PATCH v5 1/7] mips: fix mips_get_syscall

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: <alpine.DEB.2.21.2502111530080.65342@angie.orcam.me.uk>
Date: Tue, 11 Feb 2025 18:30:33 +0000 (GMT)
From: "Maciej W. Rozycki" <macro@...am.me.uk>
To: "Dmitry V. Levin" <ldv@...ace.io>, 
    Thomas Bogendoerfer <tsbogend@...ha.franken.de>
cc: Andrew Morton <akpm@...ux-foundation.org>, Oleg Nesterov <oleg@...hat.com>, 
    Alexey Gladkov <legion@...nel.org>, 
    Eugene Syromyatnikov <evgsyr@...il.com>, 
    Mike Frysinger <vapier@...too.org>, Renzo Davoli <renzo@...unibo.it>, 
    Davide Berardi <berardi.dav@...il.com>, strace-devel@...ts.strace.io, 
    linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/7] mips: fix mips_get_syscall_arg() for o32

On Mon, 10 Feb 2025, Dmitry V. Levin wrote:

> The first assertion is fixed for mips o32 by using struct pt_regs.pad0
> instead of get_user() to obtain syscall arguments.  This approach works
> due to this piece in arch/mips/kernel/scall32-o32.S:

 I've had a look now and I can see what's going on here.

 The thing is we're trying to access another task's context and obviously 
dereferencing $sp obtained from it is not going to work via get_user(), 
because that just peeks at the current task's context.  It often does not 
crash, because the default user stack always gets assigned the same VMA, 
but it is pure luck which we wouldn't have if the stack was switched (via 
setcontext(3) or however) or say a non-default process's thread peeked at, 
and in any case irrelevant data is obtained just as observed with the test 
case.

 We ought to be using access_remote_vm() to retrieve the other task's 
stack contents, but given that the data has been already obtained and 
saved in `struct pt_regs' it would be an overkill.

 So I think your change is actually the correct thing to do, but please 
let's not abuse a struct member called `pad', the name of which indicates 
its contents are not supposed to be of any use.  I have therefore posted a 
preparatory cleanup[1].  May you please rebase your patch on top of that 
and also update the change description so as to reflect the findings?

 Thomas, can you please apply my cleanup soon and ahead of Dmitry's change 
so as to make things easy to proceed with?  Or otherwise let me know what 
works for you best.

 Also I have a suspicion this stuff ought to be backported, but I guess it 
can be decided later on.

 Thank you for your patience.

[1] "MIPS: Export syscall stack arguments properly for remote use", 
    <https://lore.kernel.org/linux-mips/alpine.DEB.2.21.2502101732120.65342@angie.orcam.me.uk/>

  Maciej