[PATCH] nvme: sanitize metadata bounce buffer for reads

[Jens Axboe]

On 10/16/23 2:31 PM, Keith Busch wrote:
> On Mon, Oct 16, 2023 at 02:25:11PM -0600, Jens Axboe wrote:
>> On 10/16/23 2:21 PM, Keith Busch wrote:
>>>  	if ((req_op(req) == REQ_OP_DRV_OUT) && copy_from_user(buf, ubuf, len))
>>>  		goto out_free_meta;
>>> +	else
>>> +		memset(buf, 0, len);
>>
>> Do you need this else?
> 
> The 'if' condition just above copies user data into the 'buf', so yes,
> we need the else to separate how buf is initialized for each direction.

It still looks wrong to me. If it's writing to the drive AND we fail
copying in data, -EFAULT. But the opposite of that isn't that it's a
read from device.

	if (req_op(req) == REQ_OP_DRV_OUT) {
		ret = -EFAULT;
		if (copy_from_user(buf, ubuf, len))
			goto out_free_meta;
	} else {
		memset(buf, 0, len);
	}

would look more readable, as it splits it on direction, rather than on
direction and the status of the copy. And follows the normal idioms for
erroring on the copy condition.

-- 
Jens Axboe