| Message ID | 1597113194-90208-4-git-send-email-longli@linuxonhyperv.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Ferruh Yigit |
| Headers | show |
| Series | [1/4] net/netvsc: move rxbuf_info from per-device to per-queue | expand |
| Context | Check | Description |
|---|---|---|
| ci/travis-robot | success | Travis build: passed |
| ci/Intel-compilation | fail | Compilation issues |
| ci/checkpatch | success | coding style OK |
On Mon, 10 Aug 2020 19:33:14 -0700 longli@linuxonhyperv.com wrote: > From: Stephen Hemminger <stephen@networkplumber.org> > > The data from the host is trusted but checked by the driver. > One check that is missing is that the packet offset and length > might cause wraparound. > > Cc: stable@dpdk.org > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > Signed-off-by: Long Li <longli@microsoft.com> Reported-by: Nan Chen <whutchennan@gmail.com>
On Mon, 2020-08-10 at 19:33 -0700, longli@linuxonhyperv.com wrote: > From: Stephen Hemminger <stephen@networkplumber.org> > > The data from the host is trusted but checked by the driver. > One check that is missing is that the packet offset and length > might cause wraparound. > > Cc: stable@dpdk.org > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > Signed-off-by: Long Li <longli@microsoft.com> > --- > drivers/net/netvsc/hn_rxtx.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c > index a388ff258..d8d3f07f5 100644 > --- a/drivers/net/netvsc/hn_rxtx.c > +++ b/drivers/net/netvsc/hn_rxtx.c > @@ -666,7 +666,8 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq, > struct hn_rx_bufinfo *rxb, > void *data, uint32_t dlen) > { > - unsigned int data_off, data_len, pktinfo_off, pktinfo_len; > + unsigned int data_off, data_len, total_len; > + unsigned int pktinfo_off, pktinfo_len; > const struct rndis_packet_msg *pkt = data; > struct hn_rxinfo info = { > .vlan_info = HN_NDIS_VLAN_INFO_INVALID, > @@ -711,7 +712,8 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq, > goto error; > } > > - if (unlikely(data_off + data_len > pkt->len)) > + if (__builtin_add_overflow(data_off, data_len, &total_len) || > + total_len > pkt->len) > goto error; > > if (unlikely(data_len < RTE_ETHER_HDR_LEN)) This patch breaks the build with GCC < 5 (CentOS 7, RHEL 7, SLE 12) as __builtin_add_overflow is not available. Could you please send a follow up to fix it?
On 10/27/2020 5:10 PM, Luca Boccassi wrote: > On Mon, 2020-08-10 at 19:33 -0700, longli@linuxonhyperv.com wrote: >> From: Stephen Hemminger <stephen@networkplumber.org> >> >> The data from the host is trusted but checked by the driver. >> One check that is missing is that the packet offset and length >> might cause wraparound. >> >> Cc: stable@dpdk.org >> >> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> >> Signed-off-by: Long Li <longli@microsoft.com> >> --- >> drivers/net/netvsc/hn_rxtx.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c >> index a388ff258..d8d3f07f5 100644 >> --- a/drivers/net/netvsc/hn_rxtx.c >> +++ b/drivers/net/netvsc/hn_rxtx.c >> @@ -666,7 +666,8 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq, >> struct hn_rx_bufinfo *rxb, >> void *data, uint32_t dlen) >> { >> - unsigned int data_off, data_len, pktinfo_off, pktinfo_len; >> + unsigned int data_off, data_len, total_len; >> + unsigned int pktinfo_off, pktinfo_len; >> const struct rndis_packet_msg *pkt = data; >> struct hn_rxinfo info = { >> .vlan_info = HN_NDIS_VLAN_INFO_INVALID, >> @@ -711,7 +712,8 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq, >> goto error; >> } >> >> - if (unlikely(data_off + data_len > pkt->len)) >> + if (__builtin_add_overflow(data_off, data_len, &total_len) || >> + total_len > pkt->len) >> goto error; >> >> if (unlikely(data_len < RTE_ETHER_HDR_LEN)) > > This patch breaks the build with GCC < 5 (CentOS 7, RHEL 7, SLE 12) as > __builtin_add_overflow is not available. Could you please send a follow > up to fix it? > It should be already fixed in the repo: https://git.dpdk.org/dpdk/commit/?id=d73543b5f46d Are you getting the build error with 20.11-rc1?
On Tue, 2020-10-27 at 23:07 +0000, Ferruh Yigit wrote: > On 10/27/2020 5:10 PM, Luca Boccassi wrote: > > On Mon, 2020-08-10 at 19:33 -0700, longli@linuxonhyperv.com wrote: > > > From: Stephen Hemminger <stephen@networkplumber.org> > > > > > > The data from the host is trusted but checked by the driver. > > > One check that is missing is that the packet offset and length > > > might cause wraparound. > > > > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > > > Signed-off-by: Long Li <longli@microsoft.com> > > > --- > > > drivers/net/netvsc/hn_rxtx.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c > > > index a388ff258..d8d3f07f5 100644 > > > --- a/drivers/net/netvsc/hn_rxtx.c > > > +++ b/drivers/net/netvsc/hn_rxtx.c > > > @@ -666,7 +666,8 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq, > > > struct hn_rx_bufinfo *rxb, > > > void *data, uint32_t dlen) > > > { > > > - unsigned int data_off, data_len, pktinfo_off, pktinfo_len; > > > + unsigned int data_off, data_len, total_len; > > > + unsigned int pktinfo_off, pktinfo_len; > > > const struct rndis_packet_msg *pkt = data; > > > struct hn_rxinfo info = { > > > .vlan_info = HN_NDIS_VLAN_INFO_INVALID, > > > @@ -711,7 +712,8 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq, > > > goto error; > > > } > > > > > > - if (unlikely(data_off + data_len > pkt->len)) > > > + if (__builtin_add_overflow(data_off, data_len, &total_len) || > > > + total_len > pkt->len) > > > goto error; > > > > > > if (unlikely(data_len < RTE_ETHER_HDR_LEN)) > > > > This patch breaks the build with GCC < 5 (CentOS 7, RHEL 7, SLE 12) as > > __builtin_add_overflow is not available. Could you please send a follow > > up to fix it? > > > > It should be already fixed in the repo: > https://git.dpdk.org/dpdk/commit/?id=d73543b5f46d > > Are you getting the build error with 20.11-rc1? No, with the backport. The original patch was marked for stable, but the fixup was not. I'll pick it up.
On 10/28/2020 11:08 AM, Luca Boccassi wrote: > On Tue, 2020-10-27 at 23:07 +0000, Ferruh Yigit wrote: >> On 10/27/2020 5:10 PM, Luca Boccassi wrote: >>> On Mon, 2020-08-10 at 19:33 -0700, longli@linuxonhyperv.com wrote: >>>> From: Stephen Hemminger <stephen@networkplumber.org> >>>> >>>> The data from the host is trusted but checked by the driver. >>>> One check that is missing is that the packet offset and length >>>> might cause wraparound. >>>> >>>> Cc: stable@dpdk.org >>>> >>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> >>>> Signed-off-by: Long Li <longli@microsoft.com> >>>> --- >>>> drivers/net/netvsc/hn_rxtx.c | 6 ++++-- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c >>>> index a388ff258..d8d3f07f5 100644 >>>> --- a/drivers/net/netvsc/hn_rxtx.c >>>> +++ b/drivers/net/netvsc/hn_rxtx.c >>>> @@ -666,7 +666,8 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq, >>>> struct hn_rx_bufinfo *rxb, >>>> void *data, uint32_t dlen) >>>> { >>>> - unsigned int data_off, data_len, pktinfo_off, pktinfo_len; >>>> + unsigned int data_off, data_len, total_len; >>>> + unsigned int pktinfo_off, pktinfo_len; >>>> const struct rndis_packet_msg *pkt = data; >>>> struct hn_rxinfo info = { >>>> .vlan_info = HN_NDIS_VLAN_INFO_INVALID, >>>> @@ -711,7 +712,8 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq, >>>> goto error; >>>> } >>>> >>>> - if (unlikely(data_off + data_len > pkt->len)) >>>> + if (__builtin_add_overflow(data_off, data_len, &total_len) || >>>> + total_len > pkt->len) >>>> goto error; >>>> >>>> if (unlikely(data_len < RTE_ETHER_HDR_LEN)) >>> >>> This patch breaks the build with GCC < 5 (CentOS 7, RHEL 7, SLE 12) as >>> __builtin_add_overflow is not available. Could you please send a follow >>> up to fix it? >>> >> >> It should be already fixed in the repo: >> https://git.dpdk.org/dpdk/commit/?id=d73543b5f46d >> >> Are you getting the build error with 20.11-rc1? > > No, with the backport. The original patch was marked for stable, but > the fixup was not. Yes, it should be also marked for stable, seems missed. > I'll pick it up. > Thanks.
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c index a388ff258..d8d3f07f5 100644 --- a/drivers/net/netvsc/hn_rxtx.c +++ b/drivers/net/netvsc/hn_rxtx.c @@ -666,7 +666,8 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb, void *data, uint32_t dlen) { - unsigned int data_off, data_len, pktinfo_off, pktinfo_len; + unsigned int data_off, data_len, total_len; + unsigned int pktinfo_off, pktinfo_len; const struct rndis_packet_msg *pkt = data; struct hn_rxinfo info = { .vlan_info = HN_NDIS_VLAN_INFO_INVALID, @@ -711,7 +712,8 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq, goto error; } - if (unlikely(data_off + data_len > pkt->len)) + if (__builtin_add_overflow(data_off, data_len, &total_len) || + total_len > pkt->len) goto error; if (unlikely(data_len < RTE_ETHER_HDR_LEN))