[PATCH 1/2] staging: vchiq: Fix bulk userdata handling

[Phil Elwell]

On 04/01/2021 18:31, Dan Carpenter wrote:
> On Mon, Jan 04, 2021 at 12:09:27PM +0000, Phil Elwell wrote:
>> The addition of the local 'userdata' pointer to
>> vchiq_irq_queue_bulk_tx_rx omitted the case where neither BLOCKING nor
>> WAITING modes are used, in which case the value provided by the
>> caller is replaced with a NULL.
>>
>> Fixes: 4184da4f316a ("staging: vchiq: fix __user annotations")
>>
>> Signed-off-by: Phil Elwell <phil at raspberrypi.com>
>> ---
>>   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> index f500a7043805..2a8883673ba1 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> @@ -958,7 +958,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
>>   	struct vchiq_service *service;
>>   	struct bulk_waiter_node *waiter = NULL;
>>   	bool found = false;
>> -	void *userdata = NULL;
>> +	void *userdata;
>>   	int status = 0;
>>   	int ret;
>>   
>> @@ -997,6 +997,8 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
>>   			"found bulk_waiter %pK for pid %d", waiter,
>>   			current->pid);
>>   		userdata = &waiter->bulk_waiter;
>> +	} else {
>> +		userdata = args->userdata;
> 
> "args->userdata" is marked as a user pointer so we really don't want to
> mix user and kernel pointers here.  Presumably this opens up a large
> security hole.

It's an opaque, pointer-sized token that only exists to bereturned to userspace (or not,
without this patch) - it's hard to see that as a security hole.

Phil