All of lore.kernel.org
 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: 16+ 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:33 ` Dmitry V. Levin
2025-02-10 11:33 ` 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:34   ` Dmitry V. Levin
2025-02-10 11:34   ` 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   ` Dmitry V. Levin
2025-02-10 11:35   ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.