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: <20241022153912.hoa2wbqtkvwjzuyo@thinkpad>
Date: Tue, 22 Oct 2024 21:09:12 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Denis Kenzior <denkenz@...il.com>
Cc: netdev@...r.kernel.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 Fri, Oct 18, 2024 at 01:18:18PM -0500, 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.
> 

Thanks for your work on this! I'm yet to look into the details but wondering how
all the patches have Reviewed-by tags provided that this series is 'RFC v1'.
Also, it is quite surprising to see the review tag from Andy Gross who quit Qcom
quite a while ago and I haven't seen him reviewing any Qcom patches for so long.

- Mani

> 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.
> 
> 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(-)
> 
> -- 
> 2.45.2
> 

-- 
மணிவண்ணன் சதாசிவம்