[PATCH v2 1/1] tipc: fix double-free in tipc_buf_append()

[PATCH v2 1/1] tipc: fix double-free in tipc_buf_append()

[Lee Jones]

tipc_msg_validate() can potentially reallocate the skb it is validating,
freeing the old one.  In tipc_buf_append(), it was being called with a
pointer to a local variable which was a copy of the caller's skb
pointer.

If the skb was reallocated and validation subsequently failed, the error
handling path would free the original skb pointer, which had already
been freed, leading to double-free.

Fix this by checking if head now points to a newly allocated reassembled
skb.  If it does, reassign *headbuf for later freeing operations.

Fixes: d618d09a68e4 ("tipc: enforce valid ratio between skb truesize and contents")
Suggested-by: Tung Nguyen <tung.quang.nguyen@est.tech>
Signed-off-by: Lee Jones <lee@kernel.org>
---
1v => v2: Keep the passed pointer type the same, but reassign on-change

 net/tipc/msg.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index 76284fc538eb..b0bba0feef56 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -177,8 +177,20 @@ int tipc_buf_append(struct sk_buff **headbuf, struct sk_buff **buf)
 
 	if (fragid == LAST_FRAGMENT) {
 		TIPC_SKB_CB(head)->validated = 0;
-		if (unlikely(!tipc_msg_validate(&head)))
+
+		/* If the reassembled skb has been freed in
+		 * tipc_msg_validate() because of an invalid truesize,
+		 * then head will point to a newly allocated reassembled
+		 * skb, while *headbuf points to freed reassembled skb.
+		 * In such cases, correct *headbuf for freeing the newly
+		 * allocated reassembled skb later.
+		 */
+		if (unlikely(!tipc_msg_validate(&head))) {
+			if (head != *headbuf)
+				*headbuf = head;
 			goto err;
+		}
+
 		*buf = head;
 		TIPC_SKB_CB(head)->tail = NULL;
 		*headbuf = NULL;
-- 
2.54.0.rc1.555.g9c883467ad-goog

RE: [PATCH v2 1/1] tipc: fix double-free in tipc_buf_append()

[Tung Quang Nguyen]

>Subject: [PATCH v2 1/1] tipc: fix double-free in tipc_buf_append()
>
>tipc_msg_validate() can potentially reallocate the skb it is validating, freeing
>the old one.  In tipc_buf_append(), it was being called with a pointer to a local
>variable which was a copy of the caller's skb pointer.
>
>If the skb was reallocated and validation subsequently failed, the error
>handling path would free the original skb pointer, which had already been
>freed, leading to double-free.
>
>Fix this by checking if head now points to a newly allocated reassembled skb.
>If it does, reassign *headbuf for later freeing operations.
>
>Fixes: d618d09a68e4 ("tipc: enforce valid ratio between skb truesize and
>contents")
>Suggested-by: Tung Nguyen <tung.quang.nguyen@est.tech>
>Signed-off-by: Lee Jones <lee@kernel.org>
>---
>1v => v2: Keep the passed pointer type the same, but reassign on-change
>
> net/tipc/msg.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
>diff --git a/net/tipc/msg.c b/net/tipc/msg.c index 76284fc538eb..b0bba0feef56
>100644
>--- a/net/tipc/msg.c
>+++ b/net/tipc/msg.c
>@@ -177,8 +177,20 @@ int tipc_buf_append(struct sk_buff **headbuf, struct
>sk_buff **buf)
>
> 	if (fragid == LAST_FRAGMENT) {
> 		TIPC_SKB_CB(head)->validated = 0;
>-		if (unlikely(!tipc_msg_validate(&head)))
>+
>+		/* If the reassembled skb has been freed in
>+		 * tipc_msg_validate() because of an invalid truesize,
>+		 * then head will point to a newly allocated reassembled
>+		 * skb, while *headbuf points to freed reassembled skb.
>+		 * In such cases, correct *headbuf for freeing the newly
>+		 * allocated reassembled skb later.
>+		 */
>+		if (unlikely(!tipc_msg_validate(&head))) {
>+			if (head != *headbuf)
>+				*headbuf = head;
> 			goto err;
>+		}
>+
> 		*buf = head;
> 		TIPC_SKB_CB(head)->tail = NULL;
> 		*headbuf = NULL;
>--
>2.54.0.rc1.555.g9c883467ad-goog
Reviewed-by: Tung Nguyen <tung.quang.nguyen@est.tech>

Re: [PATCH v2 1/1] tipc: fix double-free in tipc_buf_append()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 21 Apr 2026 13:45:26 +0100 you wrote:
> tipc_msg_validate() can potentially reallocate the skb it is validating,
> freeing the old one.  In tipc_buf_append(), it was being called with a
> pointer to a local variable which was a copy of the caller's skb
> pointer.
> 
> If the skb was reallocated and validation subsequently failed, the error
> handling path would free the original skb pointer, which had already
> been freed, leading to double-free.
> 
> [...]

Here is the summary with links:
  - [v2,1/1] tipc: fix double-free in tipc_buf_append()
    https://git.kernel.org/netdev/net/c/d293ca716e7d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v2 1/1] tipc: fix double-free in tipc_buf_append()

[Simon Horman]

On Tue, Apr 21, 2026 at 01:45:26PM +0100, Lee Jones wrote:
> tipc_msg_validate() can potentially reallocate the skb it is validating,
> freeing the old one.  In tipc_buf_append(), it was being called with a
> pointer to a local variable which was a copy of the caller's skb
> pointer.
> 
> If the skb was reallocated and validation subsequently failed, the error
> handling path would free the original skb pointer, which had already
> been freed, leading to double-free.
> 
> Fix this by checking if head now points to a newly allocated reassembled
> skb.  If it does, reassign *headbuf for later freeing operations.
> 
> Fixes: d618d09a68e4 ("tipc: enforce valid ratio between skb truesize and contents")
> Suggested-by: Tung Nguyen <tung.quang.nguyen@est.tech>
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
> 1v => v2: Keep the passed pointer type the same, but reassign on-change

FTR: Sashiko has generated a review of this patch which I have examined.
I do not believe that review should halt progress of this patch
as it appears that the problem flagged pre-dates this patch. Actually,
its unclear to me if it is a problem that warrants addressing at all.
But I'd appreciate if it could be looked over as a follow-up task.