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

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]

Message-ID: <alpine.DEB.2.20.1702122006230.3734@nanos>
Date:   Sun, 12 Feb 2017 20:15:20 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Dave Hansen <dave.hansen@...ux.intel.com>
cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org,
        kirill.shutemov@...ux.intel.com
Subject: Re: [RFC][PATCH 3/7] x86, mpx: extend MPX prctl() to pass in size
 of bounds directory

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