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

[Denis Kenzior <denkenz@xxxxxxxxx>]

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