Re: [PATCH v3 2/4] x86/microcode: Fix __user annotations around generic_load_microcode()

[Borislav Petkov <bp@alien8.de>]

On Fri, Mar 29, 2019 at 10:46:50PM +0100, Jann Horn wrote:
> generic_load_microcode() deals with a pointer that can be either a kernel
> pointer or a user pointer. Pass it around as a __user pointer so that it
> can't be dereferenced accidentally while its address space is unknown.
> Use explicit casts to convert between __user and __kernel to inform the
> checker that these address space conversions are intentional.
> 
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
>  arch/x86/kernel/cpu/microcode/intel.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 16936a24795c..e8ef65c275c7 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -861,11 +861,13 @@ static enum ucode_state apply_microcode_intel(int cpu)
>  	return ret;
>  }
>  
> -static enum ucode_state generic_load_microcode(int cpu, void *data, size_t size,
> -				int (*get_ucode_data)(void *, const void *, size_t))
> +static enum ucode_state generic_load_microcode(int cpu,
> +		const void __user *data, size_t size,
> +		int (*get_ucode_data)(void *, const void __user *, size_t))

Ok, how about something completely different?

This ->get_ucode_data() BIOS-code-like contraption has always bugged me
for being too ugly to live.

How about we vmalloc() a properly sized buffer - both
generic_load_microcode() callers have the size - and then hand that
buffer into generic_load_microcode() ?

That solves the __user annotation fun immediately and would simplify
generic_load_microcode() additionally.

The disadvantage would be having to vmalloc() a couple of... , I think
it is megabytes, with that old loading method request_microcode_user()
but then if vmalloc() fails, then it was clearly too big. I don't think
the blob can ever be that big though, to fail vmalloc(), but I'm not
going to bet on it...

Hmmm...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.