All of lore.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Andrei Vagin <avagin@gmail.com>
Cc: Will Deacon <will@kernel.org>, Oleg Nesterov <oleg@redhat.com>,
	linux-arm-kernel@lists.infradead.org,
	LKML <linux-kernel@vger.kernel.org>,
	Dave Martin <Dave.Martin@arm.com>,
	Keno Fischer <keno@juliacomputing.com>
Subject: Re: [PATCH 1/4] arm64: expose orig_x0 in the user_pt_regs structure
Date: Sat, 27 Mar 2021 13:01:01 +0000	[thread overview]
Message-ID: <20210327130100.GA31076@arm.com> (raw)
In-Reply-To: <CANaxB-xTrMkJ2D19wwjSbCfizdRDnNO52fibpaRXP5g3hOcQwQ@mail.gmail.com>

On Fri, Mar 26, 2021 at 05:35:19PM -0700, Andrei Vagin wrote:
> On Fri, Mar 26, 2021 at 11:28 AM Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> >
> > On Mon, Mar 22, 2021 at 03:50:50PM -0700, Andrei Vagin wrote:
> > > diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
> > > index 758ae984ff97..3c118c5b0893 100644
> > > --- a/arch/arm64/include/uapi/asm/ptrace.h
> > > +++ b/arch/arm64/include/uapi/asm/ptrace.h
> > > @@ -90,6 +90,7 @@ struct user_pt_regs {
> > >       __u64           sp;
> > >       __u64           pc;
> > >       __u64           pstate;
> > > +     __u64           orig_x0;
> > >  };
> >
> > That's a UAPI change, likely to go wrong. For example, a
> > ptrace(PTRACE_GETREGSET, pid, REGSET_GPR, data) would write past the end
> > of an old struct user_pt_regs in the debugger.
> 
> ptrace(PTRACE_GETREGSET, ...) receives iovec:
> ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov)
> 
> iov contains a pointer to a buffer and its size and the kernel fills
> only the part that fits the buffer.
> I think this interface was invented to allow extending structures
> without breaking backward compatibility.

You are right here, it doesn't write past the end of the iov buffer.
However, it's still an ABI change. An unaware program using a newer
user_pt_regs but running on an older kernel may be surprised that the
updated iov.len is smaller than sizeof (struct user_pt_regs).

Changing this structure also changes the core dump format, see ELF_NGREG
and ELF_CORE_COPY_REGS. Maybe this doesn't matter much either since the
ELF note would have size information but I'd prefer if we didn't modify
this structure.

-- 
Catalin

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

WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: Andrei Vagin <avagin@gmail.com>
Cc: Will Deacon <will@kernel.org>, Oleg Nesterov <oleg@redhat.com>,
	linux-arm-kernel@lists.infradead.org,
	LKML <linux-kernel@vger.kernel.org>,
	Dave Martin <Dave.Martin@arm.com>,
	Keno Fischer <keno@juliacomputing.com>
Subject: Re: [PATCH 1/4] arm64: expose orig_x0 in the user_pt_regs structure
Date: Sat, 27 Mar 2021 13:01:01 +0000	[thread overview]
Message-ID: <20210327130100.GA31076@arm.com> (raw)
In-Reply-To: <CANaxB-xTrMkJ2D19wwjSbCfizdRDnNO52fibpaRXP5g3hOcQwQ@mail.gmail.com>

On Fri, Mar 26, 2021 at 05:35:19PM -0700, Andrei Vagin wrote:
> On Fri, Mar 26, 2021 at 11:28 AM Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> >
> > On Mon, Mar 22, 2021 at 03:50:50PM -0700, Andrei Vagin wrote:
> > > diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
> > > index 758ae984ff97..3c118c5b0893 100644
> > > --- a/arch/arm64/include/uapi/asm/ptrace.h
> > > +++ b/arch/arm64/include/uapi/asm/ptrace.h
> > > @@ -90,6 +90,7 @@ struct user_pt_regs {
> > >       __u64           sp;
> > >       __u64           pc;
> > >       __u64           pstate;
> > > +     __u64           orig_x0;
> > >  };
> >
> > That's a UAPI change, likely to go wrong. For example, a
> > ptrace(PTRACE_GETREGSET, pid, REGSET_GPR, data) would write past the end
> > of an old struct user_pt_regs in the debugger.
> 
> ptrace(PTRACE_GETREGSET, ...) receives iovec:
> ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov)
> 
> iov contains a pointer to a buffer and its size and the kernel fills
> only the part that fits the buffer.
> I think this interface was invented to allow extending structures
> without breaking backward compatibility.

You are right here, it doesn't write past the end of the iov buffer.
However, it's still an ABI change. An unaware program using a newer
user_pt_regs but running on an older kernel may be surprised that the
updated iov.len is smaller than sizeof (struct user_pt_regs).

Changing this structure also changes the core dump format, see ELF_NGREG
and ELF_CORE_COPY_REGS. Maybe this doesn't matter much either since the
ELF note would have size information but I'd prefer if we didn't modify
this structure.

-- 
Catalin

  reply	other threads:[~2021-03-27 13:03 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-22 22:50 [PATCH 0/4 v3] arm64/ptrace: allow to get all registers on syscall traps Andrei Vagin
2021-03-22 22:50 ` Andrei Vagin
2021-03-22 22:50 ` [PATCH 1/4] arm64: expose orig_x0 in the user_pt_regs structure Andrei Vagin
2021-03-22 22:50   ` Andrei Vagin
2021-03-26 18:28   ` Catalin Marinas
2021-03-26 18:28     ` Catalin Marinas
2021-03-27  0:35     ` Andrei Vagin
2021-03-27  0:35       ` Andrei Vagin
2021-03-27 13:01       ` Catalin Marinas [this message]
2021-03-27 13:01         ` Catalin Marinas
2021-03-22 22:50 ` [PATCH 2/4] arm64/ptrace: introduce orig_x7 " Andrei Vagin
2021-03-22 22:50   ` Andrei Vagin
2021-03-26 18:39   ` Catalin Marinas
2021-03-26 18:39     ` Catalin Marinas
2021-03-22 22:50 ` [PATCH 3/4] selftest/arm64/ptrace: add a test for orig_x0 Andrei Vagin
2021-03-22 22:50   ` Andrei Vagin
2021-03-22 22:50 ` [PATCH 4/4] selftest/arm64/ptrace: add a test for orig_x7 Andrei Vagin
2021-03-22 22:50   ` Andrei Vagin

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=20210327130100.GA31076@arm.com \
    --to=catalin.marinas@arm.com \
    --cc=Dave.Martin@arm.com \
    --cc=avagin@gmail.com \
    --cc=keno@juliacomputing.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=will@kernel.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.