Re: [Xen-devel] xennet_start_xmit assumptions

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Sowmini Varadhan [mailto:sowmini.varadhan@xxxxxxxxxx]
> Sent: 19 January 2017 11:14
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] xennet_start_xmit assumptions
> 
> On (01/19/17 09:36), Paul Durrant wrote:
> >
> > Hi Sowmini,
> >
> >   Sounds like a straightforward bug to me... netfront should be able
> > to handle an empty skb and clearly, if it's relying on skb_headlen()
> > being non-zero, that's not the case.
> >
> >   Paul
> 
> I see. Seems like there are 2 things broken here: recovering
> from skb->len = 0, and recovering from  the more complex
> case of (skb->len > 0 && skb_headlen(skb) == 0)
> 
> Do you folks want to take a shot at fixing this,
> since you know the code better? If you are interested,
> I can share my test program to help you reproduce the
> simpler skb->len == 0 case, but it's the fully non-linear
> skbs that may be more interesting to reproduce/fix.
> 
> I'll probably work on fixing packet_snd to return -EINVAL
> or similar when the len is zero this week.
> 

Sowmini,

  I knocked together the following patch, which seems to work for me:

---8<---
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 40f26b6..a957c89 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -567,6 +567,10 @@ static int xennet_start_xmit(struct sk_buff *skb, struct 
net_device *dev)
        u16 queue_index;
        struct sk_buff *nskb;

+       /* Drop packets that are not at least ETH_HLEN in length */
+       if (skb->len < ETH_HLEN)
+               goto drop;
+
        /* Drop the packet if no queues are set up */
        if (num_queues < 1)
                goto drop;
@@ -609,6 +613,8 @@ static int xennet_start_xmit(struct sk_buff *skb, struct 
net_device *dev)
        }

        len = skb_headlen(skb);
+       if ((len < ETH_HLEN) && !__pskb_pull_tail(skb, ETH_HLEN))
+               goto drop;

        spin_lock_irqsave(&queue->tx_lock, flags);

---8<---

  Making netfront cope with a fully non-linear skb looks like it would be quite 
intrusive and probably not worth it so I opted for just doing the ETH_HLEN 
pull-tail if necessary. Can you check it works for you?

  Paul


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel