Re: [PATCH net-next v2] net: mana: remove unreachable dead code in mana_bpf()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Failure to restore `apc->bpf_prog` on error path causes Use-After-Free.
--

commit 26086af444af1d715f516a007146a7dd00ce97c6
Author: Muhammad Falak R Wani <falakreyaz@gmail.com>

net: mana: remove unreachable dead code in mana_bpf()

This commit cleans up mana_bpf() by removing an unreachable return statement
after a switch statement that already unconditionally returns.

Link: https://lore.kernel.org/netdev/DKWZHHOFA1F3.1EL2DMFNGMP0Y@gmail.com/

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> index 53308e139cbe9..70bb8c9dc3413 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
[ ... ]

This is a pre-existing issue, but while reviewing this patch I noticed a
potential problem in the adjacent mana_xdp_set() function when it handles
allocation failures:

drivers/net/ethernet/microsoft/mana/mana_bpf.c:mana_xdp_set() {
        ...
        apc->bpf_prog = prog;

        if (apc->port_is_up) {
                ...
                err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
                if (err) {
                        NL_SET_ERR_MSG_MOD(extack,
                                           "XDP: Insufficient memory for tx/rx re-config");
                        return err;
                }
        ...
}

[Severity: High]
Does this error path leave a dangling pointer in apc->bpf_prog?

It appears that if mana_pre_alloc_rxbufs() fails, the function returns
immediately without restoring apc->bpf_prog = old_prog. The generic XDP caller
will see the failure, assume the driver rejected the program, and drop its BPF
program reference.

Could this leave a freed pointer in apc->bpf_prog that might be dereferenced
when the interface is later restarted or accessed?

> @@ -253,7 +253,6 @@ static int mana_xdp_set(struct net_device *ndev, struct bpf_prog *prog,
>  int mana_bpf(struct net_device *ndev, struct netdev_bpf *bpf)
>  {
>  	struct netlink_ext_ack *extack = bpf->extack;
> -	int ret;
>  
>  	switch (bpf->command) {
>  	case XDP_SETUP_PROG:
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/3d8b67307df6fefdc18743274cef3de4fca0e7de.1788339612.git.falakreyaz@gmail.com?part=1