linux-kernel - Re: [PATCH v11 2/2] ptrace,syscall_user_dispatch: checkpoint/restore support for SUD

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y/eG4oHc2jz6uSfi@memverge.com>
Date:   Thu, 23 Feb 2023 10:31:46 -0500
From:   Gregory Price <gregory.price@...verge.com>
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     Gregory Price <gourry.memverge@...il.com>,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
        avagin@...il.com, peterz@...radead.org, luto@...nel.org,
        krisman@...labora.com, tglx@...utronix.de, corbet@....net,
        shuah@...nel.org
Subject: Re: [PATCH v11 2/2] ptrace,syscall_user_dispatch: checkpoint/restore
 support for SUD

On Thu, Feb 23, 2023 at 01:30:20PM +0100, Oleg Nesterov wrote:
> On 02/22, Gregory Price wrote:
> >
> > On Wed, Feb 22, 2023 at 01:48:35PM +0100, Oleg Nesterov wrote:
> > > On 02/21, Gregory Price wrote:
> > > >
> > > > +struct ptrace_sud_config {
> > > > +	__u8  mode;
> > > > +	__u8  pad[7];
> > >               ^^^^^^
> > > Why?
> > >
> >
> > The struct isn't packed, so this is for alignment/consistency of size.
> > The padding gets added anyway, and differently between 32/64 bit.
> 
> OK, I have to admit I didn't know that alignof(long long) == 4 on 32 bit.
> 
> > > > +int syscall_user_dispatch_get_config(struct task_struct *task, unsigned long size,
> > > > +		                     void __user *data)
> > > > +{
> > > > +	struct syscall_user_dispatch *sd = &task->syscall_dispatch;
> > > > +	struct ptrace_sud_config config;
> > > > +	if (size != sizeof(struct ptrace_sud_config))
> > > > +		return -EINVAL;
> > >
> > > Andrei, do we really need this check?
> > >
> >
> > My understanding is that it's a sanity check against the above issue.
> > In fact, it was what lead me to add the padding.
> 
> Well, if this is the only reason then this check and the "size" argument
> ahould be removed, imo.
> 
> But perhaps it can be useful for future extensions, I dunno.
> 
> Oleg.
>

I suppose yes it could also be used to detect differences in versioning
if the struct changes in the future, and that would not require an API
change in the future to support it.

If something does change in the future, without the check you're kinda
SOL trying to add new fields.

~Gregory