* [PATCH net, 0/2] Fix send indirection table offset @ 2019-11-18 16:33 Haiyang Zhang 2019-11-18 16:34 ` [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() Haiyang Zhang 2019-11-18 16:34 ` [PATCH net, 2/2] hv_netvsc: Fix send_table offset in case of a host bug Haiyang Zhang 0 siblings, 2 replies; 8+ messages in thread From: Haiyang Zhang @ 2019-11-18 16:33 UTC (permalink / raw) To: sashal, linux-hyperv, netdev Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf, vkuznets, davem, linux-kernel Fix send indirection table offset issues related to guest and host bugs. Haiyang Zhang (2): hv_netvsc: Fix offset usage in netvsc_send_table() hv_netvsc: Fix send_table offset in case of a host bug drivers/net/hyperv/hyperv_net.h | 3 ++- drivers/net/hyperv/netvsc.c | 35 +++++++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 9 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() 2019-11-18 16:33 [PATCH net, 0/2] Fix send indirection table offset Haiyang Zhang @ 2019-11-18 16:34 ` Haiyang Zhang 2019-11-18 17:28 ` Vitaly Kuznetsov 2019-11-18 16:34 ` [PATCH net, 2/2] hv_netvsc: Fix send_table offset in case of a host bug Haiyang Zhang 1 sibling, 1 reply; 8+ messages in thread From: Haiyang Zhang @ 2019-11-18 16:34 UTC (permalink / raw) To: sashal, linux-hyperv, netdev Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf, vkuznets, davem, linux-kernel To reach the data region, the existing code adds offset in struct nvsp_5_send_indirect_table on the beginning of this struct. But the offset should be based on the beginning of its container, struct nvsp_message. This bug causes the first table entry missing, and adds an extra zero from the zero pad after the data region. This can put extra burden on the channel 0. So, correct the offset usage. Also add a boundary check to ensure not reading beyond data region. Fixes: 5b54dac856cb ("hyperv: Add support for virtual Receive Side Scaling (vRSS)") Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> --- drivers/net/hyperv/hyperv_net.h | 3 ++- drivers/net/hyperv/netvsc.c | 26 ++++++++++++++++++-------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h index 670ef68..fb547f3 100644 --- a/drivers/net/hyperv/hyperv_net.h +++ b/drivers/net/hyperv/hyperv_net.h @@ -609,7 +609,8 @@ struct nvsp_5_send_indirect_table { /* The number of entries in the send indirection table */ u32 count; - /* The offset of the send indirection table from top of this struct. + /* The offset of the send indirection table from the beginning of + * struct nvsp_message. * The send indirection table tells which channel to put the send * traffic on. Each entry is a channel number. */ diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index d22a36f..efd30e2 100644 --- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c @@ -1178,20 +1178,28 @@ static int netvsc_receive(struct net_device *ndev, } static void netvsc_send_table(struct net_device *ndev, - const struct nvsp_message *nvmsg) + const struct nvsp_message *nvmsg, + u32 msglen) { struct net_device_context *net_device_ctx = netdev_priv(ndev); - u32 count, *tab; + u32 count, offset, *tab; int i; count = nvmsg->msg.v5_msg.send_table.count; + offset = nvmsg->msg.v5_msg.send_table.offset; + if (count != VRSS_SEND_TAB_SIZE) { netdev_err(ndev, "Received wrong send-table size:%u\n", count); return; } - tab = (u32 *)((unsigned long)&nvmsg->msg.v5_msg.send_table + - nvmsg->msg.v5_msg.send_table.offset); + if (offset + count * sizeof(u32) > msglen) { + netdev_err(ndev, "Received send-table offset too big:%u\n", + offset); + return; + } + + tab = (void *)nvmsg + offset; for (i = 0; i < count; i++) net_device_ctx->tx_table[i] = tab[i]; @@ -1209,12 +1217,13 @@ static void netvsc_send_vf(struct net_device *ndev, net_device_ctx->vf_alloc ? "added" : "removed"); } -static void netvsc_receive_inband(struct net_device *ndev, - const struct nvsp_message *nvmsg) +static void netvsc_receive_inband(struct net_device *ndev, + const struct nvsp_message *nvmsg, + u32 msglen) { switch (nvmsg->hdr.msg_type) { case NVSP_MSG5_TYPE_SEND_INDIRECTION_TABLE: - netvsc_send_table(ndev, nvmsg); + netvsc_send_table(ndev, nvmsg, msglen); break; case NVSP_MSG4_TYPE_SEND_VF_ASSOCIATION: @@ -1232,6 +1241,7 @@ static int netvsc_process_raw_pkt(struct hv_device *device, { struct vmbus_channel *channel = nvchan->channel; const struct nvsp_message *nvmsg = hv_pkt_data(desc); + u32 msglen = hv_pkt_datalen(desc); trace_nvsp_recv(ndev, channel, nvmsg); @@ -1247,7 +1257,7 @@ static int netvsc_process_raw_pkt(struct hv_device *device, break; case VM_PKT_DATA_INBAND: - netvsc_receive_inband(ndev, nvmsg); + netvsc_receive_inband(ndev, nvmsg, msglen); break; default: -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() 2019-11-18 16:34 ` [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() Haiyang Zhang @ 2019-11-18 17:28 ` Vitaly Kuznetsov 2019-11-18 17:59 ` Stephen Hemminger 2019-11-18 18:39 ` Haiyang Zhang 0 siblings, 2 replies; 8+ messages in thread From: Vitaly Kuznetsov @ 2019-11-18 17:28 UTC (permalink / raw) To: Haiyang Zhang Cc: KY Srinivasan, Stephen Hemminger, olaf, davem, linux-kernel, sashal, linux-hyperv, netdev Haiyang Zhang <haiyangz@microsoft.com> writes: > To reach the data region, the existing code adds offset in struct > nvsp_5_send_indirect_table on the beginning of this struct. But the > offset should be based on the beginning of its container, > struct nvsp_message. This bug causes the first table entry missing, > and adds an extra zero from the zero pad after the data region. > This can put extra burden on the channel 0. > > So, correct the offset usage. Also add a boundary check to ensure > not reading beyond data region. > > Fixes: 5b54dac856cb ("hyperv: Add support for virtual Receive Side Scaling (vRSS)") > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > --- > drivers/net/hyperv/hyperv_net.h | 3 ++- > drivers/net/hyperv/netvsc.c | 26 ++++++++++++++++++-------- > 2 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h > index 670ef68..fb547f3 100644 > --- a/drivers/net/hyperv/hyperv_net.h > +++ b/drivers/net/hyperv/hyperv_net.h > @@ -609,7 +609,8 @@ struct nvsp_5_send_indirect_table { > /* The number of entries in the send indirection table */ > u32 count; > > - /* The offset of the send indirection table from top of this struct. > + /* The offset of the send indirection table from the beginning of > + * struct nvsp_message. > * The send indirection table tells which channel to put the send > * traffic on. Each entry is a channel number. > */ > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c > index d22a36f..efd30e2 100644 > --- a/drivers/net/hyperv/netvsc.c > +++ b/drivers/net/hyperv/netvsc.c > @@ -1178,20 +1178,28 @@ static int netvsc_receive(struct net_device *ndev, > } > > static void netvsc_send_table(struct net_device *ndev, > - const struct nvsp_message *nvmsg) > + const struct nvsp_message *nvmsg, > + u32 msglen) > { > struct net_device_context *net_device_ctx = netdev_priv(ndev); > - u32 count, *tab; > + u32 count, offset, *tab; > int i; > > count = nvmsg->msg.v5_msg.send_table.count; > + offset = nvmsg->msg.v5_msg.send_table.offset; > + > if (count != VRSS_SEND_TAB_SIZE) { > netdev_err(ndev, "Received wrong send-table size:%u\n", count); > return; > } > > - tab = (u32 *)((unsigned long)&nvmsg->msg.v5_msg.send_table + > - nvmsg->msg.v5_msg.send_table.offset); > + if (offset + count * sizeof(u32) > msglen) { Nit: I think this can overflow. > + netdev_err(ndev, "Received send-table offset too big:%u\n", > + offset); > + return; > + } > + > + tab = (void *)nvmsg + offset; But tab is 'u32 *', doesn't compiler complain? > > for (i = 0; i < count; i++) > net_device_ctx->tx_table[i] = tab[i]; > @@ -1209,12 +1217,13 @@ static void netvsc_send_vf(struct net_device *ndev, > net_device_ctx->vf_alloc ? "added" : "removed"); > } > > -static void netvsc_receive_inband(struct net_device *ndev, > - const struct nvsp_message *nvmsg) > +static void netvsc_receive_inband(struct net_device *ndev, > + const struct nvsp_message *nvmsg, > + u32 msglen) > { > switch (nvmsg->hdr.msg_type) { > case NVSP_MSG5_TYPE_SEND_INDIRECTION_TABLE: > - netvsc_send_table(ndev, nvmsg); > + netvsc_send_table(ndev, nvmsg, msglen); > break; > > case NVSP_MSG4_TYPE_SEND_VF_ASSOCIATION: > @@ -1232,6 +1241,7 @@ static int netvsc_process_raw_pkt(struct hv_device *device, > { > struct vmbus_channel *channel = nvchan->channel; > const struct nvsp_message *nvmsg = hv_pkt_data(desc); > + u32 msglen = hv_pkt_datalen(desc); > > trace_nvsp_recv(ndev, channel, nvmsg); > > @@ -1247,7 +1257,7 @@ static int netvsc_process_raw_pkt(struct hv_device *device, > break; > > case VM_PKT_DATA_INBAND: > - netvsc_receive_inband(ndev, nvmsg); > + netvsc_receive_inband(ndev, nvmsg, msglen); > break; > > default: -- Vitaly ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() 2019-11-18 17:28 ` Vitaly Kuznetsov @ 2019-11-18 17:59 ` Stephen Hemminger 2019-11-18 18:39 ` Haiyang Zhang 1 sibling, 0 replies; 8+ messages in thread From: Stephen Hemminger @ 2019-11-18 17:59 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf, davem, linux-kernel, sashal, linux-hyperv, netdev On Mon, 18 Nov 2019 18:28:48 +0100 Vitaly Kuznetsov <vkuznets@redhat.com> wrote: > > + netdev_err(ndev, "Received send-table offset too big:%u\n", > > + offset); > > + return; > > + } > > + > > + tab = (void *)nvmsg + offset; > > But tab is 'u32 *', doesn't compiler complain? nvmsg + offset is still of type void *. assigning void * to another pointer type is allowed with C. ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() 2019-11-18 17:28 ` Vitaly Kuznetsov 2019-11-18 17:59 ` Stephen Hemminger @ 2019-11-18 18:39 ` Haiyang Zhang 2019-11-18 20:47 ` Michael Kelley 1 sibling, 1 reply; 8+ messages in thread From: Haiyang Zhang @ 2019-11-18 18:39 UTC (permalink / raw) To: vkuznets Cc: KY Srinivasan, Stephen Hemminger, olaf, davem, linux-kernel, sashal, linux-hyperv, netdev > -----Original Message----- > From: Vitaly Kuznetsov <vkuznets@redhat.com> > Sent: Monday, November 18, 2019 12:29 PM > To: Haiyang Zhang <haiyangz@microsoft.com> > Cc: KY Srinivasan <kys@microsoft.com>; Stephen Hemminger > <sthemmin@microsoft.com>; olaf@aepfle.de; davem@davemloft.net; linux- > kernel@vger.kernel.org; sashal@kernel.org; linux-hyperv@vger.kernel.org; > netdev@vger.kernel.org > Subject: Re: [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() > > Haiyang Zhang <haiyangz@microsoft.com> writes: > > > To reach the data region, the existing code adds offset in struct > > nvsp_5_send_indirect_table on the beginning of this struct. But the > > offset should be based on the beginning of its container, > > struct nvsp_message. This bug causes the first table entry missing, > > and adds an extra zero from the zero pad after the data region. > > This can put extra burden on the channel 0. > > > > So, correct the offset usage. Also add a boundary check to ensure > > not reading beyond data region. > > > > Fixes: 5b54dac856cb ("hyperv: Add support for virtual Receive Side Scaling > (vRSS)") > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > > --- > > drivers/net/hyperv/hyperv_net.h | 3 ++- > > drivers/net/hyperv/netvsc.c | 26 ++++++++++++++++++-------- > > 2 files changed, 20 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/net/hyperv/hyperv_net.h > b/drivers/net/hyperv/hyperv_net.h > > index 670ef68..fb547f3 100644 > > --- a/drivers/net/hyperv/hyperv_net.h > > +++ b/drivers/net/hyperv/hyperv_net.h > > @@ -609,7 +609,8 @@ struct nvsp_5_send_indirect_table { > > /* The number of entries in the send indirection table */ > > u32 count; > > > > - /* The offset of the send indirection table from top of this struct. > > + /* The offset of the send indirection table from the beginning of > > + * struct nvsp_message. > > * The send indirection table tells which channel to put the send > > * traffic on. Each entry is a channel number. > > */ > > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c > > index d22a36f..efd30e2 100644 > > --- a/drivers/net/hyperv/netvsc.c > > +++ b/drivers/net/hyperv/netvsc.c > > @@ -1178,20 +1178,28 @@ static int netvsc_receive(struct net_device *ndev, > > } > > > > static void netvsc_send_table(struct net_device *ndev, > > - const struct nvsp_message *nvmsg) > > + const struct nvsp_message *nvmsg, > > + u32 msglen) > > { > > struct net_device_context *net_device_ctx = netdev_priv(ndev); > > - u32 count, *tab; > > + u32 count, offset, *tab; > > int i; > > > > count = nvmsg->msg.v5_msg.send_table.count; > > + offset = nvmsg->msg.v5_msg.send_table.offset; > > + > > if (count != VRSS_SEND_TAB_SIZE) { > > netdev_err(ndev, "Received wrong send-table size:%u\n", > count); > > return; > > } > > > > - tab = (u32 *)((unsigned long)&nvmsg->msg.v5_msg.send_table + > > - nvmsg->msg.v5_msg.send_table.offset); > > + if (offset + count * sizeof(u32) > msglen) { > > Nit: I think this can overflow. To prevent overflow, I will change it to: if (offset > msglen || offset + count * sizeof(u32) > msglen) { Thanks, - Haiyang ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() 2019-11-18 18:39 ` Haiyang Zhang @ 2019-11-18 20:47 ` Michael Kelley 2019-11-18 21:01 ` Haiyang Zhang 0 siblings, 1 reply; 8+ messages in thread From: Michael Kelley @ 2019-11-18 20:47 UTC (permalink / raw) To: Haiyang Zhang, vkuznets Cc: KY Srinivasan, Stephen Hemminger, olaf, davem, linux-kernel, sashal, linux-hyperv, netdev From: Haiyang Zhang <haiyangz@microsoft.com> Sent: Monday, November 18, 2019 10:40 AM > > -----Original Message----- > > From: Vitaly Kuznetsov <vkuznets@redhat.com> > > Sent: Monday, November 18, 2019 12:29 PM > > > > Haiyang Zhang <haiyangz@microsoft.com> writes: > > > > > To reach the data region, the existing code adds offset in struct > > > nvsp_5_send_indirect_table on the beginning of this struct. But the > > > offset should be based on the beginning of its container, > > > struct nvsp_message. This bug causes the first table entry missing, > > > and adds an extra zero from the zero pad after the data region. > > > This can put extra burden on the channel 0. > > > > > > So, correct the offset usage. Also add a boundary check to ensure > > > not reading beyond data region. > > > > > > Fixes: 5b54dac856cb ("hyperv: Add support for virtual Receive Side Scaling > > (vRSS)") > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > > > --- > > > drivers/net/hyperv/hyperv_net.h | 3 ++- > > > drivers/net/hyperv/netvsc.c | 26 ++++++++++++++++++-------- > > > 2 files changed, 20 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/net/hyperv/hyperv_net.h > > b/drivers/net/hyperv/hyperv_net.h > > > index 670ef68..fb547f3 100644 > > > --- a/drivers/net/hyperv/hyperv_net.h > > > +++ b/drivers/net/hyperv/hyperv_net.h > > > @@ -609,7 +609,8 @@ struct nvsp_5_send_indirect_table { > > > /* The number of entries in the send indirection table */ > > > u32 count; > > > > > > - /* The offset of the send indirection table from top of this struct. > > > + /* The offset of the send indirection table from the beginning of > > > + * struct nvsp_message. > > > * The send indirection table tells which channel to put the send > > > * traffic on. Each entry is a channel number. > > > */ > > > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c > > > index d22a36f..efd30e2 100644 > > > --- a/drivers/net/hyperv/netvsc.c > > > +++ b/drivers/net/hyperv/netvsc.c > > > @@ -1178,20 +1178,28 @@ static int netvsc_receive(struct net_device *ndev, > > > } > > > > > > static void netvsc_send_table(struct net_device *ndev, > > > - const struct nvsp_message *nvmsg) > > > + const struct nvsp_message *nvmsg, > > > + u32 msglen) > > > { > > > struct net_device_context *net_device_ctx = netdev_priv(ndev); > > > - u32 count, *tab; > > > + u32 count, offset, *tab; > > > int i; > > > > > > count = nvmsg->msg.v5_msg.send_table.count; > > > + offset = nvmsg->msg.v5_msg.send_table.offset; > > > + > > > if (count != VRSS_SEND_TAB_SIZE) { > > > netdev_err(ndev, "Received wrong send-table size:%u\n", > > count); > > > return; > > > } > > > > > > - tab = (u32 *)((unsigned long)&nvmsg->msg.v5_msg.send_table + > > > - nvmsg->msg.v5_msg.send_table.offset); > > > + if (offset + count * sizeof(u32) > msglen) { > > > > Nit: I think this can overflow. > > To prevent overflow, I will change it to: > if (offset > msglen || offset + count * sizeof(u32) > msglen) { > Thanks, > - Haiyang Actually, this would be simpler since we already trust msglen and count to have good values: if (offset > msglen - count * sizeof(u32)) { Michael ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() 2019-11-18 20:47 ` Michael Kelley @ 2019-11-18 21:01 ` Haiyang Zhang 0 siblings, 0 replies; 8+ messages in thread From: Haiyang Zhang @ 2019-11-18 21:01 UTC (permalink / raw) To: Michael Kelley, vkuznets Cc: KY Srinivasan, Stephen Hemminger, olaf, davem, linux-kernel, sashal, linux-hyperv, netdev > -----Original Message----- > From: Michael Kelley <mikelley@microsoft.com> > Sent: Monday, November 18, 2019 3:47 PM > To: Haiyang Zhang <haiyangz@microsoft.com>; vkuznets > <vkuznets@redhat.com> > Cc: KY Srinivasan <kys@microsoft.com>; Stephen Hemminger > <sthemmin@microsoft.com>; olaf@aepfle.de; davem@davemloft.net; linux- > kernel@vger.kernel.org; sashal@kernel.org; linux-hyperv@vger.kernel.org; > netdev@vger.kernel.org > Subject: RE: [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() > > From: Haiyang Zhang <haiyangz@microsoft.com> Sent: Monday, November 18, > 2019 10:40 AM > > > -----Original Message----- > > > From: Vitaly Kuznetsov <vkuznets@redhat.com> > > > Sent: Monday, November 18, 2019 12:29 PM > > > > > > Haiyang Zhang <haiyangz@microsoft.com> writes: > > > > > > > To reach the data region, the existing code adds offset in struct > > > > nvsp_5_send_indirect_table on the beginning of this struct. But the > > > > offset should be based on the beginning of its container, > > > > struct nvsp_message. This bug causes the first table entry missing, > > > > and adds an extra zero from the zero pad after the data region. > > > > This can put extra burden on the channel 0. > > > > > > > > So, correct the offset usage. Also add a boundary check to ensure > > > > not reading beyond data region. > > > > > > > > Fixes: 5b54dac856cb ("hyperv: Add support for virtual Receive Side > Scaling > > > (vRSS)") > > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > > > > --- > > > > drivers/net/hyperv/hyperv_net.h | 3 ++- > > > > drivers/net/hyperv/netvsc.c | 26 ++++++++++++++++++-------- > > > > 2 files changed, 20 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/drivers/net/hyperv/hyperv_net.h > > > b/drivers/net/hyperv/hyperv_net.h > > > > index 670ef68..fb547f3 100644 > > > > --- a/drivers/net/hyperv/hyperv_net.h > > > > +++ b/drivers/net/hyperv/hyperv_net.h > > > > @@ -609,7 +609,8 @@ struct nvsp_5_send_indirect_table { > > > > /* The number of entries in the send indirection table */ > > > > u32 count; > > > > > > > > - /* The offset of the send indirection table from top of this struct. > > > > + /* The offset of the send indirection table from the beginning of > > > > + * struct nvsp_message. > > > > * The send indirection table tells which channel to put the send > > > > * traffic on. Each entry is a channel number. > > > > */ > > > > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c > > > > index d22a36f..efd30e2 100644 > > > > --- a/drivers/net/hyperv/netvsc.c > > > > +++ b/drivers/net/hyperv/netvsc.c > > > > @@ -1178,20 +1178,28 @@ static int netvsc_receive(struct net_device > *ndev, > > > > } > > > > > > > > static void netvsc_send_table(struct net_device *ndev, > > > > - const struct nvsp_message *nvmsg) > > > > + const struct nvsp_message *nvmsg, > > > > + u32 msglen) > > > > { > > > > struct net_device_context *net_device_ctx = netdev_priv(ndev); > > > > - u32 count, *tab; > > > > + u32 count, offset, *tab; > > > > int i; > > > > > > > > count = nvmsg->msg.v5_msg.send_table.count; > > > > + offset = nvmsg->msg.v5_msg.send_table.offset; > > > > + > > > > if (count != VRSS_SEND_TAB_SIZE) { > > > > netdev_err(ndev, "Received wrong send-table size:%u\n", > > > count); > > > > return; > > > > } > > > > > > > > - tab = (u32 *)((unsigned long)&nvmsg->msg.v5_msg.send_table + > > > > - nvmsg->msg.v5_msg.send_table.offset); > > > > + if (offset + count * sizeof(u32) > msglen) { > > > > > > Nit: I think this can overflow. > > > > To prevent overflow, I will change it to: > > if (offset > msglen || offset + count * sizeof(u32) > msglen) { > > Thanks, > > - Haiyang > > Actually, this would be simpler since we already trust msglen and count > to have good values: > > if (offset > msglen - count * sizeof(u32)) { Great idea. Thanks! ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net, 2/2] hv_netvsc: Fix send_table offset in case of a host bug 2019-11-18 16:33 [PATCH net, 0/2] Fix send indirection table offset Haiyang Zhang 2019-11-18 16:34 ` [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() Haiyang Zhang @ 2019-11-18 16:34 ` Haiyang Zhang 1 sibling, 0 replies; 8+ messages in thread From: Haiyang Zhang @ 2019-11-18 16:34 UTC (permalink / raw) To: sashal, linux-hyperv, netdev Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf, vkuznets, davem, linux-kernel If negotiated NVSP version <= NVSP_PROTOCOL_VERSION_6, the offset may be wrong (too small) due to a host bug. This can cause missing the end of the send indirection table, and add multiple zero entries from leading zeros before the data region. This bug adds extra burden on channel 0. So fix the offset by computing it from the end of the table. This will ensure netvsc driver runs normally on unfixed hosts, and future fixed hosts. Fixes: 5b54dac856cb ("hyperv: Add support for virtual Receive Side Scaling (vRSS)") Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> --- drivers/net/hyperv/netvsc.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index efd30e2..7c5481a 100644 --- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c @@ -1178,6 +1178,7 @@ static int netvsc_receive(struct net_device *ndev, } static void netvsc_send_table(struct net_device *ndev, + struct netvsc_device *nvscdev, const struct nvsp_message *nvmsg, u32 msglen) { @@ -1193,6 +1194,13 @@ static void netvsc_send_table(struct net_device *ndev, return; } + /* If negotiated version <= NVSP_PROTOCOL_VERSION_6, the offset may be + * wrong due to a host bug. So fix the offset here. + */ + if (nvscdev->nvsp_version <= NVSP_PROTOCOL_VERSION_6) + offset = msglen - count * sizeof(u32); + + /* Boundary check for all versions */ if (offset + count * sizeof(u32) > msglen) { netdev_err(ndev, "Received send-table offset too big:%u\n", offset); @@ -1218,12 +1226,13 @@ static void netvsc_send_vf(struct net_device *ndev, } static void netvsc_receive_inband(struct net_device *ndev, + struct netvsc_device *nvscdev, const struct nvsp_message *nvmsg, u32 msglen) { switch (nvmsg->hdr.msg_type) { case NVSP_MSG5_TYPE_SEND_INDIRECTION_TABLE: - netvsc_send_table(ndev, nvmsg, msglen); + netvsc_send_table(ndev, nvscdev, nvmsg, msglen); break; case NVSP_MSG4_TYPE_SEND_VF_ASSOCIATION: @@ -1257,7 +1266,7 @@ static int netvsc_process_raw_pkt(struct hv_device *device, break; case VM_PKT_DATA_INBAND: - netvsc_receive_inband(ndev, nvmsg, msglen); + netvsc_receive_inband(ndev, net_device, nvmsg, msglen); break; default: -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-11-18 21:01 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-18 16:33 [PATCH net, 0/2] Fix send indirection table offset Haiyang Zhang 2019-11-18 16:34 ` [PATCH net, 1/2] hv_netvsc: Fix offset usage in netvsc_send_table() Haiyang Zhang 2019-11-18 17:28 ` Vitaly Kuznetsov 2019-11-18 17:59 ` Stephen Hemminger 2019-11-18 18:39 ` Haiyang Zhang 2019-11-18 20:47 ` Michael Kelley 2019-11-18 21:01 ` Haiyang Zhang 2019-11-18 16:34 ` [PATCH net, 2/2] hv_netvsc: Fix send_table offset in case of a host bug Haiyang Zhang
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).