Re: [RFC PATCH v1 06/10] net: qrtr: Allow sendmsg to target an endpoint

[Denis Kenzior <denkenz@gmail.com>]

Hi Chris,

>> @@ -106,6 +106,36 @@ static inline struct qrtr_sock *qrtr_sk(struct sock *sk)
>>       return container_of(sk, struct qrtr_sock, sk);
>>   }
>> +int qrtr_msg_get_endpoint(struct msghdr *msg, u32 *out_endpoint_id)
>> +{
>> +    struct cmsghdr *cmsg;
>> +    u32 endpoint_id = 0;
>> +
>> +    for_each_cmsghdr(cmsg, msg) {
>> +        if (!CMSG_OK(msg, cmsg))
>> +            return -EINVAL;
>> +
>> +        if (cmsg->cmsg_level != SOL_QRTR)
>> +            continue;
>> +
>> +        if (cmsg->cmsg_type != QRTR_ENDPOINT)
>> +            return -EINVAL;
>> +
>> +        if (cmsg->cmsg_len < CMSG_LEN(sizeof(u32)))
>> +            return -EINVAL;
>> +
>> +        /* Endpoint ids start at 1 */
>> +        endpoint_id = *(u32 *)CMSG_DATA(cmsg);
>> +        if (!endpoint_id)
>> +            return -EINVAL;
>> +    }
>> +
>> +    if (out_endpoint_id)
>> +        *out_endpoint_id = endpoint_id;
> 
> In the case when there is no cmsg attached to the msg. Would it be safer to 
> assign out_endpoint_id to 0 before returning?

Hmm, isn't that what happens?  endpoint_id is initialized to 0 in the 
declaration block, so if no cmsg headers are present, out_endpoint_id will get a 
0 assigned.

> 
> I see that in qrtr_sendmsg() there is a risk of using msg_endpoint_id without it 
> being initialized or assigned a value in this function.

Calling this function in qrtr_sendmsg() should always assign msg_endpoint_id 
unless an error occurred.

Regards,
-Denis