Re: [PATCH v3 2/4] x86, mpx: hook #BR exception handler to allocatebound tables

[Ren Qiaowei]

On 01/28/2014 04:36 AM, Andy Lutomirski wrote:
> > +	bd_entry = status & MPX_BNDSTA_ADDR_MASK;
> > +	if ((bd_entry >= bd_base) && (bd_entry < bd_base + bd_size))
> > +		allocate_bt(bd_entry);
>
> What happens if this fails?  Retrying forever isn't very nice.
If allocation of the bound table fail, the related entry in the bound 
directory is still invalid. The following access to this entry still 
produce #BR fault.

> > +	if (!user_mode(regs)) {
> > +		if (!fixup_exception(regs)) {
> > +			tsk->thread.error_code = error_code;
> > +			tsk->thread.trap_nr = X86_TRAP_BR;
> > +			die("bounds", regs, error_code);
> > +		}
>
> Why the fixup?  Unless I'm missing something, the kernel has no business
> getting #BR on access to a user address.
>
> Or are you adding code to allow the kernel to use MPX itself?  If so,
> shouldn't this use an MPX-specific fixup to allow normal C code to use
> this stuff?
It checks whether #BR come from user-space. You can see do_trap_no_signal().

> > +		goto exit;
> > +	}
> > +
> > +	if (!boot_cpu_has(X86_FEATURE_MPX)) {
> > +		do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, NULL);
> > +		goto exit;
>
> This, as well as the status == 0 case, should probably document that the
> exception is from BOUND, not MPX.
Ok. I will add one comment for this.


> > +		break;
> > +
> > +	case 1: /* Bound violation. */
> > +	case 0: /* No MPX exception. */
> > +		do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, NULL);
> > +		break;
>
> What does "No Intel MPX exception" mean?  Surely that has business
> sending #BR.
Oh. It comes from spec, and just mean it is not from MPX. :) I will 
change it to be accurate.

> > +
> > +	default:
> > +		break;
>
> What does status 3 mean?  The docs say "reserved".  Presumably this
> should log and kill the process.
I guess it should be a good suggestion.

Thanks,
Qiaowei

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