Re: [PATCH V4 8/9] IB/core: Add RoCE IP based addressing extensions for rdma_ucm

[Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>]

On 11/9/2013 3:36 PM, Yann Droneaud wrote:
> Le 11.09.2013 13:32, Or Gerlitz a écrit :
>> On 11/09/2013 12:52, Yann Droneaud wrote:
>>> Le 10.09.2013 16:41, Or Gerlitz a écrit :
>>>> +static ssize_t ucma_init_qp_attr_ex(struct ucma_file *file,
>>>> +                    const char __user *inbuf,
>>>> +                    int in_len, int out_len)
>>>> +{
>>>> +    struct rdma_ucm_init_qp_attr_ex cmd;
>>>> +    struct ib_uverbs_qp_attr_ex resp;
>>>> +    struct ucma_context *ctx;
>>>> +    struct ib_qp_attr qp_attr;
>>>> +    int ret;
>>>> +
>>>> +    if (out_len < sizeof(resp))
>>>> +        return -ENOSPC;
>>>> +
>>>> +    if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
>>>> +        return -EFAULT;
>>>> +
>>>> +    if (copy_from_user(&resp, (void __user *)(unsigned
>>>> long)cmd.response,
>>>> +               sizeof(resp)))
>>>> +        return -EFAULT;
>>>> +
>>>
>>>
>>> Reading from the response buffer ? I haven't seen that before in
>>> IB/core before.
>>
>> The intent here is to use copy_from_user just to make sure the user space
>> provided buffer has enough room to hold the kernel response structure.
>> This
>> this command may be extended in the future without bumping the overall
>> uverbs ABI
>> version we wanted to add this extra protection.
>
> It's checking nothing ... you should not suppose the user having / not
> having its buffer end
> aligned on a page boundary, so that the kernel will detect the end of
> buffer
> when trying to read from it.
>
> BTW, out_len is already checked against resp size ... so I don't
> understand yet.

That's right - we're not checking anything here.
struct ib_uverbs_qp_attr_ex contains 2 user pointers:
+       /* represents: struct ib_uverbs_ah_attr_ex * __user */
+       void __user *ah_attr_ex;
+       /* represents: struct ib_uverbs_ah_attr_ex * __user */
+       void __user *alt_ah_attr_ex;

Those pointers should be given to rdma_init_qp_attr in order to 
initialize them.

We are using pointers here in order to maintain future extendability of 
the address handle structures.

>
> Regards.
>
Regards
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html