* [PATCH v3 net] net: hv_netvsc: fix netvsc_send_completion to avoid multiple message length checks
@ 2023-08-03 0:45 Sonia Sharma
2023-08-03 2:04 ` Michael Kelley (LINUX)
2023-08-03 12:14 ` Simon Horman
0 siblings, 2 replies; 4+ messages in thread
From: Sonia Sharma @ 2023-08-03 0:45 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, netdev, sosha, kys, mikelley, haiyangz, wei.liu,
decui, longli, davem, edumazet, kuba, pabeni
From: Sonia Sharma <sonia.sharma@linux.microsoft.com>
The switch statement in netvsc_send_completion() is incorrectly validating
the length of incoming network packets by falling through to the next case.
Avoid the fallthrough. Instead break after a case match and then process
the complete() call.
Signed-off-by: Sonia Sharma <sonia.sharma@linux.microsoft.com>
---
Changes in v3:
* added return statement in default case as pointed by Michael Kelley..
---
drivers/net/hyperv/netvsc.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 82e9796c8f5e..0f7e4d377776 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -851,7 +851,7 @@ static void netvsc_send_completion(struct net_device *ndev,
msglen);
return;
}
- fallthrough;
+ break;
case NVSP_MSG1_TYPE_SEND_RECV_BUF_COMPLETE:
if (msglen < sizeof(struct nvsp_message_header) +
@@ -860,7 +860,7 @@ static void netvsc_send_completion(struct net_device *ndev,
msglen);
return;
}
- fallthrough;
+ break;
case NVSP_MSG1_TYPE_SEND_SEND_BUF_COMPLETE:
if (msglen < sizeof(struct nvsp_message_header) +
@@ -869,7 +869,7 @@ static void netvsc_send_completion(struct net_device *ndev,
msglen);
return;
}
- fallthrough;
+ break;
case NVSP_MSG5_TYPE_SUBCHANNEL:
if (msglen < sizeof(struct nvsp_message_header) +
@@ -878,10 +878,6 @@ static void netvsc_send_completion(struct net_device *ndev,
msglen);
return;
}
- /* Copy the response back */
- memcpy(&net_device->channel_init_pkt, nvsp_packet,
- sizeof(struct nvsp_message));
- complete(&net_device->channel_init_wait);
break;
case NVSP_MSG1_TYPE_SEND_RNDIS_PKT_COMPLETE:
@@ -904,13 +900,19 @@ static void netvsc_send_completion(struct net_device *ndev,
netvsc_send_tx_complete(ndev, net_device, incoming_channel,
desc, budget);
- break;
+ return;
default:
netdev_err(ndev,
"Unknown send completion type %d received!!\n",
nvsp_packet->hdr.msg_type);
+ return;
}
+
+ /* Copy the response back */
+ memcpy(&net_device->channel_init_pkt, nvsp_packet,
+ sizeof(struct nvsp_message));
+ complete(&net_device->channel_init_wait);
}
static u32 netvsc_get_next_send_section(struct netvsc_device *net_device)
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH v3 net] net: hv_netvsc: fix netvsc_send_completion to avoid multiple message length checks
2023-08-03 0:45 [PATCH v3 net] net: hv_netvsc: fix netvsc_send_completion to avoid multiple message length checks Sonia Sharma
@ 2023-08-03 2:04 ` Michael Kelley (LINUX)
2023-08-03 12:14 ` Simon Horman
1 sibling, 0 replies; 4+ messages in thread
From: Michael Kelley (LINUX) @ 2023-08-03 2:04 UTC (permalink / raw)
To: Sonia Sharma, linux-kernel@vger.kernel.org
Cc: linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
Sonia Sharma, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
Dexuan Cui, Long Li, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com
From: Sonia Sharma <sosha@linux.microsoft.com> Sent: Wednesday, August 2, 2023 5:45 PM
>
> The switch statement in netvsc_send_completion() is incorrectly validating
> the length of incoming network packets by falling through to the next case.
> Avoid the fallthrough. Instead break after a case match and then process
> the complete() call.
>
> Signed-off-by: Sonia Sharma <sonia.sharma@linux.microsoft.com>
> ---
> Changes in v3:
> * added return statement in default case as pointed by Michael Kelley..
> ---
> drivers/net/hyperv/netvsc.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 82e9796c8f5e..0f7e4d377776 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -851,7 +851,7 @@ static void netvsc_send_completion(struct net_device *ndev,
> msglen);
> return;
> }
> - fallthrough;
> + break;
>
> case NVSP_MSG1_TYPE_SEND_RECV_BUF_COMPLETE:
> if (msglen < sizeof(struct nvsp_message_header) +
> @@ -860,7 +860,7 @@ static void netvsc_send_completion(struct net_device *ndev,
> msglen);
> return;
> }
> - fallthrough;
> + break;
>
> case NVSP_MSG1_TYPE_SEND_SEND_BUF_COMPLETE:
> if (msglen < sizeof(struct nvsp_message_header) +
> @@ -869,7 +869,7 @@ static void netvsc_send_completion(struct net_device *ndev,
> msglen);
> return;
> }
> - fallthrough;
> + break;
>
> case NVSP_MSG5_TYPE_SUBCHANNEL:
> if (msglen < sizeof(struct nvsp_message_header) +
> @@ -878,10 +878,6 @@ static void netvsc_send_completion(struct net_device *ndev,
> msglen);
> return;
> }
> - /* Copy the response back */
> - memcpy(&net_device->channel_init_pkt, nvsp_packet,
> - sizeof(struct nvsp_message));
> - complete(&net_device->channel_init_wait);
> break;
>
> case NVSP_MSG1_TYPE_SEND_RNDIS_PKT_COMPLETE:
> @@ -904,13 +900,19 @@ static void netvsc_send_completion(struct net_device
> *ndev,
>
> netvsc_send_tx_complete(ndev, net_device, incoming_channel,
> desc, budget);
> - break;
> + return;
>
> default:
> netdev_err(ndev,
> "Unknown send completion type %d received!!\n",
> nvsp_packet->hdr.msg_type);
> + return;
> }
> +
> + /* Copy the response back */
> + memcpy(&net_device->channel_init_pkt, nvsp_packet,
> + sizeof(struct nvsp_message));
> + complete(&net_device->channel_init_wait);
> }
>
> static u32 netvsc_get_next_send_section(struct netvsc_device *net_device)
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 net] net: hv_netvsc: fix netvsc_send_completion to avoid multiple message length checks
2023-08-03 0:45 [PATCH v3 net] net: hv_netvsc: fix netvsc_send_completion to avoid multiple message length checks Sonia Sharma
2023-08-03 2:04 ` Michael Kelley (LINUX)
@ 2023-08-03 12:14 ` Simon Horman
2023-08-03 17:23 ` Jakub Kicinski
1 sibling, 1 reply; 4+ messages in thread
From: Simon Horman @ 2023-08-03 12:14 UTC (permalink / raw)
To: Sonia Sharma
Cc: linux-kernel, linux-hyperv, netdev, sosha, kys, mikelley,
haiyangz, wei.liu, decui, longli, davem, edumazet, kuba, pabeni
On Wed, Aug 02, 2023 at 05:45:28PM -0700, Sonia Sharma wrote:
> From: Sonia Sharma <sonia.sharma@linux.microsoft.com>
>
> The switch statement in netvsc_send_completion() is incorrectly validating
> the length of incoming network packets by falling through to the next case.
> Avoid the fallthrough. Instead break after a case match and then process
> the complete() call.
>
> Signed-off-by: Sonia Sharma <sonia.sharma@linux.microsoft.com>
Hi Sonia,
if this is a bug-fix, which seems to be the case, then it probably warrants
a Fixes tag.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 net] net: hv_netvsc: fix netvsc_send_completion to avoid multiple message length checks
2023-08-03 12:14 ` Simon Horman
@ 2023-08-03 17:23 ` Jakub Kicinski
0 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2023-08-03 17:23 UTC (permalink / raw)
To: Simon Horman
Cc: Sonia Sharma, linux-kernel, linux-hyperv, netdev, sosha, kys,
mikelley, haiyangz, wei.liu, decui, longli, davem, edumazet,
pabeni
On Thu, 3 Aug 2023 14:14:01 +0200 Simon Horman wrote:
> > The switch statement in netvsc_send_completion() is incorrectly validating
> > the length of incoming network packets by falling through to the next case.
> > Avoid the fallthrough. Instead break after a case match and then process
> > the complete() call.
> >
> > Signed-off-by: Sonia Sharma <sonia.sharma@linux.microsoft.com>
>
> Hi Sonia,
>
> if this is a bug-fix, which seems to be the case, then it probably warrants
> a Fixes tag.
And a description of what this problem results in. The commit message
kinda tells us what the patch does, which we already see from the code.
Paraphrasing corporate America "focus on the impact"...
--
pw-bot: cr
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-03 17:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-03 0:45 [PATCH v3 net] net: hv_netvsc: fix netvsc_send_completion to avoid multiple message length checks Sonia Sharma
2023-08-03 2:04 ` Michael Kelley (LINUX)
2023-08-03 12:14 ` Simon Horman
2023-08-03 17:23 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).