All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Amit Kachhap <amit.kachhap@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Kees Cook <keescook@chromium.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Kristina Martsenko <kristina.martsenko@arm.com>,
	Mark Brown <broonie@kernel.org>,
	James Morse <james.morse@arm.com>,
	Vincenzo Frascino <Vincenzo.Frascino@arm.com>,
	Dave Martin <Dave.Martin@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/2] arm64: kprobe: disable probe of fault prone ptrauth instruction
Date: Fri, 28 Feb 2020 11:18:59 +0000	[thread overview]
Message-ID: <20200228111856.GB2941@willie-the-truck> (raw)
In-Reply-To: <52db2533-488a-1e27-947c-ec92048f26ae@arm.com>

On Fri, Feb 28, 2020 at 04:42:10PM +0530, Amit Kachhap wrote:
> On 2/27/20 10:18 PM, Mark Rutland wrote:
> > On Wed, Feb 19, 2020 at 06:30:40PM +0530, Amit Daniel Kachhap wrote:
> > > diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> > > index 4a9e773..87f7c8a 100644
> > > --- a/arch/arm64/kernel/insn.c
> > > +++ b/arch/arm64/kernel/insn.c
> > > @@ -63,6 +63,7 @@ bool __kprobes aarch64_insn_is_nop(u32 insn)
> > >   	case AARCH64_INSN_HINT_WFI:
> > >   	case AARCH64_INSN_HINT_SEV:
> > >   	case AARCH64_INSN_HINT_SEVL:
> > > +	case AARCH64_INSN_HINT_AUTIASP:
> > >   		return false;
> > >   	default:
> > >   		return true;
> > 
> > I'm afraid that the existing code here is simply wrong, and this is
> > adding to the mess.
> > 
> > We have no idea what HINT space instructions will be in the future, so
> > the only sensible implementations of aarch64_insn_is_nop() are something
> > like:
> > 
> > bool __kprobes aarch64_insn_is_nop(u32 insn)
> > {
> > 	return insn == aarch64_insn_gen_hint(AARCH64_INSN_HINT_NOP);
> > }
> > 
> > ... and if we want to check for other HINT instructions, they should be
> > checked explicitly.
> > 
> > Can you please change aarch64_insn_is_nop() as above?
> 
> Agree that current implementation is unclear.
> I will implement as you suggested.

Well, please don't throw the baby out with the bath water. The existing
code might be badly structured, but I think it's going a bit far to say
it's "simply wrong". We need a whitelist /somewhere/ and I'd prefer not
to regress the existing behaviour just because the whitelist is embedded
in a function with "is_nop" in the name.

Will

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

  reply	other threads:[~2020-02-28 11:19 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-19 13:00 [PATCH 0/2] arm64: add Armv8.6 pointer authentication Amit Daniel Kachhap
2020-02-19 13:00 ` [PATCH 1/2] arm64: ptrauth: add pointer authentication Armv8.6 enhanced feature Amit Daniel Kachhap
2020-02-28 11:57   ` Will Deacon
2020-02-28 12:03     ` Mark Rutland
2020-02-28 12:23       ` Will Deacon
2020-03-02 12:48         ` Amit Kachhap
2020-03-02 16:29           ` Will Deacon
2020-02-19 13:00 ` [PATCH 2/2] arm64: kprobe: disable probe of fault prone ptrauth instruction Amit Daniel Kachhap
2020-02-27 16:48   ` Mark Rutland
2020-02-28 11:12     ` Amit Kachhap
2020-02-28 11:18       ` Will Deacon [this message]
2020-02-28 11:31         ` 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=20200228111856.GB2941@willie-the-truck \
    --to=will@kernel.org \
    --cc=Dave.Martin@arm.com \
    --cc=Vincenzo.Frascino@arm.com \
    --cc=amit.kachhap@arm.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=james.morse@arm.com \
    --cc=keescook@chromium.org \
    --cc=kristina.martsenko@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=suzuki.poulose@arm.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.