All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee@kernel.org>
To: Tung Quang Nguyen <tung.quang.nguyen@est.tech>
Cc: Jon Maloy <jmaloy@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"tipc-discussion@lists.sourceforge.net"
	<tipc-discussion@lists.sourceforge.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1] tipc: fix double-free in tipc_buf_append()
Date: Mon, 20 Apr 2026 16:10:40 +0100	[thread overview]
Message-ID: <20260420151040.GF3202366@google.com> (raw)
In-Reply-To: <GV1P189MB198853F316580DDA547F4925C62F2@GV1P189MB1988.EURP189.PROD.OUTLOOK.COM>

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

  reply	other threads:[~2026-04-20 15:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-20 13:05 [PATCH 1/1] tipc: fix double-free in tipc_buf_append() Lee Jones
2026-04-20 13:46 ` Tung Quang Nguyen
2026-04-20 14:33   ` Lee Jones
2026-04-20 14:49     ` Tung Quang Nguyen
2026-04-20 15:10       ` Lee Jones [this message]
2026-04-21 10:35         ` Lee Jones
2026-04-21 12:10           ` Tung Quang Nguyen
2026-04-21 12:28             ` Lee Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260420151040.GF3202366@google.com \
    --to=lee@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jmaloy@redhat.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=tipc-discussion@lists.sourceforge.net \
    --cc=tung.quang.nguyen@est.tech \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.