linux-kernel - Re: [RFC PATCH v1 00/10] QRTR Multi-endpoint support

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: <0275bd96-ce6a-454a-a407-b2d8fc60e156@quicinc.com>
Date: Tue, 22 Oct 2024 22:07:38 -0700
From: Chris Lew <quic_clew@...cinc.com>
To: Denis Kenzior <denkenz@...il.com>, <netdev@...r.kernel.org>
CC: 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 00/10] QRTR Multi-endpoint support



On 10/18/2024 11:18 AM, Denis Kenzior wrote:
> The current implementation of QRTR assumes that each entity on the QRTR
> IPC bus is uniquely identifiable by its node/port combination, with
> node/port combinations being used to route messages between entities.
> 
> However, this assumption of uniqueness is problematic in scenarios
> where multiple devices with the same node/port combinations are
> connected to the system.  A practical example is a typical consumer PC
> with multiple PCIe-based devices, such as WiFi cards or 5G modems, where
> each device could potentially have the same node identifier set.  In
> such cases, the current QRTR protocol implementation does not provide a
> mechanism to differentiate between these devices, making it impossible
> to support communication with multiple identical devices.
> 
> This patch series addresses this limitation by introducing support for
> a concept of an 'endpoint.' Multiple devices with conflicting node/port
> combinations can be supported by assigning a unique endpoint identifier
> to each one.  Such endpoint identifiers can then be used to distinguish
> between devices while sending and receiving messages over QRTR sockets.
> 
> The patch series maintains backward compatibility with existing clients:
> the endpoint concept is added using auxiliary data that can be added to
> recvmsg and sendmsg system calls.  The QRTR socket interface is extended
> as follows:
> 
> - Adds QRTR_ENDPOINT auxiliary data element that reports which endpoint
>    generated a particular message.  This auxiliary data is only reported
>    if the socket was explicitly opted in using setsockopt, enabling the
>    QRTR_REPORT_ENDPOINT socket option.  SOL_QRTR socket level was added
>    to facilitate this.  This requires QRTR clients to be updated to use
>    recvmsg instead of the more typical recvfrom() or recv() use.
> 
> - Similarly, QRTR_ENDPOINT auxiliary data element can be included in
>    sendmsg() requests.  This will allow clients to route QRTR messages
>    to the desired endpoint, even in cases of node/port conflict between
>    multiple endpoints.
> 
> - Finally, QRTR_BIND_ENDPOINT socket option is introduced.  This allows
>    clients to bind to a particular endpoint (such as a 5G PCIe modem) if
>    they're only interested in receiving or sending messages to this
>    device.
> 
> NOTE: There is 32-bit unsafe use of radix_tree_insert in this patch set.
> This follows the existing usage inside net/qrtr/af_qrtr.c in
> qrtr_tx_wait(), qrtr_tx_resume() and qrtr_tx_flow_failed().  This was
> done deliberately in order to keep the changes as minimal as possible
> until it is known whether the approach outlined is generally acceptable.
> 

Hi Denis,

Thank you for taking a stab at this long standing problem. We've been 
going back and forth on how to solve this but haven't had anyone 
dedicated to working out a solution. From a first pass I think this 
looks very reasonable and I only have a few nitpicks here and there. 
Hopefully Bjorn and Mani will provide more feedback.

Thanks!
Chris


> Denis Kenzior (10):
>    net: qrtr: ns: validate msglen before ctrl_pkt use
>    net: qrtr: allocate and track endpoint ids
>    net: qrtr: support identical node ids
>    net: qrtr: Report sender endpoint in aux data
>    net: qrtr: Report endpoint for locally generated messages
>    net: qrtr: Allow sendmsg to target an endpoint
>    net: qrtr: allow socket endpoint binding
>    net: qrtr: Drop remote {NEW|DEL}_LOOKUP messages
>    net: qrtr: ns: support multiple endpoints
>    net: qrtr: mhi: Report endpoint id in sysfs
> 
>   include/linux/socket.h    |   1 +
>   include/uapi/linux/qrtr.h |   7 +
>   net/qrtr/af_qrtr.c        | 297 +++++++++++++++++++++++++++++++------
>   net/qrtr/mhi.c            |  14 ++
>   net/qrtr/ns.c             | 299 +++++++++++++++++++++++---------------
>   net/qrtr/qrtr.h           |   4 +
>   6 files changed, 459 insertions(+), 163 deletions(-)
>