mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Dmitry V. Levin" <ldv@strace.io>
To: "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Alexey Gladkov <legion@kernel.org>,
	Eugene Syromyatnikov <evgsyr@gmail.com>,
	Mike Frysinger <vapier@gentoo.org>,
	Renzo Davoli <renzo@cs.unibo.it>,
	Davide Berardi <berardi.dav@gmail.com>,
	strace-devel@lists.strace.io, linux-mips@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 1/7] mips: fix mips_get_syscall_arg() for o32
Date: Wed, 12 Feb 2025 00:03:33 +0200	[thread overview]
Message-ID: <20250211220333.GA19256@strace.io> (raw)
In-Reply-To: <alpine.DEB.2.21.2502111530080.65342@angie.orcam.me.uk>

On Tue, Feb 11, 2025 at 06:30:33PM +0000, Maciej W. Rozycki wrote:
> 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/>

Thanks for the analysis, I'm going to rebase my fix and send it
as a follow-up to your cleanup patch.


-- 
ldv

  reply	other threads:[~2025-02-11 22:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-10 11:33 [PATCH v5 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API Dmitry V. Levin
2025-02-10 11:34 ` [PATCH v5 1/7] mips: fix mips_get_syscall_arg() for o32 Dmitry V. Levin
2025-02-11 18:30   ` Maciej W. Rozycki
2025-02-11 22:03     ` Dmitry V. Levin [this message]
2025-02-10 11:34 ` [PATCH v5 2/7] hexagon: add syscall_set_return_value() Dmitry V. Levin
2025-02-10 11:34 ` [PATCH v5 3/7] syscall.h: add syscall_set_arguments() Dmitry V. Levin
2025-02-10 11:35 ` [PATCH v5 4/7] syscall.h: introduce syscall_set_nr() Dmitry V. Levin
2025-02-10 11:35 ` [PATCH v5 5/7] ptrace_get_syscall_info: factor out ptrace_get_syscall_info_op Dmitry V. Levin
2025-02-10 11:35 ` [PATCH v5 6/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO request Dmitry V. Levin
2025-02-10 11:35 ` [PATCH v5 7/7] selftests/ptrace: add a test case for PTRACE_SET_SYSCALL_INFO Dmitry V. Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250211220333.GA19256@strace.io \
    --to=ldv@strace.io \
    --cc=akpm@linux-foundation.org \
    --cc=berardi.dav@gmail.com \
    --cc=evgsyr@gmail.com \
    --cc=legion@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=macro@orcam.me.uk \
    --cc=oleg@redhat.com \
    --cc=renzo@cs.unibo.it \
    --cc=strace-devel@lists.strace.io \
    --cc=tsbogend@alpha.franken.de \
    --cc=vapier@gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®