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

[Lee Jones]

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@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.24
> >> 33372-1-nicholas@xxxxxxxxxxx/ 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.
> >
> As I explained, your fix adds extra overhead to normal path while the error path is corner case and it rarely happens.
> Whatever approach is applied, we need to add explanation to understand more easily the logic and hidden trick in tipc_msg_validate().

Very well.  I have made the recommended changes.

The patch is currently in my build-test environment.

I will post v2, when everything has been satisfied.

-- 
Lee Jones [李琼斯]