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

Lee Jones
2026-04-20T14:33:15Z

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 [李琼斯]