Re: [Qemu-devel] [PATCH] net/vmxnet3: Refine l2 header validation

[Dmitry Fleytman]

ACK.

> On Aug 18, 2015, at 02:45 AM, Shmulik Ladkani <address@hidden> wrote:
> 
> From: Dana Rubin <address@hidden>
> 
> 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 <address@hidden>
> Signed-off-by: Shmulik Ladkani <address@hidden>
> ---
> 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
>