Thread (3 messages) 3 messages, 3 authors, 2023-02-14

RE: [PATCH net-next v3 1/1] hv_netvsc: Check status in SEND_RNDIS_PKT completion message

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: 2023-02-13 13:52:29
Also in: linux-hyperv, lkml

quoted hunk ↗ jump to hunk
-----Original Message-----
From: Michael Kelley (LINUX) <redacted>
Sent: Monday, February 13, 2023 12:08 AM
To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
[off-list ref]; wei.liu@kernel.org; Dexuan Cui
[off-list ref]; davem@davemloft.net; edumazet@google.com;
kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; linux-
hyperv@vger.kernel.org; linux-kernel@vger.kernel.org
Cc: Michael Kelley (LINUX) <redacted>
Subject: [PATCH net-next v3 1/1] hv_netvsc: Check status in
SEND_RNDIS_PKT completion message

Completion responses to SEND_RNDIS_PKT messages are currently processed
regardless of the status in the response, so that resources associated
with the request are freed.  While this is appropriate, code bugs that
cause sending a malformed message, or errors on the Hyper-V host, go
undetected. Fix this by checking the status and outputting a rate-limited
message if there is an error.

Signed-off-by: Michael Kelley <redacted>
---

Changes in v3:
* Fix rate-limit logic when msglen is too small [Haiyang Zhang]

Changes in v2:
* Add rate-limiting to error messages [Haiyang Zhang]

 drivers/net/hyperv/netvsc.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 661bbe6..f702807 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -813,6 +813,7 @@ static void netvsc_send_completion(struct net_device
*ndev,
 	u32 msglen = hv_pkt_datalen(desc);
 	struct nvsp_message *pkt_rqst;
 	u64 cmd_rqst;
+	u32 status;

 	/* First check if this is a VMBUS completion without data payload */
 	if (!msglen) {
@@ -884,6 +885,23 @@ static void netvsc_send_completion(struct
net_device *ndev,
 		break;

 	case NVSP_MSG1_TYPE_SEND_RNDIS_PKT_COMPLETE:
+		if (msglen < sizeof(struct nvsp_message_header) +
+		    sizeof(struct
nvsp_1_message_send_rndis_packet_complete)) {
+			if (net_ratelimit())
+				netdev_err(ndev, "nvsp_rndis_pkt_complete
length too small: %u\n",
+					   msglen);
+			return;
+		}
+
+		/* If status indicates an error, output a message so we know
+		 * there's a problem. But process the completion anyway so
the
+		 * resources are released.
+		 */
+		status = nvsp_packet-
quoted
msg.v1_msg.send_rndis_pkt_complete.status;
+		if (status != NVSP_STAT_SUCCESS && net_ratelimit())
+			netdev_err(ndev, "nvsp_rndis_pkt_complete error
status: %x\n",
+				   status);
+
 		netvsc_send_tx_complete(ndev, net_device,
incoming_channel,
 					desc, budget);
 		break;
--
1.8.3.1
Thank you!

Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help