[PATCH 4/4] efi_runtime: Do not pass user addresses to firmware

[Matt Fleming]

On Fri, 04 Apr, at 10:18:24AM, Borislav Petkov wrote:
> 
> Just to show what I mean when we were talking on IRC: Since the caller
> needs to free *dst, the comment over the function should state that, the
> very least.
 
Yeah, that's a good point as the calling is asymmetric wrt memory
allocation.

> Then, we probably could call those functions something like
> copy_uc2_to_user() and so on, to state that we're shuffling data to and
> fro userspace and have the get/put_ucs2 names for allocating/freeing
> buffers.

Another good idea, although I'm not keen on having a simple wrapper
around kfree() just to maintain symmetry. I do think the name changes
make sense though, but I'm happy to leave the kfree() stuff and simply
delete put_ucs2(), otherwise you get stuff like this,

In hindsight, get_* and put_* imply some kind of reference counting or
automatic alloc/free. I was really just trying to mirror put_user() and
get_user().

>  	data = kmalloc(datasize, GFP_KERNEL);
>  	if (copy_from_user(data, psetvariable->Data, datasize)) {
> -		kfree(name);
> +		put_ucs2(name);
>  		return -EFAULT;
>  	}
>  
>  	status = efi.set_variable(name, &vendor, attr, datasize, data);
>  
>  	kfree(data);
> -	kfree(name);
> +	put_ucs2(name);

where 'name' looks special in some way, but it's really not.

Colin, would you like me to send out a v2 with the improved comments,
deleted put_ucs2() (since it's unused) and improved naming convention?

-- 
Matt Fleming, Intel Open Source Technology Center