Re: [RFC][PATCH 3/7] x86, mpx: extend MPX prctl() to pass in size of bounds directory

[Thomas Gleixner]

On Wed, 1 Feb 2017, Dave Hansen wrote:
> FIXME: we also need to ensure that we check the current state of the
> larger address space opt-in.  If we've opted in to larger address spaces
> we can not allow a small bounds directory to be used.  Also, if we've
> not opted in, we can not allow the larger bounds directory to be used.
> This can be fixed once the in-kernel API for opting in/out is settled.

Ok.

>  /* Register/unregister a process' MPX related resource */
> -#define MPX_ENABLE_MANAGEMENT()	mpx_enable_management()
> +#define MPX_ENABLE_MANAGEMENT(bd_size)	mpx_enable_management(bd_size)
>  #define MPX_DISABLE_MANAGEMENT()	mpx_disable_management()

Please add another tab before mpx_disable so both are aligned.

> -int mpx_enable_management(void)
> +int mpx_set_mm_bd_size(unsigned long bd_size)

static ?

> +{
> +	struct mm_struct *mm = current->mm;
> +
> +	switch ((unsigned long long)bd_size) {
> +	case 0:
> +		/* Legacy call to prctl(): */
> +		mm->context.mpx_bd_shift = 0;
> +		return 0;
> +	case MPX_BD_SIZE_BYTES_32:
> +		/* 32-bit, legacy-sized bounds directory: */
> +		if (is_64bit_mm(mm))
> +			return -EINVAL;
> +		mm->context.mpx_bd_shift = 0;
> +		return 0;
> +	case MPX_BD_BASE_SIZE_BYTES_64:
> +		/* 64-bit, legacy-sized bounds directory: */
> +		if (!is_64bit_mm(mm)
> +		// FIXME && ! opted-in to larger address space

Hmm. Confused. This is where we enable MPX and decode the requested address
space. How can an already opt in happen?

If that's a enable call for an already enabled thing, then we should catch
that at the call site, I think.

> +	case MPX_BD_BASE_SIZE_BYTES_64 << MPX_LARGE_BOUNDS_DIR_SHIFT:
> +		/*
> +		 * Non-legacy call, with larger directory.
> +		 * Note that there is no 32-bit equivalent for
> +		 * this case since its address space does not
> +		 * change sizes.
> +		 */
> +		if (!is_64bit_mm(mm))
> +			return -EINVAL;
> +		/*
> +		 * Do not let this be enabled unles we are on
> +		 * 5-level hardware *and* have that feature
> +		 * enabled. FIXME: need runtime check

Runtime check? Isn't the feature bit enough?

Thanks,

	tglx