diff mbox series

[V2,net-next,1/2] net: qrtr: Prevent stale ports from sending

Message ID 20230920053317.2165867-2-quic_srichara@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series net: qrtr: Few qrtr changes | expand

Commit Message

Sricharan Ramabadhran Sept. 20, 2023, 5:33 a.m. UTC 
From: Chris Lew <quic_clew@quicinc.com>

If some client tries to initialize a QRTR socket during QRTR
init, the socket will become stale after the ns(namespace server)
binds to the QRTR control port. The client should close and reopen
the QRTR socket once ENETRESET is posted to the stale socket.

There is a possibility that a client tries to send to the NS before
processing the ENETRESET. In the case of a NEW_SERVER control message,
the control message will reach the NS and be forwarded to the firmware.
The client will then process the ENETRESET closing and re-opening the
socket which triggers a DEL_SERVER and then a second NEW_SERVER.
This scenario will give an unnecessary disconnect to the clients on the
firmware who were able to initialize on the first NEW_SERVER.

This was seen when qrtr-ns was a separate application, but there is
still a potential gap between AF_QIPCRTR socket register and when
qrtr_ns_init binds to the socket where this issue can still occur.

Signed-off-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Vignesh Viswanathan <quic_viswanat@quicinc.com>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
---
 [v2]  Added more appropriate commit text,
       Removed a redundant check and fixed local variables
       in reverse-christmas tree order.
       Added 'Chris Lew' Signed-off tag.

 net/qrtr/af_qrtr.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Bjorn Andersson Sept. 20, 2023, 12:48 p.m. UTC | #1 
On Wed, Sep 20, 2023 at 11:03:16AM +0530, Sricharan Ramabadhran wrote:
> From: Chris Lew <quic_clew@quicinc.com>
> 
> If some client tries to initialize a QRTR socket during QRTR
> init, the socket will become stale after the ns(namespace server)
> binds to the QRTR control port. The client should close and reopen
> the QRTR socket once ENETRESET is posted to the stale socket.
> 
> There is a possibility that a client tries to send to the NS before
> processing the ENETRESET. In the case of a NEW_SERVER control message,
> the control message will reach the NS and be forwarded to the firmware.
> The client will then process the ENETRESET closing and re-opening the
> socket which triggers a DEL_SERVER and then a second NEW_SERVER.
> This scenario will give an unnecessary disconnect to the clients on the
> firmware who were able to initialize on the first NEW_SERVER.
> 
> This was seen when qrtr-ns was a separate application, but there is
> still a potential gap between AF_QIPCRTR socket register and when
> qrtr_ns_init binds to the socket where this issue can still occur.
> 
> Signed-off-by: Chris Lew <quic_clew@quicinc.com>
> Signed-off-by: Vignesh Viswanathan <quic_viswanat@quicinc.com>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn
diff mbox series

Patch

diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index 41ece61eb57a..e5cf4245c3dc 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -849,6 +849,7 @@  static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb,
 			      int type, struct sockaddr_qrtr *from,
 			      struct sockaddr_qrtr *to)
 {
+	struct sock *sk = skb->sk;
 	struct qrtr_sock *ipc;
 	struct qrtr_cb *cb;
 
@@ -860,6 +861,14 @@  static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb,
 		return -ENODEV;
 	}
 
+	/* Keep resetting NETRESET until socket is closed */
+	if (sk && sk->sk_err == ENETRESET) {
+		sk_error_report(sk);
+		qrtr_port_put(ipc);
+		kfree_skb(skb);
+		return 0;
+	}
+
 	cb = (struct qrtr_cb *)skb->cb;
 	cb->src_node = from->sq_node;
 	cb->src_port = from->sq_port;