| Message ID | 20250321121352.29750-1-qasdev00@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dl2k: fix potential null deref in receive_packet() | expand |
On Fri, Mar 21, 2025 at 1:15 PM Qasim Ijaz <qasdev00@gmail.com> wrote: > > If the pkt_len is less than the copy_thresh the netdev_alloc_skb_ip_align() > is called to allocate an skbuff, on failure it can return NULL. Since > there is no NULL check a NULL deref can occur when setting > skb->protocol. > > Fix this by introducing a NULL check to handle allocation failure. > > Fixes: 89d71a66c40d ("net: Use netdev_alloc_skb_ip_align()") This commit has not changed the behavior in case of memory alloc error. > Signed-off-by: Qasim Ijaz <qasdev00@gmail.com> > --- > drivers/net/ethernet/dlink/dl2k.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c > index d0ea92607870..22e9432adea0 100644 > --- a/drivers/net/ethernet/dlink/dl2k.c > +++ b/drivers/net/ethernet/dlink/dl2k.c > @@ -968,6 +968,11 @@ receive_packet (struct net_device *dev) > np->rx_buf_sz, > DMA_FROM_DEVICE); > } > + > + if (unlikely(!skb)) { > + np->rx_ring[entry].fraginfo = 0; Not sure how this was tested... I think this would leak memory. > + break; > + } > skb->protocol = eth_type_trans (skb, dev); > #if 0 > /* Checksum done by hw, but csum value unavailable. */ > -- > 2.39.5
On Fri, Mar 21, 2025 at 04:13:04PM +0100, Eric Dumazet wrote: > On Fri, Mar 21, 2025 at 1:15 PM Qasim Ijaz <qasdev00@gmail.com> wrote: > > > > If the pkt_len is less than the copy_thresh the netdev_alloc_skb_ip_align() > > is called to allocate an skbuff, on failure it can return NULL. Since > > there is no NULL check a NULL deref can occur when setting > > skb->protocol. > > > > Fix this by introducing a NULL check to handle allocation failure. > > > > Fixes: 89d71a66c40d ("net: Use netdev_alloc_skb_ip_align()") > > This commit has not changed the behavior in case of memory alloc error. > Hi Eric, Thanks for pointing this out, I referenced this commit because it added the netdev_alloc_skb_ip_align() call without ensuring the result of it succeeds or not. Before this change the code used netdev_alloc_skb(), so I now see that there is no functional difference since an allocation occurs in both versions. > > Signed-off-by: Qasim Ijaz <qasdev00@gmail.com> > > --- > > drivers/net/ethernet/dlink/dl2k.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c > > index d0ea92607870..22e9432adea0 100644 > > --- a/drivers/net/ethernet/dlink/dl2k.c > > +++ b/drivers/net/ethernet/dlink/dl2k.c > > @@ -968,6 +968,11 @@ receive_packet (struct net_device *dev) > > np->rx_buf_sz, > > DMA_FROM_DEVICE); > > } > > + > > + if (unlikely(!skb)) { > > + np->rx_ring[entry].fraginfo = 0; > > Not sure how this was tested... > > I think this would leak memory. Could you please elaborate on where you think this may arise so I can investigate further and try to amend it? Thanks Qasim > > > + break; > > + } > > skb->protocol = eth_type_trans (skb, dev); > > #if 0 > > /* Checksum done by hw, but csum value unavailable. */ > > -- > > 2.39.5
diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c index d0ea92607870..22e9432adea0 100644 --- a/drivers/net/ethernet/dlink/dl2k.c +++ b/drivers/net/ethernet/dlink/dl2k.c @@ -968,6 +968,11 @@ receive_packet (struct net_device *dev) np->rx_buf_sz, DMA_FROM_DEVICE); } + + if (unlikely(!skb)) { + np->rx_ring[entry].fraginfo = 0; + break; + } skb->protocol = eth_type_trans (skb, dev); #if 0 /* Checksum done by hw, but csum value unavailable. */
If the pkt_len is less than the copy_thresh the netdev_alloc_skb_ip_align() is called to allocate an skbuff, on failure it can return NULL. Since there is no NULL check a NULL deref can occur when setting skb->protocol. Fix this by introducing a NULL check to handle allocation failure. Fixes: 89d71a66c40d ("net: Use netdev_alloc_skb_ip_align()") Signed-off-by: Qasim Ijaz <qasdev00@gmail.com> --- drivers/net/ethernet/dlink/dl2k.c | 5 +++++ 1 file changed, 5 insertions(+)