QRTR Multi-endpoint support
Subsystems (5)
v2 cover QRTR Multi-endpoint support
I am incredibly thankful for Denis's work on this. To get this back on track and to eventually get it merged, I'm resubmitting his patch set with issues in the first review round resolved. This feature is a prerequisite for my work on ath1{1,2}k to allow using multiple devices in one computer.
The original description follows:
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.
v2:
- rebased against current master
- fixed most issues found in first review round (see individual commits), minus the 32-bit long
unsafe use
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 | 286 ++++++++++++++++++++++++++++++------ net/qrtr/mhi.c | 14 ++ net/qrtr/ns.c | 299 +++++++++++++++++++++++--------------- net/qrtr/qrtr.h | 4 + 6 files changed, 448 insertions(+), 163 deletions(-)
Signature · 1 line
2.50.0
PATCH v2 1/10 net: qrtr: ns: validate msglen before ctrl_pkt use
From: Denis Kenzior <[email protected]>
The qrtr_ctrl_pkt structure is currently accessed without checking if the received payload is large enough to hold the structure's fields. Add a check to ensure the payload length is sufficient.
---
v2:
- rebase against current master
- use correct size of packet structure as per review comment
--- net/qrtr/ns.c | 3 +++ 1 file changed, 3 insertions(+)
net/qrtr/ns.c +3 -0
@@ -619,6 +619,9 @@ static void qrtr_ns_worker(struct work_struct *work)break ; } +if ((size_t )msglen <sizeof (*pkt)) +break ; + pkt = recv_buf; cmd =le32_to_cpu (pkt->cmd); if (cmd < ARRAY_SIZE(qrtr_ctrl_pkt_strings) &&
Signature · 1 line
2.50.0
Re: [PATCH v2 01/10] net: qrtr: ns: validate msglen before ctrl_pkt use
Hi Mihai
Quoted 10 lines · Mihai Moldovan
From: Denis Kenzior <[email protected]>
The
qrtr_ctrl_pktstructure is currently accessed without checking if the received payload is large enough to hold the structure's fields. Add a check to ensure the payload length is sufficient.Signed-off-by Denis Kenzior <[email protected]>Reviewed-by Marcel Holtmann <[email protected]>Reviewed-by Andy Gross <[email protected]>Signed-off-by Mihai Moldovan <[email protected]>
I think this is missing a Fixes: tag?
Kind regards,
Quoted 24 lines
---
v2:
- rebase against current master
- use correct size of packet structure as per review comment--- net/qrtr/ns.c | 3 +++ 1 file changed, 3 insertions(+)net/qrtr/ns.c +3
-0@@ -619,6 +619,9 @@ static void qrtr_ns_worker(struct work_struct *work)break ; } +if ((size_t )msglen <sizeof (*pkt)) +break ; + pkt = recv_buf; cmd =le32_to_cpu (pkt->cmd); if (cmd < ARRAY_SIZE(qrtr_ctrl_pkt_strings) &&
Signature · 1 line
// Casey (she/her)
Re: [PATCH v2 01/10] net: qrtr: ns: validate msglen before ctrl_pkt use
Quoted 3 lines · * On 7/21/25 13:02, Casey Connolly wrote:
On 19/07/2025 20:59, Mihai Moldovan wrote:
I think this is missing a Fixes: tag?
Thanks.
Will add Fixes: 0c2204a4ad71 ("net: qrtr: Migrate nameservice to kernel from userspace") in v3.
I haven't seen any reports of this actually tripping anything up.
Mihai
PATCH v2 2/10 net: qrtr: allocate and track endpoint ids
In reply to Mihai MoldovanFrom: Denis Kenzior <[email protected]>
Currently, QRTR endpoints are tracked solely by their pointer value, which is sufficient as they are not exposed to user space and it is assumed that each endpoint has a unique set of node identifiers associated with it. However, this assumption does not hold when multiple devices of the same type are connected to the system. For example, multiple PCIe based 5G modems. Such a setup results in multiple endpoints with confliciting node identifiers.
To enable support for such scenarios, introduce the ability to track and assign unique identifiers to QRTR endpoints. These identifiers can then be exposed to user space, allowing for userspace clients to identify which endpoint sent a given message, or to direct a message to a specific endpoint.
A simple allocation strategy is used based on xa_alloc_cyclic. Remote endpoint ids start at 'qrtr_local_nid' + 1. Since qrtr_local_nid is currently always set to 1 and never changed, node identifiers start at '1' for the local endpoint and 2..INT_MAX for remote endpoints.
---
v2:
- rebase against current master
--- net/qrtr/af_qrtr.c | 24 ++++++++++++++++++++++++ net/qrtr/qrtr.h | 1 + 2 files changed, 25 insertions(+)
net/qrtr/af_qrtr.c +24 -0
@@ -22,6 +22,7 @@#define QRTR_MAX_EPH_SOCKET 0x7fff#define QRTR_EPH_PORT_RANGE \ XA_LIMIT(QRTR_MIN_EPH_SOCKET, QRTR_MAX_EPH_SOCKET) +#define QRTR_ENDPOINT_RANGE XA_LIMIT(qrtr_local_nid + 1, INT_MAX)#define QRTR_PORT_CTRL_LEGACY 0xffff
@@ -109,6 +110,10 @@ static LIST_HEAD(qrtr_all_nodes);/* lock for qrtr_all_nodes and node reference */ static DEFINE_MUTEX (qrtr_node_lock); +/* endpoint id allocation management */ +static DEFINE_XARRAY_ALLOC (qrtr_endpoints); +static u32 next_endpoint_id; +/* local port allocation management */ static DEFINE_XARRAY_ALLOC (qrtr_ports);
@@ -585,6 +590,8 @@ static struct sk_buff *qrtr_alloc_ctrl_packet(struct qrtr_ctrl_pkt **pkt,int qrtr_endpoint_register (struct qrtr_endpoint *ep,unsigned int nid) {struct qrtr_node *node; +u32 endpoint_id; +int rc;if (!ep || !ep->xmit)return -EINVAL ;
@@ -593,6 +600,13 @@ int qrtr_endpoint_register(struct qrtr_endpoint *ep, unsigned int nid)if (!node)return -ENOMEM ; + rc =xa_alloc_cyclic (&qrtr_endpoints, &endpoint_id,NULL , +QRTR_ENDPOINT_RANGE , &next_endpoint_id, +GFP_KERNEL ); + +if (rc <0 ) + goto free_node; +kref_init (&node->ref);mutex_init (&node->ep_lock);skb_queue_head_init (&node->rx_queue);
@@ -608,8 +622,12 @@ int qrtr_endpoint_register(struct qrtr_endpoint *ep, unsigned int nid)list_add (&node->item, &qrtr_all_nodes);mutex_unlock (&qrtr_node_lock); ep->node = node; + ep->id = endpoint_id;return 0 ; +free_node: +kfree (node); +return rc; }EXPORT_SYMBOL_GPL (qrtr_endpoint_register);
@@ -628,8 +646,10 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep)struct sk_buff *skb;unsigned long flags;void __rcu **slot; +u32 endpoint_id;mutex_lock (&node->ep_lock); + endpoint_id = node->ep->id; node->ep =NULL ;mutex_unlock (&node->ep_lock);
@@ -656,6 +676,10 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep)mutex_unlock (&node->qrtr_tx_lock);qrtr_node_release (node); + +xa_erase (&qrtr_endpoints, endpoint_id); + + ep->id =0 ; ep->node =NULL ; }EXPORT_SYMBOL_GPL (qrtr_endpoint_unregister);
net/qrtr/qrtr.h +1 -0
@@ -21,6 +21,7 @@ struct qrtr_endpoint {int (*xmit)(struct qrtr_endpoint *ep,struct sk_buff *skb);/* private: not for endpoint use */ struct qrtr_node *node; +u32 id; };int qrtr_endpoint_register (struct qrtr_endpoint *ep,unsigned int nid);
Signature · 1 line
2.50.0
PATCH v2 3/10 net: qrtr: support identical node ids
In reply to Mihai MoldovanFrom: Denis Kenzior <[email protected]>
Add support for tracking multiple endpoints that may have conflicting node identifiers. This is achieved by using both the node and endpoint identifiers as the key inside the radix_tree data structure.
For backward compatibility with existing clients, the previous key schema (node identifier only) is preserved. However, this schema will only support the first endpoint/node combination. This is acceptable for legacy clients as support for multiple endpoints with conflicting node identifiers was not previously possible.
---
v2:
- rebase against current master
- no action on review comment regarding integer overflow on 32 bit long platforms (thus far)
--- net/qrtr/af_qrtr.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
net/qrtr/af_qrtr.c +9 -1
@@ -418,12 +418,20 @@ static struct qrtr_node *qrtr_node_lookup(unsigned int nid)static void qrtr_node_assign (struct qrtr_node *node,unsigned int nid) {unsigned long flags; +unsigned long key;if (nid ==QRTR_EP_NID_AUTO )return ;spin_lock_irqsave (&qrtr_nodes_lock, flags); -radix_tree_insert (&qrtr_nodes, nid, node); + +/* Always insert with the endpoint_id + node_id */ + key = (unsigned long )node->ep->id <<32 | nid; +radix_tree_insert (&qrtr_nodes, key, node); + +if (!radix_tree_lookup (&qrtr_nodes, nid)) +radix_tree_insert (&qrtr_nodes, nid, node); +if (node->nid ==QRTR_EP_NID_AUTO ) node->nid = nid;spin_unlock_irqrestore (&qrtr_nodes_lock, flags);
Signature · 1 line
2.50.0
kernel test robot
build botRe: [PATCH v2 03/10] net: qrtr: support identical node ids
kernel test robot noticed the following build warnings:
Hi Mihai,
kernel test robot noticed the following build warnings:
[auto build test WARNING on mani-mhi/mhi-next]
[also build test WARNING on net-next/main net/main linus/master v6.16-rc6 next-20250718] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mihai-Moldovan/net-qrtr-ns-validate-msglen-before-ctrl_pkt-use/20250720-030426 base: https://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git mhi-next patch link: https://lore.kernel.org/r/4d0fe1eab4b38fb85e2ec53c07289bc0843611a2.1752947108.git.ionic%40ionic.de patch subject: [PATCH v2 03/10] net: qrtr: support identical node ids config: arc-randconfig-002-20250720 (https://download.01.org/0day-ci/archive/20250720/[email protected]/config) compiler: arc-linux-gcc (GCC) 8.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250720/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
net/qrtr/af_qrtr.c: In function 'qrtr_node_assign':
>> net/qrtr/af_qrtr.c:429:36: warning: left shift count >= width of type [-Wshift-count-overflow]
key = (unsigned long)node->ep->id << 32 | nid;
^~vim +429 net/qrtr/af_qrtr.c
412
413 /* Assign node id to node.
414 *
415 * This is mostly useful for automatic node id assignment, based on
416 * the source id in the incoming packet.
417 */
418 static void qrtr_node_assign(struct qrtr_node *node, unsigned int nid)
419 {
420 unsigned long flags;
421 unsigned long key;
422
423 if (nid == QRTR_EP_NID_AUTO)
424 return;
425
426 spin_lock_irqsave(&qrtr_nodes_lock, flags);
427
428 /* Always insert with the endpoint_id + node_id */
> 429 key = (unsigned long)node->ep->id << 32 | nid;
430 radix_tree_insert(&qrtr_nodes, key, node);
431
432 if (!radix_tree_lookup(&qrtr_nodes, nid))
433 radix_tree_insert(&qrtr_nodes, nid, node);
434
435 if (node->nid == QRTR_EP_NID_AUTO)
436 node->nid = nid;
437 spin_unlock_irqrestore(&qrtr_nodes_lock, flags);
438 }
439
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
PATCH v2 4/10 net: qrtr: Report sender endpoint in aux data
In reply to Mihai MoldovanFrom: Denis Kenzior <[email protected]>
Introduce support for reporting the remote endpoint that generated a given QRTR message to clients using AF_QIPCRTR family sockets. This is achieved by including QRTR_ENDPOINT auxiliary data, which carries the endpoint identifier of the message sender. To receive this auxiliary data, clients must explicitly opt-in by using setsockopt with the QRTR_REPORT_ENDPOINT option enabled.
Implementation of getsockopt and setsockopt is provided. An additional level 'SOL_QRTR' is added to socket.h for use by AF_QIPCRTR family sockets.
---
v2:
- rebase against current master
- dropped socket locking in qrtr_setsockopt() and qrtr_getsockopt() as per review comment
--- include/linux/socket.h | 1 + include/uapi/linux/qrtr.h | 6 +++ net/qrtr/af_qrtr.c | 78 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+)
include/linux/socket.h +1 -0
@@ -386,6 +386,7 @@ struct ucred {#define SOL_MCTP 285#define SOL_SMC 286#define SOL_VSOCK 287 +#define SOL_QRTR 288/* IPX options */ #define IPX_TYPE 1
include/uapi/linux/qrtr.h +6 -0
@@ -46,4 +46,10 @@ struct qrtr_ctrl_pkt { }; } __packed; +/* setsockopt / getsockopt */ +#define QRTR_REPORT_ENDPOINT 1 + +/* CMSG */ +#define QRTR_ENDPOINT 1 +#endif /* _LINUX_QRTR_H */
net/qrtr/af_qrtr.c +78 -0
126 lines not on this page · raw message
Signature · 1 line
2.50.0
PATCH v2 5/10 net: qrtr: Report endpoint for locally generated messages
In reply to Mihai MoldovanFrom: Denis Kenzior <[email protected]>
For messages generated by the local endpoint destined
to the local endpoint, report the local endpoint identifier. Same QRTR_ENDPOINT auxiliary data and QRTR_REPORT_ENDPOINT socket option semantics apply as for messages generated by remote endpoints.
---
v2:
- rebase against current master
--- net/qrtr/af_qrtr.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)
net/qrtr/af_qrtr.c +17 -8
@@ -163,9 +163,11 @@ struct qrtr_tx_flow {#define QRTR_TX_FLOW_LOW 5static int qrtr_local_enqueue (struct qrtr_node *node,struct sk_buff *skb, +u32 endpoint_id,int type,struct sockaddr_qrtr *from,struct sockaddr_qrtr *to);static int qrtr_bcast_enqueue (struct qrtr_node *node,struct sk_buff *skb, +u32 endpoint_id,int type,struct sockaddr_qrtr *from,struct sockaddr_qrtr *to);static struct qrtr_sock *qrtr_port_lookup (int port);
@@ -349,6 +351,7 @@ static void qrtr_tx_flow_failed(struct qrtr_node *node, int dest_node,/* Pass an outgoing packet socket buffer to the endpoint driver. */ static int qrtr_node_enqueue (struct qrtr_node *node,struct sk_buff *skb, +u32 endpoint_id,int type,struct sockaddr_qrtr *from,struct sockaddr_qrtr *to) {
@@ -678,7 +681,8 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep) skb =qrtr_alloc_ctrl_packet (&pkt,GFP_ATOMIC );if (skb) { pkt->cmd =cpu_to_le32 (QRTR_TYPE_BYE ); -qrtr_local_enqueue (NULL , skb,QRTR_TYPE_BYE , &src, &dst); +qrtr_local_enqueue (NULL , skb, endpoint_id, +QRTR_TYPE_BYE , &src, &dst); } }spin_unlock_irqrestore (&qrtr_nodes_lock, flags);
@@ -745,8 +749,8 @@ static void qrtr_port_remove(struct qrtr_sock *ipc) pkt->client.port =cpu_to_le32 (ipc->us.sq_port);skb_set_owner_w (skb, &ipc->sk); -qrtr_bcast_enqueue (NULL , skb,QRTR_TYPE_DEL_CLIENT , &ipc->us, - &to); +qrtr_bcast_enqueue (NULL , skb, qrtr_local_nid, +QRTR_TYPE_DEL_CLIENT , &ipc->us, &to); }if (port ==QRTR_PORT_CTRL )
@@ -886,6 +890,7 @@ static int qrtr_bind(struct socket *sock, struct sockaddr *saddr, int len)/* Queue packet to local peer socket. */ static int qrtr_local_enqueue (struct qrtr_node *node,struct sk_buff *skb, +u32 endpoint_id,int type,struct sockaddr_qrtr *from,struct sockaddr_qrtr *to) {
@@ -903,6 +908,7 @@ static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb, cb = (struct qrtr_cb *)skb->cb; cb->src_node = from->sq_node; cb->src_port = from->sq_port; + cb->endpoint_id = endpoint_id;if (sock_queue_rcv_skb (&ipc->sk, skb)) {qrtr_port_put (ipc);
@@ -917,6 +923,7 @@ static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb,/* Queue packet for broadcast. */ static int qrtr_bcast_enqueue (struct qrtr_node *node,struct sk_buff *skb, +u32 endpoint_id,int type,struct sockaddr_qrtr *from,struct sockaddr_qrtr *to) {
@@ -928,11 +935,11 @@ static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb,if (!skbn)break ;skb_set_owner_w (skbn, skb->sk); -qrtr_node_enqueue (node, skbn, type, from, to); +qrtr_node_enqueue (node, skbn, endpoint_id, type, from, to); }mutex_unlock (&qrtr_node_lock); -qrtr_local_enqueue (NULL , skb, type, from, to); +qrtr_local_enqueue (NULL , skb, endpoint_id, type, from, to);return 0 ; }
@@ -940,12 +947,13 @@ static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb,static int qrtr_sendmsg (struct socket *sock,struct msghdr *msg,size_t len) { DECLARE_SOCKADDR(struct sockaddr_qrtr *, addr, msg->msg_name); -int (*enqueue_fn)(struct qrtr_node *,struct sk_buff *,int , +int (*enqueue_fn)(struct qrtr_node *,struct sk_buff *,u32 ,int ,struct sockaddr_qrtr *,struct sockaddr_qrtr *);__le32 qrtr_type =cpu_to_le32 (QRTR_TYPE_DATA );struct qrtr_sock *ipc =qrtr_sk (sock->sk);struct sock *sk = sock->sk;struct qrtr_node *node; +u32 endpoint_id = qrtr_local_nid;struct sk_buff *skb;size_t plen;u32 type;
@@ -1029,7 +1037,7 @@ static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) } type =le32_to_cpu (qrtr_type); - rc =enqueue_fn (node, skb, type, &ipc->us, addr); + rc =enqueue_fn (node, skb, endpoint_id, type, &ipc->us, addr);if (rc >=0 ) rc = len;
@@ -1061,7 +1069,8 @@ static int qrtr_send_resume_tx(struct qrtr_cb *cb) pkt->client.node =cpu_to_le32 (cb->dst_node); pkt->client.port =cpu_to_le32 (cb->dst_port); - ret =qrtr_node_enqueue (node, skb,QRTR_TYPE_RESUME_TX , &local, &remote); + ret =qrtr_node_enqueue (node, skb, cb->endpoint_id, +QRTR_TYPE_RESUME_TX , &local, &remote);qrtr_node_release (node);
Signature · 1 line
2.50.0
PATCH v2 6/10 net: qrtr: Allow sendmsg to target an endpoint
In reply to Mihai MoldovanFrom: Denis Kenzior <[email protected]>
Allow QIPCRTR family sockets to include QRTR_ENDPOINT auxiliary data as part of the sendmsg system call. By including this parameter, the client can ask the kernel to route the message to a given endpoint, in situations where multiple endpoints with conflicting node identifier sets exist in the system.
For legacy clients, or clients that do not include QRTR_ENDPOINT data, the endpoint is looked up, as before, by only using the node identifier of the destination qrtr socket address.
---
v2:
- rebase against current master
- no action on review comment regarding initializing out_endpoint_id, since that's rightfully already being done
--- net/qrtr/af_qrtr.c | 80 +++++++++++++++++++++++++++++++++------------- net/qrtr/qrtr.h | 2 ++ 2 files changed, 60 insertions(+), 22 deletions(-)
net/qrtr/af_qrtr.c +58 -22
@@ -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; + +return 0 ; +} +static unsigned int qrtr_local_nid =1 ;/* for node ids */
@@ -404,14 +434,16 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,* * callers must release with qrtr_node_release() */ -static struct qrtr_node *qrtr_node_lookup (unsigned int nid) +static struct qrtr_node *qrtr_node_lookup (unsigned int endpoint_id, +unsigned int nid) {struct qrtr_node *node;unsigned long flags; +unsigned long key = (unsigned long )endpoint_id <<32 | nid;mutex_lock (&qrtr_node_lock);spin_lock_irqsave (&qrtr_nodes_lock, flags); - node =radix_tree_lookup (&qrtr_nodes, nid); + node =radix_tree_lookup (&qrtr_nodes, key); node =qrtr_node_acquire (node);spin_unlock_irqrestore (&qrtr_nodes_lock, flags);mutex_unlock (&qrtr_node_lock);
@@ -953,6 +985,7 @@ static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)struct qrtr_sock *ipc =qrtr_sk (sock->sk);struct sock *sk = sock->sk;struct qrtr_node *node; +u32 msg_endpoint_id;u32 endpoint_id = qrtr_local_nid;struct sk_buff *skb;size_t plen;
@@ -965,46 +998,48 @@ static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)if (len >65535 )return -EMSGSIZE ; + rc =qrtr_msg_get_endpoint (msg, &msg_endpoint_id); +if (rc <0 ) +return rc; +lock_sock (sk);if (addr) { -if (msg->msg_namelen <sizeof (*addr)) { -release_sock (sk); -return -EINVAL ; - } + rc = -EINVAL ; -if (addr->sq_family !=AF_QIPCRTR ) { -release_sock (sk); -return -EINVAL ; - } +if (msg->msg_namelen <sizeof (*addr)) + goto release_sock; + +if (addr->sq_family !=AF_QIPCRTR ) + goto release_sock; rc =qrtr_autobind (sock); -if (rc) { -release_sock (sk); -return rc; - } +if (rc) + goto release_sock; }else if (sk->sk_state ==TCP_ESTABLISHED ) { addr = &ipc->peer; }else { -release_sock (sk); -return -ENOTCONN ; + rc = -ENOTCONN ; + goto release_sock; } node =NULL ;if (addr->sq_node ==QRTR_NODE_BCAST ) {if (addr->sq_port !=QRTR_PORT_CTRL && qrtr_local_nid !=QRTR_NODE_BCAST ) { -release_sock (sk); -return -ENOTCONN ; + rc = -ENOTCONN ; + goto release_sock; } enqueue_fn = qrtr_bcast_enqueue; }else if (addr->sq_node == ipc->us.sq_node) { enqueue_fn = qrtr_local_enqueue; }else { - node =qrtr_node_lookup (addr->sq_node); + endpoint_id = msg_endpoint_id; + + node =qrtr_node_lookup (endpoint_id, addr->sq_node);if (!node) { -release_sock (sk); -return -ECONNRESET ; + rc = endpoint_id ? -ENXIO : -ECONNRESET ; + goto release_sock; } enqueue_fn = qrtr_node_enqueue; }
@@ -1043,6 +1078,7 @@ static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) out_node:qrtr_node_release (node); +release_sock:release_sock (sk);return rc;
@@ -1057,7 +1093,7 @@ static int qrtr_send_resume_tx(struct qrtr_cb *cb)struct sk_buff *skb;int ret; - node =qrtr_node_lookup (remote.sq_node); + node =qrtr_node_lookup (cb->endpoint_id, remote.sq_node);if (!node)return -EINVAL ;
net/qrtr/qrtr.h +2 -0
@@ -34,4 +34,6 @@ int qrtr_ns_init(void);void qrtr_ns_remove (void ); +int qrtr_msg_get_endpoint (struct msghdr *msg,u32 *out_endpoint_id); +#endif
Signature · 1 line
2.50.0
PATCH v2 7/10 net: qrtr: allow socket endpoint binding
In reply to Mihai MoldovanFrom: Denis Kenzior <[email protected]>
Introduce the ability to bind a QIPCRTR family socket to a specific endpoint. When a socket is bound, only messages from the bound endpoint can be received, and any messages sent from the socket are by default directed to the bound endpoint. Clients can bind a socket by using the setsockopt system call with the QRTR_BIND_ENDPOINT option set to the desired endpoint binding.
A previously set binding can be reset by setting QRTR_BIND_ENDPOINT option to zero. This behavior matches that of SO_BINDTOIFINDEX.
This functionality is useful for clients that need to communicate with a specific device (i.e. endpoint), such as a PCIe-based 5G modem, and are not interested in messages from other endpoints / nodes.
---
v2:
- rebase against current master
- use WRITE_ONCE() to write value in qrtr_setsockopt() and READ_ONCE() to read it in qrtr_getsockopt() as per review comment
--- include/uapi/linux/qrtr.h | 1 + net/qrtr/af_qrtr.c | 54 ++++++++++++++++++++++++++++----------- 2 files changed, 40 insertions(+), 15 deletions(-)
include/uapi/linux/qrtr.h +1 -0
@@ -48,6 +48,7 @@ struct qrtr_ctrl_pkt {/* setsockopt / getsockopt */ #define QRTR_REPORT_ENDPOINT 1 +#define QRTR_BIND_ENDPOINT 2/* CMSG */ #define QRTR_ENDPOINT 1
net/qrtr/af_qrtr.c +39 -15
102 lines not on this page · raw message
Signature · 1 line
2.50.0
PATCH v2 8/10 net: qrtr: Drop remote {NEW|DEL}_LOOKUP messages
In reply to Mihai MoldovanFrom: Denis Kenzior <[email protected]>
These messages are explicitly filtered out by the in-kernel name service (ns.c). Filter them out even earlier to save some CPU cycles.
---
v2:
- rebase against current master
--- net/qrtr/af_qrtr.c | 5 +++++ 1 file changed, 5 insertions(+)
net/qrtr/af_qrtr.c +5 -0
@@ -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; +if ((cb->type ==QRTR_TYPE_NEW_SERVER || cb->type ==QRTR_TYPE_RESUME_TX ) && size <sizeof (struct qrtr_ctrl_pkt ))
Signature · 1 line
2.50.0
PATCH v2 9/10 net: qrtr: ns: support multiple endpoints
In reply to Mihai MoldovanFrom: Denis Kenzior <[email protected]>
Extend the qrtr name service with the concept of an endpoint. Endpoints can be remote or local, and are represented by a unique endpoint id. This allows the qrtr name service to support multiple devices (endpoints) that might have overlapping node / port combinations.
The socket used by the name service is subscribed to receive endpoint information via a mechanism similar to the SOL_QRTR QRTR_REPORT_ENDPOINT socket option. Internal data structures are then extended to track endpoint information in addition to nodes and ports.
The name service directs packets to the endpoint originating the request. For NEW_SERVER and DEL_SERVER messages triggered using lookups or due to remote endpoints sending the corresponding message, qrtr name service generates a NEW_SERVER and DEL_SERVER messages to all local sockets registered to receive such notifications. The messages are made to look as if they're coming from the remote endpoint using a special extension to AF_QRTR sendmsg operation. This extension only works for the local socket that owns the QRTR_PORT_CTRL port (name service).
---
v2:
- rebase against current master
- drop socket locking from qrtr_sock_set_report_endpoint() as per review comment
--- net/qrtr/af_qrtr.c | 12 ++ net/qrtr/ns.c | 296 +++++++++++++++++++++++++++------------------ net/qrtr/qrtr.h | 1 + 3 files changed, 191 insertions(+), 118 deletions(-)
net/qrtr/af_qrtr.c +12 -0
@@ -107,6 +107,13 @@ static inline struct qrtr_sock *qrtr_sk(struct sock *sk) return container_of(sk, struct qrtr_sock, sk); } +void qrtr_sock_set_report_endpoint (struct sock *sk) +{ +struct qrtr_sock *ipc =qrtr_sk (sk); + +assign_bit (QRTR_F_REPORT_ENDPOINT , &ipc->flags,1 ); +} +int qrtr_msg_get_endpoint (struct msghdr *msg,u32 *out_endpoint_id) {struct cmsghdr *cmsg;
@@ -1092,6 +1099,11 @@ static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)/* control messages already require the type as 'command' */ skb_copy_bits (skb,0 , &qrtr_type,4 ); +/* +* Allow local name service to make packets appear as if +* they originated remotely +*/ + endpoint_id = msg_endpoint_id; } type =le32_to_cpu (qrtr_type);
net/qrtr/ns.c +178 -118
632 lines not on this page · raw message
net/qrtr/qrtr.h +1 -0
@@ -35,5 +35,6 @@ int qrtr_ns_init(void);void qrtr_ns_remove (void );int qrtr_msg_get_endpoint (struct msghdr *msg,u32 *out_endpoint_id); +void qrtr_sock_set_report_endpoint (struct sock *sk);#endif
Signature · 1 line
2.50.0
PATCH v2 10/10 net: qrtr: mhi: Report endpoint id in sysfs
In reply to Mihai MoldovanFrom: Denis Kenzior <[email protected]>
Add a read-only 'endpoint' sysfs entry that contains the qrtr endpoint identifier assigned to this mhi device. Can be used to direct / receive qrtr traffic only from a particular MHI device.
---
v2:
- rebase against current master
- use %u formatter instead of %d when printing endpoint id (u32) as per review comment
--- net/qrtr/mhi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
net/qrtr/mhi.c +14 -0
@@ -72,6 +72,16 @@ static int qcom_mhi_qrtr_send(struct qrtr_endpoint *ep, struct sk_buff *skb)return rc; } +static ssize_t endpoint_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +struct qrtr_mhi_dev *qdev =dev_get_drvdata (dev); + +return sprintf (buf,"%u\n" , qdev->ep.id); +} + +static DEVICE_ATTR_RO (endpoint); +static int qcom_mhi_qrtr_probe (struct mhi_device *mhi_dev,const struct mhi_device_id *id) {
@@ -91,6 +101,9 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,if (rc)return rc; +if (device_create_file (&mhi_dev->dev, &dev_attr_endpoint) <0 ) +dev_err (qdev->dev,"Failed to create endpoint attribute\n" ); +/* start channels */ rc =mhi_prepare_for_transfer_autoqueue (mhi_dev);if (rc) {
@@ -107,6 +120,7 @@ static void qcom_mhi_qrtr_remove(struct mhi_device *mhi_dev) {struct qrtr_mhi_dev *qdev =dev_get_drvdata (&mhi_dev->dev); +device_remove_file (&mhi_dev->dev, &dev_attr_endpoint);qrtr_endpoint_unregister (&qdev->ep);mhi_unprepare_from_transfer (mhi_dev);dev_set_drvdata (&mhi_dev->dev,NULL );
Signature · 1 line
2.50.0
Quoted 5 lines · * On 7/19/25 20:59, Mihai Moldovan wrote:
NOTE: There is 32-bit unsafe use of
radix_tree_insertin this patch set. This follows the existing usage insidenet/qrtr/af_qrtr.cinqrtr_tx_wait(),qrtr_tx_resume()andqrtr_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.
Since this is an actual problem and has to be eventually resolved, I'd like to ask for some guidance.
The Radix Tree API is fixed to using unsigned long keys, and my best idea (and the easiest thing to implement) thus far is to just go with that and restrict node IDs, endpoint IDs and port numbers to sizeof(unsigned long) / 2 bytes, which for platforms with 32-bit longs would be 16 bits. Not quite ideal, but probably good enough at the very least for port numbers (I figure).
Something like that:
#define RADIX_TREE_HALF_INDEX_BITS (RADIX_TREE_INDEX_BITS >> 1)
#define RADIX_TREE_HALF_INDEX_MAX_VALUE ((unsigned long)(-1) >> RADIX_TREE_HALF_INDEX_BITS)
with checks to make sure that node IDs, endpoint IDs and port numbers fit within RADIX_TREE_HALF_INDEX_MAX_VALUE.
Is this limitation acceptable? How big can node IDs get, also accounting for uncommon (and maybe also unrealistic) but conceivable use cases?
Mihai