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

[Colin Ian King]

On 04/04/14 10:46, Matt Fleming wrote:
> 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, please do. Thanks

Colin