Messages in this thread | | | Subject | Re: [PATCH NET v3 5/7] vrf: use skb_expand_head in vrf_finish_output | From | Vasily Averin <> | Date | Thu, 5 Aug 2021 15:55:15 +0300 |
| |
On 8/5/21 2:55 PM, Julian Wiedmann wrote: > On 02.08.21 11:52, Vasily Averin wrote: >> Unlike skb_realloc_headroom, new helper skb_expand_head >> does not allocate a new skb if possible. >> >> Signed-off-by: Vasily Averin <vvs@virtuozzo.com> >> --- >> drivers/net/vrf.c | 21 +++++++-------------- >> 1 file changed, 7 insertions(+), 14 deletions(-) >> > > [...] > >> /* Be paranoid, rather than too clever. */ >> if (unlikely(skb_headroom(skb) < hh_len && dev->header_ops)) { >> - struct sk_buff *skb2; >> - >> - skb2 = skb_realloc_headroom(skb, LL_RESERVED_SPACE(dev)); >> - if (!skb2) { >> - ret = -ENOMEM; >> - goto err; >> + skb = skb_expand_head(skb, hh_len); >> + if (!skb) { >> + skb->dev->stats.tx_errors++; >> + return -ENOMEM; > > Hello Vasily, > > FYI, Coverity complains that we check skb != NULL here but then > still dereference skb->dev: > > > *** CID 1506214: Null pointer dereferences (FORWARD_NULL) > /drivers/net/vrf.c: 867 in vrf_finish_output() > 861 nf_reset_ct(skb); > 862 > 863 /* Be paranoid, rather than too clever. */ > 864 if (unlikely(skb_headroom(skb) < hh_len && dev->header_ops)) { > 865 skb = skb_expand_head(skb, hh_len); > 866 if (!skb) { >>>> CID 1506214: Null pointer dereferences (FORWARD_NULL) >>>> Dereferencing null pointer "skb". > 867 skb->dev->stats.tx_errors++; > 868 return -ENOMEM;
My fault, I missed it.
Thank you, Vasily Averin
| |