All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: "Dmitry V. Levin" <ldv@strace.io>
Cc: 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-kernel@vger.kernel.org,
	linux-api@vger.kernel.org
Subject: Re: [PATCH v2 6/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO request
Date: Mon, 20 Jan 2025 20:56:40 +0100	[thread overview]
Message-ID: <20250120195640.GE7432@redhat.com> (raw)
In-Reply-To: <20250119124427.GA3487@strace.io>

On 01/19, Dmitry V. Levin wrote:
>
> On Sat, Jan 18, 2025 at 03:13:42PM +0100, Oleg Nesterov wrote:
> > On 01/17, Dmitry V. Levin wrote:
> [...]
> > > For example, on x86_64 sizeof(struct ptrace_syscall_info) is currently 88,
> > > while on x86 it is 84.
> >
> > Not good, but too late to complain...
>
> Actually, I don't think it's too late to add an extra __u32 padding
> there since it wouldn't affect PTRACE_GET_SYSCALL_INFO.

Hmm, indeed thanks for correcting me. I forgot that ptrace_get_syscall_info()
returns actual_size, not sizeof().

> I can add an explicit padding to the structure if you say
> you like it better this way.

I dunno, up to you...

Well if we add "__u32 padding" at the end, we can probably use sizeof(info)
instead of min_size = offsetofend(struct ptrace_syscall_info, seccomp.ret_data)
in ptrace_set_syscall_info(), but then it probably makes sense to check
info->padding == 0 (just like info.flags || info.reserved) and rename this
member to reserved2.

Again, up to you, I don't know.

> > Currently we have PTRACE_SYSCALL_INFO_SIZE_VER0, when we add the new
> > "artificial" member we will have PTRACE_SYSCALL_INFO_SIZE_VER1. Granted,
> > this way set_syscall_info() can't use sizeof(info), it should do
> >
> > 	ptrace(PTRACE_SET_SYSCALL_INFO, PTRACE_SYSCALL_INFO_SIZE_VER1, info);
> >
> > and the kernel needs more checks, but this is what I had in mind when I said
> > that the 1st version can just require "user_size == PTRACE_SYSCALL_INFO_SIZE_VER0".
>
> ... it wouldn't be a big deal for user-space to specify also an
> appropriate "user_size", e.g. PTRACE_SYSCALL_INFO_SIZE_VER1 when it starts
> using the interface available since VER1, but it wouldn't help user-space
> programs either as they would have to update "op" and/or "flags" anyway,

Sure, and yes, "flags" is needed anyway.

> and "user_size" would become just yet another detail they have to care
> about.

True.

It is not that I ever thought that my suggestion could "help user-space".
Not at all. Just imo it would be better to fail "early" on the older kernel
in the case when user-space expects the "extended" API, even if flags == 0.
And no, it is not that I am 100% sure it would be always better.

So let me repeat: please do what you think is right, I won't argue. I just
tried to understand your points and explain mine to ensure we more or less
understand each other.

Oleg.


  reply	other threads:[~2025-01-20 19:57 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20250113170925.GA392@strace.io>
2025-01-13 17:10 ` [PATCH v2 1/7] powerpc: properly negate error in syscall_set_return_value() Dmitry V. Levin
2025-01-13 17:34   ` Christophe Leroy
2025-01-13 17:54     ` Dmitry V. Levin
2025-01-14 17:04     ` Dmitry V. Levin
2025-01-20 13:51       ` Christophe Leroy
2025-01-20 17:12         ` Dmitry V. Levin
2025-01-21 11:13           ` Madhavan Srinivasan
2025-01-21 11:28             ` Christophe Leroy
2025-01-21 12:25               ` Madhavan Srinivasan
2025-01-21 12:42                 ` Dmitry V. Levin
2025-01-23 18:28         ` Dmitry V. Levin
2025-01-23 19:11           ` Eugene Syromyatnikov
2025-01-23 22:16             ` Dmitry V. Levin
2025-01-23 22:07           ` Christophe Leroy
2025-01-23 22:35             ` Dmitry V. Levin
2025-01-27 11:20             ` Dmitry V. Levin
2025-01-27 11:36               ` Christophe Leroy
2025-01-27 11:44                 ` Dmitry V. Levin
2025-01-27 12:04                   ` Christophe Leroy
2025-01-27 12:26                     ` Dmitry V. Levin
2025-01-23 23:43           ` Dmitry V. Levin
2025-01-24 15:18             ` Alexey Gladkov
2025-01-25  0:25               ` Dmitry V. Levin
2025-01-25 12:18               ` Michael Ellerman
2025-01-27 11:13                 ` Dmitry V. Levin
2025-01-25 12:17             ` Michael Ellerman
2025-01-25 20:48               ` Dmitry V. Levin
2025-01-25 12:17           ` Michael Ellerman
2025-01-25 21:25             ` Dmitry V. Levin
2025-01-14 13:00   ` Alexey Gladkov
2025-01-14 13:48     ` Dmitry V. Levin
2025-01-14 14:53       ` Alexey Gladkov
2025-01-13 17:11 ` [PATCH v2 2/7] mips: fix mips_get_syscall_arg() for O32 and N32 Dmitry V. Levin
2025-01-14  3:29   ` Maciej W. Rozycki
2025-01-14  8:47     ` Dmitry V. Levin
2025-01-14 16:03       ` Maciej W. Rozycki
2025-01-14 16:42         ` Dmitry V. Levin
2025-01-13 17:11 ` [PATCH v2 3/7] syscall.h: add syscall_set_arguments() and syscall_set_return_value() Dmitry V. Levin
2025-01-13 17:11   ` Dmitry V. Levin
2025-01-13 17:11   ` Dmitry V. Levin
2025-01-16  2:20   ` Charlie Jenkins
2025-01-16  2:20     ` Charlie Jenkins
2025-01-16  2:20     ` Charlie Jenkins
2025-01-17  0:59     ` H. Peter Anvin
2025-01-17  0:59       ` H. Peter Anvin
2025-01-17  0:59       ` H. Peter Anvin
2025-01-17 15:45       ` Eugene Syromyatnikov
2025-01-17 15:45         ` Eugene Syromyatnikov
2025-01-17 15:45         ` Eugene Syromyatnikov
2025-01-18  4:34         ` H. Peter Anvin
2025-01-18  4:34           ` H. Peter Anvin
2025-01-18  4:34           ` H. Peter Anvin
2025-01-13 17:11 ` [PATCH v2 4/7] syscall.h: introduce syscall_set_nr() Dmitry V. Levin
2025-01-13 17:11   ` Dmitry V. Levin
2025-01-13 17:11   ` Dmitry V. Levin
2025-01-16  2:20   ` Charlie Jenkins
2025-01-16  2:20     ` Charlie Jenkins
2025-01-16  2:20     ` Charlie Jenkins
2025-01-13 17:12 ` [PATCH v2 5/7] ptrace_get_syscall_info: factor out ptrace_get_syscall_info_op Dmitry V. Levin
2025-01-13 17:12 ` [PATCH v2 6/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO request Dmitry V. Levin
2025-01-15 16:38   ` Oleg Nesterov
2025-01-15 17:36     ` Dmitry V. Levin
2025-01-15 19:10       ` Oleg Nesterov
2025-01-16  1:55   ` Charlie Jenkins
2025-01-16  8:33     ` Dmitry V. Levin
2025-01-16 21:07       ` Charlie Jenkins
2025-01-16 21:47         ` Charlie Jenkins
2025-01-16 15:21   ` Oleg Nesterov
2025-01-16 16:04     ` Dmitry V. Levin
2025-01-16 16:40       ` Dmitry V. Levin
2025-01-17 14:45       ` Oleg Nesterov
2025-01-17 15:06         ` Dmitry V. Levin
2025-01-17 15:32           ` Oleg Nesterov
2025-01-17 16:22             ` Dmitry V. Levin
2025-01-18 14:13               ` Oleg Nesterov
2025-01-19 12:44                 ` Dmitry V. Levin
2025-01-20 19:56                   ` Oleg Nesterov [this message]
2025-01-19 14:38                 ` Aleksa Sarai
2025-01-13 17:12 ` [PATCH v2 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=20250120195640.GE7432@redhat.com \
    --to=oleg@redhat.com \
    --cc=berardi.dav@gmail.com \
    --cc=evgsyr@gmail.com \
    --cc=ldv@strace.io \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=renzo@cs.unibo.it \
    --cc=strace-devel@lists.strace.io \
    --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.