netdev - Re: [RFC PATCH v1 08/10] net: qrtr: Drop remote {NEW|DEL}

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: <462e098b-efca-4be1-ba5f-e44d8e3b4189@gmail.com>
Date: Thu, 24 Oct 2024 13:03: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 08/10] net: qrtr: Drop remote {NEW|DEL}_LOOKUP
 messages

Hi Chris,

>> @@ -560,6 +560,11 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const 
>> void *data, size_t len)
>>       if (!size || len != ALIGN(size, 4) + hdrlen)
>>           goto err;
>> +    /* Don't allow remote lookups */
>> +    if (cb->type == QRTR_TYPE_NEW_LOOKUP ||
>> +        cb->type == QRTR_TYPE_DEL_LOOKUP)
>> +        goto err;
>> +
> 
> Just curious, was this case observed? I thought we blocked clients from sending 
> this control message to remotes and I didnt think the ns broadcasts it either.

No I didn't see this in practice, so this patch is not strictly necessary.

One thing I thought about originally was to remove the check in ns.c in order to 
extend struct qrtr_lookup with the endpoint id: an application interested only 
in services on a certain endpoint could send NEW_LOOKUP with the endpoint id 
included as a CMSG header.  It made the proposal more complicated though and I 
didn't think it was really needed since QRTR_BIND_ENDPOINT was the better tool 
for this kind of use case.

Regards,
-Denis