linux-kernel - Re: [PATCH v2 6/7] ptrace: introduce PTRACE_SET_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] [day] [month] [year] [list]

Message-ID: <20250120195640.GE7432@redhat.com>
Date: Mon, 20 Jan 2025 20:56:40 +0100
From: Oleg Nesterov <oleg@...hat.com>
To: "Dmitry V. Levin" <ldv@...ace.io>
Cc: 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-kernel@...r.kernel.org,
	linux-api@...r.kernel.org
Subject: Re: [PATCH v2 6/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO request

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.