[bug report] net: atlantic: Implement xdp data plane

[Dan Carpenter <dan.carpenter@oracle.com>]

Hello Taehee Yoo,

The patch 26efaef759a1: "net: atlantic: Implement xdp data plane"
from Apr 17, 2022, leads to the following Smatch static checker
warning:

drivers/net/ethernet/aquantia/atlantic/aq_ring.c:496 aq_xdp_run_prog() error: passing non negative 6 to ERR_PTR

drivers/net/ethernet/aquantia/atlantic/aq_ring.c
    415 static struct sk_buff *aq_xdp_run_prog(struct aq_nic_s *aq_nic,
    416                                        struct xdp_buff *xdp,
    417                                        struct aq_ring_s *rx_ring,
    418                                        struct aq_ring_buff_s *buff)
    419 {
    420         int result = NETDEV_TX_BUSY;
    421         struct aq_ring_s *tx_ring;
    422         struct xdp_frame *xdpf;
    423         struct bpf_prog *prog;
    424         u32 act = XDP_ABORTED;
    425         struct sk_buff *skb;
    426 
    427         u64_stats_update_begin(&rx_ring->stats.rx.syncp);
    428         ++rx_ring->stats.rx.packets;
    429         rx_ring->stats.rx.bytes += xdp_get_buff_len(xdp);
    430         u64_stats_update_end(&rx_ring->stats.rx.syncp);
    431 
    432         prog = READ_ONCE(rx_ring->xdp_prog);
    433         if (!prog)
    434                 goto pass;
    435 
    436         prefetchw(xdp->data_hard_start); /* xdp_frame write */
    437 
    438         /* single buffer XDP program, but packet is multi buffer, aborted */
    439         if (xdp_buff_has_frags(xdp) && !prog->aux->xdp_has_frags)
    440                 goto out_aborted;
    441 
    442         act = bpf_prog_run_xdp(prog, xdp);
    443         switch (act) {
    444         case XDP_PASS:
    445 pass:
    446                 xdpf = xdp_convert_buff_to_frame(xdp);
    447                 if (unlikely(!xdpf))
    448                         goto out_aborted;
    449                 skb = xdp_build_skb_from_frame(xdpf, aq_nic->ndev);
    450                 if (!skb)
    451                         goto out_aborted;
    452                 u64_stats_update_begin(&rx_ring->stats.rx.syncp);
    453                 ++rx_ring->stats.rx.xdp_pass;
    454                 u64_stats_update_end(&rx_ring->stats.rx.syncp);
    455                 aq_get_rxpages_xdp(buff, xdp);
    456                 return skb;
    457         case XDP_TX:
    458                 xdpf = xdp_convert_buff_to_frame(xdp);
    459                 if (unlikely(!xdpf))
    460                         goto out_aborted;
    461                 tx_ring = aq_nic->aq_ring_tx[rx_ring->idx];
    462                 result = aq_nic_xmit_xdpf(aq_nic, tx_ring, xdpf);
    463                 if (result == NETDEV_TX_BUSY)
    464                         goto out_aborted;

The aq_nic_xmit_xdpf() function returns zero on success and a mix of
NETDEV_TX_BUSY or negative error codes on failure.  Mixing error codes...

    465                 u64_stats_update_begin(&rx_ring->stats.rx.syncp);
    466                 ++rx_ring->stats.rx.xdp_tx;
    467                 u64_stats_update_end(&rx_ring->stats.rx.syncp);
    468                 aq_get_rxpages_xdp(buff, xdp);
    469                 break;
    470         case XDP_REDIRECT:
    471                 if (xdp_do_redirect(aq_nic->ndev, xdp, prog) < 0)
    472                         goto out_aborted;
    473                 xdp_do_flush();
    474                 u64_stats_update_begin(&rx_ring->stats.rx.syncp);
    475                 ++rx_ring->stats.rx.xdp_redirect;
    476                 u64_stats_update_end(&rx_ring->stats.rx.syncp);
    477                 aq_get_rxpages_xdp(buff, xdp);
    478                 break;
    479         default:
    480                 fallthrough;
    481         case XDP_ABORTED:
    482 out_aborted:
    483                 u64_stats_update_begin(&rx_ring->stats.rx.syncp);
    484                 ++rx_ring->stats.rx.xdp_aborted;
    485                 u64_stats_update_end(&rx_ring->stats.rx.syncp);
    486                 trace_xdp_exception(aq_nic->ndev, prog, act);
    487                 bpf_warn_invalid_xdp_action(aq_nic->ndev, prog, act);
    488                 break;
    489         case XDP_DROP:
    490                 u64_stats_update_begin(&rx_ring->stats.rx.syncp);
    491                 ++rx_ring->stats.rx.xdp_drop;
    492                 u64_stats_update_end(&rx_ring->stats.rx.syncp);
    493                 break;
    494         }
    495 
--> 496         return ERR_PTR(-result);

... is always risky and will lead to a crash in the caller.

    497 }

regards,
dan carpenter