linux-kernel - Re: [PATCH][RFC] jump_labels/x86: Use either 5 byte or 2 byte jumps

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]

Date:	Fri, 07 Oct 2011 15:58:20 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	Richard Henderson <rth@...hat.com>,
	Jason Baron <jbaron@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"David S. Miller" <davem@...emloft.net>,
	David Daney <david.daney@...ium.com>,
	Michael Ellerman <michael@...erman.id.au>,
	Jan Glauber <jang@...ux.vnet.ibm.com>,
	the arch/x86 maintainers <x86@...nel.org>,
	Xen Devel <xen-devel@...ts.xensource.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@...rix.com>,
	peterz@...radead.org
Subject: Re: [PATCH][RFC] jump_labels/x86: Use either 5 byte or 2 byte jumps

On Fri, 2011-10-07 at 12:40 -0700, Jeremy Fitzhardinge wrote:
> On 10/07/2011 10:09 AM, Steven Rostedt wrote:
> > diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
> > index 3fee346..1f7f88f 100644
> > --- a/arch/x86/kernel/jump_label.c
> > +++ b/arch/x86/kernel/jump_label.c
> > @@ -16,34 +16,75 @@
> >  
> >  #ifdef HAVE_JUMP_LABEL
> >  
> > +static unsigned char nop_short[] = { P6_NOP2 };
> > +
> >  union jump_code_union {
> >  	char code[JUMP_LABEL_NOP_SIZE];
> >  	struct {
> >  		char jump;
> >  		int offset;
> >  	} __attribute__((packed));
> > +	struct {
> > +		char jump_short;
> > +		char offset_short;
> > +	} __attribute__((packed));
> >  };
> >  
> >  void arch_jump_label_transform(struct jump_entry *entry,
> >  			       enum jump_label_type type)
> >  {
> >  	union jump_code_union code;
> > +	unsigned char op;
> > +	unsigned size;
> > +	unsigned char nop;
> > +
> > +	/* Use probe_kernel_read()? */
> > +	op = *(unsigned char *)entry->code;
> > +	nop = ideal_nops[NOP_ATOMIC5][0];
> >  
> >  	if (type == JUMP_LABEL_ENABLE) {
> > -		code.jump = 0xe9;
> > -		code.offset = entry->target -
> > -				(entry->code + JUMP_LABEL_NOP_SIZE);
> > -	} else
> > -		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
> > +		if (op == 0xe9 || op == 0xeb)
> > +			/* Already enabled. Warn? */
> > +			return;
> > +
> > +		/* FIXME for all archs */
> 
> By "archs", do you mean different x86 variants?

Yeah, that was a confusing use of archs. This was to make sure it works
for all nops for different variants of x86.

> 
> > +		if (op == nop_short[0]) {
> 
> My gut feeling is that all this "trying to determine the jump size by
> sniffing the instruction" stuff seems pretty fragile.  Couldn't you
> store the jump size in the jump_label structure (even as a bit hidden
> away somewhere)?

We could but it's not as fragile as you think. This is machine code, and
it should be a jump or not. I could add more checks, that is, to look at
the full nop to make sure it is truly a nop. But for the jump side, a
byte instruction that starts with e9 is definitely a jump.

I could harden this more like what we do with mcount updates in the
function tracer. I actually calculate what I expect to be there before
looking at what is there. The entire instruction is checked. If it does
not match, then we fail and give big warnings about it.

Other than that, it should be quite solid. If we don't get a match, we
should warn and disable jump labels.

No BUG()!

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/