Re: [Intel-wired-lan] [bug report] ixgbe: Fix passing 0 to ERR_PTR in ixgbe_run_xdp()

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 1/10/25 07:34, Dan Carpenter wrote:
> Hello Yue Haibing,
> 

Dan, Thank you for the report.

@Piotr, could you please fix it?

> Commit c824125cbb18 ("ixgbe: Fix passing 0 to ERR_PTR in
> ixgbe_run_xdp()") from Jan 6, 2025 (linux-next), leads to the
> following Smatch static checker warning:
> 
> 	drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:2108 ixgbe_put_rx_buffer()
> 	warn: possible NULL dereference of 'skb'
> 
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>      2099 static void ixgbe_put_rx_buffer(struct ixgbe_ring *rx_ring,
>      2100                                 struct ixgbe_rx_buffer *rx_buffer,
>      2101                                 struct sk_buff *skb,
>      2102                                 int rx_buffer_pgcnt)
>      2103 {
>      2104         if (ixgbe_can_reuse_rx_page(rx_buffer, rx_buffer_pgcnt)) {
>      2105                 /* hand second half of page back to the ring */
>      2106                 ixgbe_reuse_rx_page(rx_ring, rx_buffer);
>      2107         } else {
> --> 2108                 if (!IS_ERR(skb) && IXGBE_CB(skb)->dma == rx_buffer->dma) {
>                                       ^^^
> This can't be an error pointer and probably it should be a NULL check.
> I'm not sure if your patch introduced this issue or just exposed it.
> 
>      2109                         /* the page has been released from the ring */
>      2110                         IXGBE_CB(skb)->page_released = true;
>      2111                 } else {
>      2112                         /* we are not reusing the buffer so unmap it */
>      2113                         dma_unmap_page_attrs(rx_ring->dev, rx_buffer->dma,
>      2114                                              ixgbe_rx_pg_size(rx_ring),
>      2115                                              DMA_FROM_DEVICE,
>      2116                                              IXGBE_RX_DMA_ATTR);
>      2117                 }
>      2118                 __page_frag_cache_drain(rx_buffer->page,
>      2119                                         rx_buffer->pagecnt_bias);
>      2120         }
>      2121
>      2122         /* clear contents of rx_buffer */
>      2123         rx_buffer->page = NULL;
>      2124         rx_buffer->skb = NULL;
>      2125 }
> 
> regards,
> dan carpenter