Skip to contentSkip to conversation

ppp: fix memory leak in pad_compress_skb

PATCH Applied Qingfang Deng 4 participants 4 messages Sep 3, 2025 – Sep 4, 2025

Subsystems (3)
Networking Network drivers PPP protocol drivers and compressors
lore
Share & tools
lore

Qingfang Deng

author
#0 lore raw

PATCH 1/1 ppp: fix memory leak in pad_compress_skb

If alloc_skb() fails in pad_compress_skb(), it returns NULL without releasing the old skb. The caller does:

    skb = pad_compress_skb(ppp, skb);
    if (!skb)
        goto drop;

drop:

    kfree_skb(skb);

When pad_compress_skb() returns NULL, the reference to the old skb is lost and kfree_skb(skb) ends up doing nothing, leading to a memory leak.

Align pad_compress_skb() semantics with realloc(): only free the old skb if allocation and compression succeed. At the call site, use the new_skb variable so the original skb is not lost when pad_compress_skb() fails.

Fixes b3f9b92a6ec1 ("[PPP]: add PPP MPPE encryption module")
Signed-off-by Qingfang Deng <[email protected]>
---
 drivers/net/ppp/ppp_generic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/ppp/ppp_generic.c +3 -3
index 65795d099166..f9f0f16c41d1 100644
@@ -1744,7 +1744,6 @@ pad_compress_skb(struct ppp *ppp, struct sk_buff *skb)
 		 */
 		if (net_ratelimit())
 			netdev_err(ppp->dev, "ppp: compressor dropped pkt\n");
-		kfree_skb(skb);
 		consume_skb(new_skb);
 		new_skb = NULL;
 	}

@@ -1845,9 +1844,10 @@ ppp_send_frame(struct ppp *ppp, struct sk_buff *skb)
 					   "down - pkt dropped.\n");
 			goto drop;
 		}
-		skb = pad_compress_skb(ppp, skb);
-		if (!skb)
+		new_skb = pad_compress_skb(ppp, skb);
+		if (!new_skb)
 			goto drop;
+		skb = new_skb;
 	}
 
 	/*
Signature · 1 line
2.43.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 14 lines · Qingfang Deng

If alloc_skb() fails in pad_compress_skb(), it returns NULL without releasing the old skb. The caller does:

    skb = pad_compress_skb(ppp, skb);
    if (!skb)
        goto drop;

drop:

    kfree_skb(skb);

When pad_compress_skb() returns NULL, the reference to the old skb is lost and kfree_skb(skb) ends up doing nothing, leading to a memory leak.

Quoted 8 lines

Align pad_compress_skb() semantics with realloc(): only free the old skb if allocation and compression succeed. At the call site, use the new_skb variable so the original skb is not lost when pad_compress_skb() fails.

Fixes b3f9b92a6ec1 ("[PPP]: add PPP MPPE encryption module")
Signed-off-by Qingfang Deng <[email protected]>

---

Reviewed-by Eric Dumazet <[email protected]>
Reviewed-by on v1 1/1
#2 lore raw
In reply to Qingfang Deng
Quoted 39 lines · Qingfang Deng

If alloc_skb() fails in pad_compress_skb(), it returns NULL without releasing the old skb. The caller does:

    skb = pad_compress_skb(ppp, skb);
    if (!skb)
        goto drop;

drop:

    kfree_skb(skb);

When pad_compress_skb() returns NULL, the reference to the old skb is lost and kfree_skb(skb) ends up doing nothing, leading to a memory leak.

Align pad_compress_skb() semantics with realloc(): only free the old skb if allocation and compression succeed. At the call site, use the new_skb variable so the original skb is not lost when pad_compress_skb() fails.

Fixes b3f9b92a6ec1 ("[PPP]: add PPP MPPE encryption module")
Signed-off-by Qingfang Deng <[email protected]>
---
 drivers/net/ppp/ppp_generic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/ppp/ppp_generic.c +0 -1
index 65795d099166..f9f0f16c41d1 100644
@@ -1744,7 +1744,6 @@ pad_compress_skb(struct ppp *ppp, struct sk_buff *skb)
 		 */
 		if (net_ratelimit())
 			netdev_err(ppp->dev, "ppp: compressor dropped pkt\n");
-		kfree_skb(skb);
 		consume_skb(new_skb);
 		new_skb = NULL;
 	}

@@ -1845,9 +1844,10 @@ ppp_send_frame(struct ppp *ppp, struct sk_buff *skb)
 					   "down - pkt dropped.\n");
 			goto drop;

drivers/net/ppp/ppp_generic.c ppp_send_frame(struct ppp *ppp, struct sk_buff *skb) · line 1853

Quoted 10 lines
drivers/net/ppp/ppp_generic.c +3 -2
 		}
-		skb = pad_compress_skb(ppp, skb);
-		if (!skb)
+		new_skb = pad_compress_skb(ppp, skb);
+		if (!new_skb)
 			goto drop;
+		skb = new_skb;
 	}
 
 	/*
Reviewed-by Yue Haibing <[email protected]>
CI Applied
#3 lore raw
In reply to Qingfang Deng
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 8 lines · On Wed, 3 Sep 2025 18:07:26 +0800 you wrote:

If alloc_skb() fails in pad_compress_skb(), it returns NULL without releasing the old skb. The caller does:

    skb = pad_compress_skb(ppp, skb);
    if (!skb)
        goto drop;

[...]

Here is the summary with links:
- [net] ppp: fix memory leak in pad_compress_skb

    https://git.kernel.org/netdev/net/c/4844123fe0b8

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html