Message ID | 1439891155-16011-1-git-send-email-shmulik.ladkani@ravellosystems.com |
---|---|
State | New |
Headers | show |
ACK. > On Aug 18, 2015, at 02:45 AM, Shmulik Ladkani <shmulik.ladkani@ravellosystems.com> wrote: > > From: Dana Rubin <dana.rubin@ravellosystems.com> > > Validation of l2 header length assumed minimal packet size as > eth_header + 2 * vlan_header regardless of the actual protocol. > > This caused crash for valid non-IP packets shorter than 22 bytes, as > 'tx_pkt->packet_type' hasn't been assigned for such packets, and > 'vmxnet3_on_tx_done_update_stats()' expects it to be properly set. > > Refine header length validation in 'vmxnet_tx_pkt_parse_headers'. > Check its return value during packet processing flow. > > As a side effect, in case IPv4 and IPv6 header validation failure, > corrupt packets will be dropped. > > Signed-off-by: Dana Rubin <dana.rubin@ravellosystems.com> > Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com> > --- > hw/net/vmxnet3.c | 4 +--- > hw/net/vmxnet_tx_pkt.c | 19 ++++++++++++++++--- > 2 files changed, 17 insertions(+), 6 deletions(-) > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index 59b06b8..f37297f 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -729,9 +729,7 @@ static void vmxnet3_process_tx_queue(VMXNET3State *s, int qidx) > } > > if (txd.eop) { > - if (!s->skip_current_tx_pkt) { > - vmxnet_tx_pkt_parse(s->tx_pkt); > - > + if (!s->skip_current_tx_pkt && vmxnet_tx_pkt_parse(s->tx_pkt)) { > if (s->needs_vlan) { > vmxnet_tx_pkt_setup_vlan_header(s->tx_pkt, s->tci); > } > diff --git a/hw/net/vmxnet_tx_pkt.c b/hw/net/vmxnet_tx_pkt.c > index f7344c4..eb88ddf 100644 > --- a/hw/net/vmxnet_tx_pkt.c > +++ b/hw/net/vmxnet_tx_pkt.c > @@ -142,11 +142,24 @@ static bool vmxnet_tx_pkt_parse_headers(struct VmxnetTxPkt *pkt) > > bytes_read = iov_to_buf(pkt->raw, pkt->raw_frags, 0, l2_hdr->iov_base, > ETH_MAX_L2_HDR_LEN); > - if (bytes_read < ETH_MAX_L2_HDR_LEN) { > + if (bytes_read < sizeof(struct eth_header)) { > + l2_hdr->iov_len = 0; > + return false; > + } > + > + l2_hdr->iov_len = sizeof(struct eth_header); > + switch (be16_to_cpu(PKT_GET_ETH_HDR(l2_hdr->iov_base)->h_proto)) { > + case ETH_P_VLAN: > + l2_hdr->iov_len += sizeof(struct vlan_header); > + break; > + case ETH_P_DVLAN: > + l2_hdr->iov_len += 2 * sizeof(struct vlan_header); > + break; > + } > + > + if (bytes_read < l2_hdr->iov_len) { > l2_hdr->iov_len = 0; > return false; > - } else { > - l2_hdr->iov_len = eth_get_l2_hdr_length(l2_hdr->iov_base); > } > > l3_proto = eth_get_l3_proto(l2_hdr->iov_base, l2_hdr->iov_len); > -- > 1.9.1 >
On Tue, Aug 18, 2015 at 12:45:55PM +0300, Shmulik Ladkani wrote: > From: Dana Rubin <dana.rubin@ravellosystems.com> > > Validation of l2 header length assumed minimal packet size as > eth_header + 2 * vlan_header regardless of the actual protocol. > > This caused crash for valid non-IP packets shorter than 22 bytes, as > 'tx_pkt->packet_type' hasn't been assigned for such packets, and > 'vmxnet3_on_tx_done_update_stats()' expects it to be properly set. > > Refine header length validation in 'vmxnet_tx_pkt_parse_headers'. > Check its return value during packet processing flow. > > As a side effect, in case IPv4 and IPv6 header validation failure, > corrupt packets will be dropped. > > Signed-off-by: Dana Rubin <dana.rubin@ravellosystems.com> > Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com> > --- > hw/net/vmxnet3.c | 4 +--- > hw/net/vmxnet_tx_pkt.c | 19 ++++++++++++++++--- > 2 files changed, 17 insertions(+), 6 deletions(-) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan
Hi, On Thu, 3 Sep 2015 17:45:34 +0100 Stefan Hajnoczi <stefanha@gmail.com> wrote: > Thanks, applied to my net tree: > https://github.com/stefanha/qemu/commits/net For some reason, the patch isn't present on Stefan's last pull requests. Can you please verify this gets merged? Thanks, Shmulik
On 09/18/2015 02:13 PM, Shmulik Ladkani wrote: > Hi, > > On Thu, 3 Sep 2015 17:45:34 +0100 Stefan Hajnoczi <stefanha@gmail.com> wrote: >> Thanks, applied to my net tree: >> https://github.com/stefanha/qemu/commits/net > For some reason, the patch isn't present on Stefan's last pull requests. > > Can you please verify this gets merged? > > Thanks, > Shmulik Applied in https://github.com/jasowang/qemu/commits/net Will be in next pull request. Thanks.
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 59b06b8..f37297f 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -729,9 +729,7 @@ static void vmxnet3_process_tx_queue(VMXNET3State *s, int qidx) } if (txd.eop) { - if (!s->skip_current_tx_pkt) { - vmxnet_tx_pkt_parse(s->tx_pkt); - + if (!s->skip_current_tx_pkt && vmxnet_tx_pkt_parse(s->tx_pkt)) { if (s->needs_vlan) { vmxnet_tx_pkt_setup_vlan_header(s->tx_pkt, s->tci); } diff --git a/hw/net/vmxnet_tx_pkt.c b/hw/net/vmxnet_tx_pkt.c index f7344c4..eb88ddf 100644 --- a/hw/net/vmxnet_tx_pkt.c +++ b/hw/net/vmxnet_tx_pkt.c @@ -142,11 +142,24 @@ static bool vmxnet_tx_pkt_parse_headers(struct VmxnetTxPkt *pkt) bytes_read = iov_to_buf(pkt->raw, pkt->raw_frags, 0, l2_hdr->iov_base, ETH_MAX_L2_HDR_LEN); - if (bytes_read < ETH_MAX_L2_HDR_LEN) { + if (bytes_read < sizeof(struct eth_header)) { + l2_hdr->iov_len = 0; + return false; + } + + l2_hdr->iov_len = sizeof(struct eth_header); + switch (be16_to_cpu(PKT_GET_ETH_HDR(l2_hdr->iov_base)->h_proto)) { + case ETH_P_VLAN: + l2_hdr->iov_len += sizeof(struct vlan_header); + break; + case ETH_P_DVLAN: + l2_hdr->iov_len += 2 * sizeof(struct vlan_header); + break; + } + + if (bytes_read < l2_hdr->iov_len) { l2_hdr->iov_len = 0; return false; - } else { - l2_hdr->iov_len = eth_get_l2_hdr_length(l2_hdr->iov_base); } l3_proto = eth_get_l3_proto(l2_hdr->iov_base, l2_hdr->iov_len);