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

[Lee Jones <lee@kernel.org>]

On Mon, 20 Apr 2026, Tung Quang Nguyen wrote:

> 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@kernel.org>
> >---
> > 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@carlini.com/
> The author did not respond to my comment.
> Can you improve the fix by applying my patch?

I'd be happy to make any required changes.

However, is this approach superior to simply passing a reference?

v1 appears to be simpler, easier to read and avoids the explanation.

> 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
> >
> 

-- 
Lee Jones [李琼斯]