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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2c027bc3-6cab-4892-9544-10a0a23db871@gmail.com>
Date: Thu, 24 Oct 2024 12:40:43 -0500
From: Denis Kenzior <denkenz@...il.com>
To: Chris Lew <quic_clew@...cinc.com>, netdev@...r.kernel.org
Cc: Marcel Holtmann <marcel@...tmann.org>, Andy Gross <agross@...nel.org>,
 Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v1 06/10] net: qrtr: Allow sendmsg to target an
 endpoint

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