On Tue, 2017-04-18 at 22:09 +0000, [email protected] wrote:
> > > @@ -2067,13 +2067,9 @@ static struct sk_buff *smsc95xx_tx_fixup(struct
> > usbnet *dev,
> > > /* We do not advertise SG, so skbs should be already linearized */
> > > BUG_ON(skb_shinfo(skb)->nr_frags);
> > >
> > > - if (skb_headroom(skb) < overhead) {
> > > - struct sk_buff *skb2 = skb_copy_expand(skb,
> > > - overhead, 0, flags);
> > > - dev_kfree_skb_any(skb);
> > > - skb = skb2;
> > > - if (!skb)
> > > - return NULL;
> > > + /* Make writable and expand header space by overhead if required
> > */
> > > + if (skb_cow_head(skb, overhead)) {
> >
> > I believe you still need to
> > dev_kfree_skb_any(skb);
> >
> I think caller of usbnet_start_xmit() takes care of free when return NULL.
I do not think so. Here is the code in usbnet_start_xmit() :
if (info->tx_fixup) {
skb = info->tx_fixup (dev, skb, GFP_ATOMIC);
if (!skb) { // Note that skb is NULL now
if (info->flags & FLAG_MULTI_PACKET)
goto not_drop;
netif_dbg(dev, tx_err, dev->net, "can't tx_fixup skb\n");
goto drop;
If you really think about it (even before double checking the code in
usbnet_start_xmit()), that would have been a very serious bug.
Calling dev_kfree_skb_any(skb) in this fixup code, then later from
usbnet_start_xmit() would have been a double free, or use after free.
