can: can327: fix potential skb leak when netdev is down

Message ID 20221110061437.411525-1-william.xuanziyang@huawei.com
State New
Headers
Series can: can327: fix potential skb leak when netdev is down |

Commit Message

Ziyang Xuan Nov. 10, 2022, 6:14 a.m. UTC
  In can327_feed_frame_to_netdev(), it did not free the skb when netdev
is down, and all callers of can327_feed_frame_to_netdev() did not free
allocated skb too. That would trigger skb leak.

Fix it by adding kfree_skb() in can327_feed_frame_to_netdev() when netdev
is down. Not tested, just compiled.

Fixes: 43da2f07622f ("can: can327: CAN/ldisc driver for ELM327 based OBD-II adapters")
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
---
 drivers/net/can/can327.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
  

Comments

Max Staudt Nov. 10, 2022, 4:04 p.m. UTC | #1
(CC Vincent, he may be interested)


On Thu, 10 Nov 2022 14:14:37 +0800
Ziyang Xuan <william.xuanziyang@huawei.com> wrote:

> Fix it by adding kfree_skb() in can327_feed_frame_to_netdev() when netdev
> is down. Not tested, just compiled.

Looks correct to me, so:

Reviewed-by: Max Staudt <max@enpas.org>


Thank you very much for finding and fixing this!

Max
  
Ziyang Xuan Nov. 22, 2022, 2:10 a.m. UTC | #2
Hello,

Gently ask.

Is there any other problem? And can it be applied?

Thanks.

> (CC Vincent, he may be interested)
> 
> 
> On Thu, 10 Nov 2022 14:14:37 +0800
> Ziyang Xuan <william.xuanziyang@huawei.com> wrote:
> 
>> Fix it by adding kfree_skb() in can327_feed_frame_to_netdev() when netdev
>> is down. Not tested, just compiled.
> 
> Looks correct to me, so:
> 
> Reviewed-by: Max Staudt <max@enpas.org>
> 
> 
> Thank you very much for finding and fixing this!
> 
> Max
> 
> .
>
  
Max Staudt Nov. 22, 2022, 10:09 a.m. UTC | #3
Marc, Wolfgang,

Could you please include William's patch to can327, provided that you
see no issue with it?


Thanks :)

Max




On Tue, 22 Nov 2022 10:10:50 +0800
"Ziyang Xuan (William)" <william.xuanziyang@huawei.com> wrote:

> Hello,
> 
> Gently ask.
> 
> Is there any other problem? And can it be applied?
> 
> Thanks.
> 
> > (CC Vincent, he may be interested)
> > 
> > 
> > On Thu, 10 Nov 2022 14:14:37 +0800
> > Ziyang Xuan <william.xuanziyang@huawei.com> wrote:
> >   
> >> Fix it by adding kfree_skb() in can327_feed_frame_to_netdev() when netdev
> >> is down. Not tested, just compiled.  
> > 
> > Looks correct to me, so:
> > 
> > Reviewed-by: Max Staudt <max@enpas.org>
> > 
> > 
> > Thank you very much for finding and fixing this!
> > 
> > Max
> > 
> > .
> >
  

Patch

diff --git a/drivers/net/can/can327.c b/drivers/net/can/can327.c
index 0aa1af31d0fe..17bca63f3dd3 100644
--- a/drivers/net/can/can327.c
+++ b/drivers/net/can/can327.c
@@ -263,8 +263,10 @@  static void can327_feed_frame_to_netdev(struct can327 *elm, struct sk_buff *skb)
 {
 	lockdep_assert_held(&elm->lock);
 
-	if (!netif_running(elm->dev))
+	if (!netif_running(elm->dev)) {
+		kfree_skb(skb);
 		return;
+	}
 
 	/* Queue for NAPI pickup.
 	 * rx-offload will update stats and LEDs for us.