2023-02-23 06:55:43

by Li Qiong

Subject: [PATCH] net/atm/mpc: Fix dereference NULL pointer in mpc_send_packet()

The 'non_ip' statement need do 'mpc' pointer dereference,
so return '-ENODEV' if 'mpc' is NULL.

Signed-off-by: Li Qiong <[email protected]>
---
net/atm/mpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/atm/mpc.c b/net/atm/mpc.c
index 033871e718a3..1cd6610b8a12 100644
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -577,7 +577,7 @@ static netdev_tx_t mpc_send_packet(struct sk_buff *skb,
mpc = find_mpc_by_lec(dev); /* this should NEVER fail */
if (mpc == NULL) {
pr_info("(%s) no MPC found\n", dev->name);
- goto non_ip;
+ return -ENODEV;
}

eth = (struct ethhdr *)skb->data;
--
2.11.0

2023-02-23 07:59:16

by Dan Carpenter

Subject: Re: [PATCH] net/atm/mpc: Fix dereference NULL pointer in mpc_send_packet()

On Thu, Feb 23, 2023 at 02:54:46PM +0800, Li Qiong wrote:
> The 'non_ip' statement need do 'mpc' pointer dereference,
> so return '-ENODEV' if 'mpc' is NULL.
>
> Signed-off-by: Li Qiong <[email protected]>
> ---
> net/atm/mpc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/atm/mpc.c b/net/atm/mpc.c
> index 033871e718a3..1cd6610b8a12 100644
> --- a/net/atm/mpc.c
> +++ b/net/atm/mpc.c
> @@ -577,7 +577,7 @@ static netdev_tx_t mpc_send_packet(struct sk_buff *skb,
> mpc = find_mpc_by_lec(dev); /* this should NEVER fail */
> if (mpc == NULL) {
> pr_info("(%s) no MPC found\n", dev->name);
> - goto non_ip;
> + return -ENODEV;
> }

The comment says that find_mpc_by_lec() can't fail. This business of
handling impossible situations is very complicated. Should this
free the skb before returning? Eventually static checkers will detect
that.

Generally the rule is that we don't have checks for impossible
conditions. That would trigger a warning for certain static checkers
but it would be a false positive. Otherwise we need to add a whole
bunch of code to silence all warnings about handling an impossible
situation correctly.

regards,
dan carpenter