From ae068f561baa003d260475c3e441ca454b186726 Mon Sep 17 00:00:00 2001 From: Loic Poulain Date: Fri, 6 Nov 2020 18:33:26 +0100 Subject: [PATCH 1/5] net: qrtr: Fix port ID for control messages The port ID for control messages was uncorrectly set with broadcast node ID value, causing message to be dropped on remote side since not passing packet filtering (cb->dst_port != QRTR_PORT_CTRL). Fixes: d27e77a3de28 ("net: qrtr: Reset the node and port ID of broadcast messages") Signed-off-by: Loic Poulain Signed-off-by: Jakub Kicinski --- net/qrtr/qrtr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index 957aa9263ba4..d7134c558993 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -347,7 +347,7 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb, hdr->src_port_id = cpu_to_le32(from->sq_port); if (to->sq_port == QRTR_PORT_CTRL) { hdr->dst_node_id = cpu_to_le32(node->nid); - hdr->dst_port_id = cpu_to_le32(QRTR_NODE_BCAST); + hdr->dst_port_id = cpu_to_le32(QRTR_PORT_CTRL); } else { hdr->dst_node_id = cpu_to_le32(to->sq_node); hdr->dst_port_id = cpu_to_le32(to->sq_port); From 87f547c108e4a12c3284ca8298d2d81a371f840e Mon Sep 17 00:00:00 2001 From: Loic Poulain Date: Fri, 6 Nov 2020 18:33:27 +0100 Subject: [PATCH 2/5] net: qrtr: Allow forwarded services A remote endpoint (immediate neighbors node) can forward services from other nodes (non-immadiate), in that case ctrl packet node ID (offering distant service) can differ from the qrtr source node (forwarding the packet). Signed-off-by: Loic Poulain Reviewed-by: Bjorn Andersson Reviewed-by: Manivannan Sadhasivam Signed-off-by: Jakub Kicinski --- net/qrtr/ns.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c index b8559c882431..56aaf8cb6527 100644 --- a/net/qrtr/ns.c +++ b/net/qrtr/ns.c @@ -517,10 +517,6 @@ static int ctrl_cmd_new_server(struct sockaddr_qrtr *from, port = from->sq_port; } - /* Don't accept spoofed messages */ - if (from->sq_node != node_id) - return -EINVAL; - srv = server_add(service, instance, node_id, port); if (!srv) return -EINVAL; @@ -559,10 +555,6 @@ static int ctrl_cmd_del_server(struct sockaddr_qrtr *from, port = from->sq_port; } - /* Don't accept spoofed messages */ - if (from->sq_node != node_id) - return -EINVAL; - /* Local servers may only unregister themselves */ if (from->sq_node == qrtr_ns.local_node && from->sq_port != port) return -EINVAL; From 0baa99ee353c20e5acce15c675773d6d1b0bda05 Mon Sep 17 00:00:00 2001 From: Loic Poulain Date: Fri, 6 Nov 2020 18:33:28 +0100 Subject: [PATCH 3/5] net: qrtr: Allow non-immediate node routing In order to reach non-immediate remote node services that are accessed through an intermediate node, the route to the remote node needs to be saved. E.g for a [node1 <=> node2 <=> node3] network - node2 forwards node3 service to node1 - node1 must save node2 as route for reaching node3 Signed-off-by: Loic Poulain Reviewed-by: Bjorn Andersson Signed-off-by: Jakub Kicinski --- net/qrtr/qrtr.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index d7134c558993..5d9039cb9064 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -401,12 +401,13 @@ static void qrtr_node_assign(struct qrtr_node *node, unsigned int nid) { unsigned long flags; - if (node->nid != QRTR_EP_NID_AUTO || nid == QRTR_EP_NID_AUTO) + if (nid == QRTR_EP_NID_AUTO) return; spin_lock_irqsave(&qrtr_nodes_lock, flags); radix_tree_insert(&qrtr_nodes, nid, node); - node->nid = nid; + if (node->nid == QRTR_EP_NID_AUTO) + node->nid = nid; spin_unlock_irqrestore(&qrtr_nodes_lock, flags); } @@ -494,6 +495,13 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len) qrtr_node_assign(node, cb->src_node); + if (cb->type == QRTR_TYPE_NEW_SERVER) { + /* Remote node endpoint can bridge other distant nodes */ + const struct qrtr_ctrl_pkt *pkt = data + hdrlen; + + qrtr_node_assign(node, le32_to_cpu(pkt->server.node)); + } + if (cb->type == QRTR_TYPE_RESUME_TX) { qrtr_tx_resume(node, skb); } else { From f7dec6cb914c8923808eefc034bba8227a5dfe3a Mon Sep 17 00:00:00 2001 From: Loic Poulain Date: Fri, 6 Nov 2020 18:33:29 +0100 Subject: [PATCH 4/5] net: qrtr: Add GFP flags parameter to qrtr_alloc_ctrl_packet This will be requested for allocating control packet in atomic context. Signed-off-by: Loic Poulain Signed-off-by: Jakub Kicinski --- net/qrtr/qrtr.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index 5d9039cb9064..d4305deb3bdc 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -527,18 +527,20 @@ EXPORT_SYMBOL_GPL(qrtr_endpoint_post); /** * qrtr_alloc_ctrl_packet() - allocate control packet skb * @pkt: reference to qrtr_ctrl_pkt pointer + * @flags: the type of memory to allocate * * Returns newly allocated sk_buff, or NULL on failure * * This function allocates a sk_buff large enough to carry a qrtr_ctrl_pkt and * on success returns a reference to the control packet in @pkt. */ -static struct sk_buff *qrtr_alloc_ctrl_packet(struct qrtr_ctrl_pkt **pkt) +static struct sk_buff *qrtr_alloc_ctrl_packet(struct qrtr_ctrl_pkt **pkt, + gfp_t flags) { const int pkt_len = sizeof(struct qrtr_ctrl_pkt); struct sk_buff *skb; - skb = alloc_skb(QRTR_HDR_MAX_SIZE + pkt_len, GFP_KERNEL); + skb = alloc_skb(QRTR_HDR_MAX_SIZE + pkt_len, flags); if (!skb) return NULL; @@ -607,7 +609,7 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep) mutex_unlock(&node->ep_lock); /* Notify the local controller about the event */ - skb = qrtr_alloc_ctrl_packet(&pkt); + skb = qrtr_alloc_ctrl_packet(&pkt, GFP_KERNEL); if (skb) { pkt->cmd = cpu_to_le32(QRTR_TYPE_BYE); qrtr_local_enqueue(NULL, skb, QRTR_TYPE_BYE, &src, &dst); @@ -664,7 +666,7 @@ static void qrtr_port_remove(struct qrtr_sock *ipc) to.sq_node = QRTR_NODE_BCAST; to.sq_port = QRTR_PORT_CTRL; - skb = qrtr_alloc_ctrl_packet(&pkt); + skb = qrtr_alloc_ctrl_packet(&pkt, GFP_KERNEL); if (skb) { pkt->cmd = cpu_to_le32(QRTR_TYPE_DEL_CLIENT); pkt->client.node = cpu_to_le32(ipc->us.sq_node); @@ -990,7 +992,7 @@ static int qrtr_send_resume_tx(struct qrtr_cb *cb) if (!node) return -EINVAL; - skb = qrtr_alloc_ctrl_packet(&pkt); + skb = qrtr_alloc_ctrl_packet(&pkt, GFP_KERNEL); if (!skb) return -ENOMEM; From 90829f07baea7efe4ca77f79820f3849081186a8 Mon Sep 17 00:00:00 2001 From: Loic Poulain Date: Fri, 6 Nov 2020 18:33:30 +0100 Subject: [PATCH 5/5] net: qrtr: Release distant nodes along the bridge node Distant QRTR nodes can be accessed via an other node that acts as a bridge. When the a QRTR endpoint associated to a bridge node is released, all the linked distant nodes should also be released. This patch fixes endpoint release by: - Submitting QRTR BYE message locally on behalf of all the nodes accessible through the endpoint. - Removing all the routable node IDs from radix tree pointing to the released node endpoint. Signed-off-by: Loic Poulain Signed-off-by: Jakub Kicinski --- net/qrtr/qrtr.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index d4305deb3bdc..f4ab3ca6d73b 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -171,8 +171,13 @@ static void __qrtr_node_release(struct kref *kref) void __rcu **slot; spin_lock_irqsave(&qrtr_nodes_lock, flags); - if (node->nid != QRTR_EP_NID_AUTO) - radix_tree_delete(&qrtr_nodes, node->nid); + /* If the node is a bridge for other nodes, there are possibly + * multiple entries pointing to our released node, delete them all. + */ + radix_tree_for_each_slot(slot, &qrtr_nodes, &iter, 0) { + if (*slot == node) + radix_tree_iter_delete(&qrtr_nodes, &iter, slot); + } spin_unlock_irqrestore(&qrtr_nodes_lock, flags); list_del(&node->item); @@ -602,6 +607,7 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep) struct qrtr_ctrl_pkt *pkt; struct qrtr_tx_flow *flow; struct sk_buff *skb; + unsigned long flags; void __rcu **slot; mutex_lock(&node->ep_lock); @@ -609,11 +615,18 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep) mutex_unlock(&node->ep_lock); /* Notify the local controller about the event */ - skb = qrtr_alloc_ctrl_packet(&pkt, GFP_KERNEL); - if (skb) { - pkt->cmd = cpu_to_le32(QRTR_TYPE_BYE); - qrtr_local_enqueue(NULL, skb, QRTR_TYPE_BYE, &src, &dst); + spin_lock_irqsave(&qrtr_nodes_lock, flags); + radix_tree_for_each_slot(slot, &qrtr_nodes, &iter, 0) { + if (*slot != node) + continue; + src.sq_node = iter.index; + 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); + } } + spin_unlock_irqrestore(&qrtr_nodes_lock, flags); /* Wake up any transmitters waiting for resume-tx from the node */ mutex_lock(&node->qrtr_tx_lock);