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

[Tung Quang Nguyen]

Subject: [PATCH 1/1] tipc: fix double-free in tipc_buf_append()
>
>The tipc_msg_validate() function 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 passing the caller's skb pointer-pointer directly to
>tipc_msg_validate(), ensuring any modification is reflected correctly.
>The local skb pointer is then updated from the (possibly modified) caller's
>pointer.
>
>Fixes: d618d09a68e4 ("tipc: enforce valid ratio between skb truesize and
>contents")
>Assisted-by: Gemini:gemini-3.1-pro-preview
>Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
>---
> net/tipc/msg.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/net/tipc/msg.c b/net/tipc/msg.c index 76284fc538eb..9f4f612ee027
>100644
>--- a/net/tipc/msg.c
>+++ b/net/tipc/msg.c
>@@ -177,8 +177,9 @@ 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 (unlikely(!tipc_msg_validate(headbuf)))
> 			goto err;
>+		head = *headbuf;
This is a known issue and was reported via https://patchwork.kernel.org/project/netdevbpf/patch/20260330205313.2433372-1-nicholas@xxxxxxxxxxx/
The author did not respond to my comment.
Can you improve the fix by applying my patch?

diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index 76284fc538eb..01a693559589 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -177,8 +177,19 @@ 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 (unlikely(!tipc_msg_validate(&head))) {
+                       /* reassembled skb has been freed in
+                        * tipc_msg_validate() because of invalid truesize.
+                        * head now points to newly-allocated reassembled skb
+                        * while *headbuf points to freed reassembled skb.
+                        * So, correct *headbuf for freeing newly-allocated
+                        * reassembled skb later.
+                        */
+                       if (head != *headbuf)
+                               *headbuf = head;
+
                        goto err;
+               }
                *buf = head;
                TIPC_SKB_CB(head)->tail = NULL;
                *headbuf = NULL;
> 		*buf = head;
> 		TIPC_SKB_CB(head)->tail = NULL;
> 		*headbuf = NULL;
>--
>2.54.0.rc1.513.gad8abe7a5a-goog
>