All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dmitry V. Levin" <ldv@altlinux.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	He Zhe <zhe.he@windriver.com>,
	oleg@redhat.com, linux-arm-kernel@lists.infradead.org,
	paul@paul-moore.com, eparis@redhat.com, linux-audit@redhat.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] arm64: ptrace: Add is_syscall_success to handle compat
Date: Thu, 22 Apr 2021 21:57:43 +0300	[thread overview]
Message-ID: <20210422185743.GC12226@altlinux.org> (raw)
In-Reply-To: <20210422164228.GD66392@C02TD0UTHF1T.local>

On Thu, Apr 22, 2021 at 05:42:28PM +0100, Mark Rutland wrote:
> On Thu, Apr 22, 2021 at 05:07:53PM +0100, Will Deacon wrote:
> > On Wed, Apr 21, 2021 at 06:10:05PM +0100, Mark Rutland wrote:
> > > On Mon, Apr 19, 2021 at 01:19:33PM +0100, Will Deacon wrote:
> > > > On Fri, Apr 16, 2021 at 02:34:41PM +0100, Mark Rutland wrote:
> > > > > I think this is a problem we created for ourselves back in commit:
> > > > > 
> > > > >   15956689a0e60aa0 ("arm64: compat: Ensure upper 32 bits of x0 are zero on syscall return)
> > > > > 
> > > > > AFAICT, the perf regs samples are the only place this matters, since for
> > > > > ptrace the compat regs are implicitly truncated to compat_ulong_t, and
> > > > > audit expects the non-truncated return value. Other architectures don't
> > > > > truncate here, so I think we're setting ourselves up for a game of
> > > > > whack-a-mole to truncate and extend wherever we need to.
> > > > > 
> > > > > Given that, I suspect it'd be better to do something like the below.
> > > > > 
> > > > > Will, thoughts?
> > > > 
> > > > I think perf is one example, but this is also visible to userspace via the
> > > > native ptrace interface and I distinctly remember needing this for some
> > > > versions of arm64 strace to work correctly when tracing compat tasks.
> > > 
> > > FWIW, you've convinced me on your approach (more on that below), but
> > > when I went digging here this didn't seem to be exposed via ptrace --
> > > for any task tracing a compat task, the GPRs are exposed via
> > > compat_gpr_{get,set}(), which always truncate to compat_ulong_t, giving
> > > the lower 32 bits. See task_user_regset_view() for where we get the
> > > regset.
> > > 
> > > Am I missing something, or are you thinking of another issue you fixed
> > > at the same time?
> > 
> > I think it may depend on whether strace pokes at the GPRs or instead issues
> > a PTRACE_GET_SYSCALL_INFO request but I've forgotten the details,
> > unfortunately. I do remember seeing an issue though, and it was only last
> > year.
> 
> Ah; I hadn't spotted PTRACE_GET_SYSCALL_INFO, thanks for the pointer. I
> see that gets at the regs via syscall_get_arguments(), which doesn't
> truncate them.
> 
> That makes me wonder whether x86 and others do the right thing here...

Yes, some architectures had to be fixed, but they mostly do the right
thing nowadays.

Feel free to use tools/testing/selftests/ptrace/get_syscall_info.c for
testing, or indeed use strace test suite.


-- 
ldv

WARNING: multiple messages have this Message-ID (diff)
From: "Dmitry V. Levin" <ldv@altlinux.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: He Zhe <zhe.he@windriver.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	oleg@redhat.com, linux-kernel@vger.kernel.org,
	linux-audit@redhat.com, Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/3] arm64: ptrace: Add is_syscall_success to handle compat
Date: Thu, 22 Apr 2021 21:57:43 +0300	[thread overview]
Message-ID: <20210422185743.GC12226@altlinux.org> (raw)
In-Reply-To: <20210422164228.GD66392@C02TD0UTHF1T.local>

On Thu, Apr 22, 2021 at 05:42:28PM +0100, Mark Rutland wrote:
> On Thu, Apr 22, 2021 at 05:07:53PM +0100, Will Deacon wrote:
> > On Wed, Apr 21, 2021 at 06:10:05PM +0100, Mark Rutland wrote:
> > > On Mon, Apr 19, 2021 at 01:19:33PM +0100, Will Deacon wrote:
> > > > On Fri, Apr 16, 2021 at 02:34:41PM +0100, Mark Rutland wrote:
> > > > > I think this is a problem we created for ourselves back in commit:
> > > > > 
> > > > >   15956689a0e60aa0 ("arm64: compat: Ensure upper 32 bits of x0 are zero on syscall return)
> > > > > 
> > > > > AFAICT, the perf regs samples are the only place this matters, since for
> > > > > ptrace the compat regs are implicitly truncated to compat_ulong_t, and
> > > > > audit expects the non-truncated return value. Other architectures don't
> > > > > truncate here, so I think we're setting ourselves up for a game of
> > > > > whack-a-mole to truncate and extend wherever we need to.
> > > > > 
> > > > > Given that, I suspect it'd be better to do something like the below.
> > > > > 
> > > > > Will, thoughts?
> > > > 
> > > > I think perf is one example, but this is also visible to userspace via the
> > > > native ptrace interface and I distinctly remember needing this for some
> > > > versions of arm64 strace to work correctly when tracing compat tasks.
> > > 
> > > FWIW, you've convinced me on your approach (more on that below), but
> > > when I went digging here this didn't seem to be exposed via ptrace --
> > > for any task tracing a compat task, the GPRs are exposed via
> > > compat_gpr_{get,set}(), which always truncate to compat_ulong_t, giving
> > > the lower 32 bits. See task_user_regset_view() for where we get the
> > > regset.
> > > 
> > > Am I missing something, or are you thinking of another issue you fixed
> > > at the same time?
> > 
> > I think it may depend on whether strace pokes at the GPRs or instead issues
> > a PTRACE_GET_SYSCALL_INFO request but I've forgotten the details,
> > unfortunately. I do remember seeing an issue though, and it was only last
> > year.
> 
> Ah; I hadn't spotted PTRACE_GET_SYSCALL_INFO, thanks for the pointer. I
> see that gets at the regs via syscall_get_arguments(), which doesn't
> truncate them.
> 
> That makes me wonder whether x86 and others do the right thing here...

Yes, some architectures had to be fixed, but they mostly do the right
thing nowadays.

Feel free to use tools/testing/selftests/ptrace/get_syscall_info.c for
testing, or indeed use strace test suite.


-- 
ldv

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


WARNING: multiple messages have this Message-ID (diff)
From: "Dmitry V. Levin" <ldv@altlinux.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	He Zhe <zhe.he@windriver.com>,
	oleg@redhat.com, linux-arm-kernel@lists.infradead.org,
	paul@paul-moore.com, eparis@redhat.com, linux-audit@redhat.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] arm64: ptrace: Add is_syscall_success to handle compat
Date: Thu, 22 Apr 2021 21:57:43 +0300	[thread overview]
Message-ID: <20210422185743.GC12226@altlinux.org> (raw)
In-Reply-To: <20210422164228.GD66392@C02TD0UTHF1T.local>

On Thu, Apr 22, 2021 at 05:42:28PM +0100, Mark Rutland wrote:
> On Thu, Apr 22, 2021 at 05:07:53PM +0100, Will Deacon wrote:
> > On Wed, Apr 21, 2021 at 06:10:05PM +0100, Mark Rutland wrote:
> > > On Mon, Apr 19, 2021 at 01:19:33PM +0100, Will Deacon wrote:
> > > > On Fri, Apr 16, 2021 at 02:34:41PM +0100, Mark Rutland wrote:
> > > > > I think this is a problem we created for ourselves back in commit:
> > > > > 
> > > > >   15956689a0e60aa0 ("arm64: compat: Ensure upper 32 bits of x0 are zero on syscall return)
> > > > > 
> > > > > AFAICT, the perf regs samples are the only place this matters, since for
> > > > > ptrace the compat regs are implicitly truncated to compat_ulong_t, and
> > > > > audit expects the non-truncated return value. Other architectures don't
> > > > > truncate here, so I think we're setting ourselves up for a game of
> > > > > whack-a-mole to truncate and extend wherever we need to.
> > > > > 
> > > > > Given that, I suspect it'd be better to do something like the below.
> > > > > 
> > > > > Will, thoughts?
> > > > 
> > > > I think perf is one example, but this is also visible to userspace via the
> > > > native ptrace interface and I distinctly remember needing this for some
> > > > versions of arm64 strace to work correctly when tracing compat tasks.
> > > 
> > > FWIW, you've convinced me on your approach (more on that below), but
> > > when I went digging here this didn't seem to be exposed via ptrace --
> > > for any task tracing a compat task, the GPRs are exposed via
> > > compat_gpr_{get,set}(), which always truncate to compat_ulong_t, giving
> > > the lower 32 bits. See task_user_regset_view() for where we get the
> > > regset.
> > > 
> > > Am I missing something, or are you thinking of another issue you fixed
> > > at the same time?
> > 
> > I think it may depend on whether strace pokes at the GPRs or instead issues
> > a PTRACE_GET_SYSCALL_INFO request but I've forgotten the details,
> > unfortunately. I do remember seeing an issue though, and it was only last
> > year.
> 
> Ah; I hadn't spotted PTRACE_GET_SYSCALL_INFO, thanks for the pointer. I
> see that gets at the regs via syscall_get_arguments(), which doesn't
> truncate them.
> 
> That makes me wonder whether x86 and others do the right thing here...

Yes, some architectures had to be fixed, but they mostly do the right
thing nowadays.

Feel free to use tools/testing/selftests/ptrace/get_syscall_info.c for
testing, or indeed use strace test suite.


-- 
ldv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-04-22 18:57 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-16  7:55 [PATCH 1/3] arm64: ptrace: Add is_syscall_success to handle compat He Zhe
2021-04-16  7:55 ` He Zhe
2021-04-16  7:55 ` He Zhe
2021-04-16  7:55 ` [PATCH 2/3] arm64: syscall.h: Add sign extension handling in syscall_get_return_value for compat He Zhe
2021-04-16  7:55   ` He Zhe
2021-04-16  7:55   ` He Zhe
2021-04-16  9:43   ` Oleg Nesterov
2021-04-16  9:43     ` Oleg Nesterov
2021-04-16  9:43     ` Oleg Nesterov
2021-04-20  8:38     ` He Zhe
2021-04-20  8:38       ` He Zhe
2021-04-20  8:38       ` He Zhe
2021-04-21 17:41   ` Mark Rutland
2021-04-21 17:41     ` Mark Rutland
2021-04-21 17:41     ` Mark Rutland
2021-04-22 16:55     ` Mark Rutland
2021-04-22 16:55       ` Mark Rutland
2021-04-22 16:55       ` Mark Rutland
2021-04-16  7:55 ` [PATCH 3/3] audit: Use syscall_get_return_value to get syscall return code in audit_syscall_exit He Zhe
2021-04-16  7:55   ` He Zhe
2021-04-16  7:55   ` He Zhe
2021-04-16 12:33 ` [PATCH 1/3] arm64: ptrace: Add is_syscall_success to handle compat Catalin Marinas
2021-04-16 12:33   ` Catalin Marinas
2021-04-16 12:33   ` Catalin Marinas
2021-04-16 13:34   ` Mark Rutland
2021-04-16 13:34     ` Mark Rutland
2021-04-16 13:34     ` Mark Rutland
2021-04-17 13:19     ` David Laight
2021-04-17 13:19       ` David Laight
2021-04-17 13:19       ` David Laight
2021-04-19 12:19     ` Will Deacon
2021-04-19 12:19       ` Will Deacon
2021-04-19 12:19       ` Will Deacon
2021-04-20  8:54       ` He Zhe
2021-04-20  8:54         ` He Zhe
2021-04-20  8:54         ` He Zhe
2021-04-21 17:10       ` Mark Rutland
2021-04-21 17:10         ` Mark Rutland
2021-04-21 17:10         ` Mark Rutland
2021-04-22 16:07         ` Will Deacon
2021-04-22 16:07           ` Will Deacon
2021-04-22 16:07           ` Will Deacon
2021-04-22 16:42           ` Mark Rutland
2021-04-22 16:42             ` Mark Rutland
2021-04-22 16:42             ` Mark Rutland
2021-04-22 18:57             ` Dmitry V. Levin [this message]
2021-04-22 18:57               ` Dmitry V. Levin
2021-04-22 18:57               ` Dmitry V. Levin
2021-04-20  8:42   ` He Zhe
2021-04-20  8:42     ` He Zhe
2021-04-20  8:42     ` He Zhe
2021-04-21 17:17     ` Mark Rutland
2021-04-21 17:17       ` Mark Rutland
2021-04-21 17:17       ` Mark Rutland

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=20210422185743.GC12226@altlinux.org \
    --to=ldv@altlinux.org \
    --cc=catalin.marinas@arm.com \
    --cc=eparis@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=oleg@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=will@kernel.org \
    --cc=zhe.he@windriver.com \
    /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.