Re: Re: Re: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty

[王明煜]

Hi Florian and Sabrina,

I checked the stable branches for a potential backport. It turns out no backport is needed because the stable trees are not affected by this specific leak path.

Looking closely at `ip6_setup_cork()` in the stable trees, `cork->base.dst = &rt->dst;` is assigned at the very beginning of the function. 

If a subsequent memory allocation fails (e.g., `kzalloc` failing due to failslab), it returns `-ENOBUFS`, but the `dst` reference is already linked to the cork. When `ip6_make_skb()` catches this error, it calls `ip6_cork_release(cork, &v6_cork)`, which successfully frees the `dst`.

As Sabrina correctly pointed out regarding my v1, adding an explicit `dst_release()` here would actually cause a double free.

The original syzbot memory leak report under `failslab` seems to be a false positive or related to a different object entirely. The `dst_entry` handling in this error path is already robust. 

This issue can be closed without any patches. Thanks for the discussion.

Best regards,
Mingyu Wang


> -----原始邮件-----
> 发件人: "Bezdeka, Florian" <florian.bezdeka@xxxxxxxxxxx>
> 发送时间:2026-04-24 14:56:35 (星期五)
> 收件人: "willemdebruijn.kernel@xxxxxxxxx" <willemdebruijn.kernel@xxxxxxxxx>, "25181214217@xxxxxxxxxxxxxxxxx" <25181214217@xxxxxxxxxxxxxxxxx>
> 抄送: "syzbot+e5d6936b9f4545fd88ab@xxxxxxxxxxxxxxxxxxxxxxxxx" <syzbot+e5d6936b9f4545fd88ab@xxxxxxxxxxxxxxxxxxxxxxxxx>, "davem@xxxxxxxxxxxxx" <davem@xxxxxxxxxxxxx>, "dsahern@xxxxxxxxxx" <dsahern@xxxxxxxxxx>, "sd@xxxxxxxxxxxxxxx" <sd@xxxxxxxxxxxxxxx>, "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>, "netdev@xxxxxxxxxxxxxxx" <netdev@xxxxxxxxxxxxxxx>, "horms@xxxxxxxxxx" <horms@xxxxxxxxxx>, "edumazet@xxxxxxxxxx" <edumazet@xxxxxxxxxx>, "kuba@xxxxxxxxxx" <kuba@xxxxxxxxxx>, "pabeni@xxxxxxxxxx" <pabeni@xxxxxxxxxx>
> 主题: Re: Re: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
> 
> On Fri, 2026-04-24 at 11:26 +0800, 王明煜 wrote:
> > Hi Sabrina and Jakub,
> > 
> > Before sending out the v3 patch, I synced my tree to the latest mainline and checked the current state of `ip6_make_skb()`. 
> > 
> > It turns out that the missing `ip6_cork_release(cork)` in the error path was already naturally resolved by Eric Dumazet's recent refactoring commit:
> > b409a7f7176b ("ipv6: colocate inet6_cork in inet_cork_full")
> > 
> > With Eric's changes, the error handling now correctly calls `ip6_cork_release(cork)` if `ip6_setup_cork()` fails, meaning the memory leak is no longer present in the latest tree.
> 
> Fine, so mainline is correct now. What about the stable trees? Did you
> check them already?
> 
> Florian
> 
> > 
> > Please disregard my v1 and v2 patches. I am also telling syzbot to close this report based on Eric's commit.
> > 
> > Thank you all again for your time, the deep code review, and for guiding me to find the true root cause. I learned a huge amount from this discussion!
> > 
> > #syz fix: ipv6: colocate inet6_cork in inet_cork_full
> > 
> > Best regards,
> > Mingyu Wang
> > 
> > 2026-04-24 11:16:30 "王明煜" <25181214217@xxxxxxxxxxxxxxxxx> 写道：
> > > Hi,
> > > 
> > > Thank you so much for the review and for pointing me to the correct Fixes tag!
> > > 
> > > You hit the nail on the head regarding `__ip6_append_data()`. After re-evaluating the code path based on your question, I realize my assumption in v2 was incorrect. `__ip6_append_data()` does indeed guarantee that an skb is queued upon success, making the `skb == NULL` path dead code in this context.
> > > 
> > > I traced the `failslab` memory leak back to its true origin: the lockless fast path wrapper `ip6_make_skb()`.
> > > 
> > > Sabrina previously noted that `ip6_setup_cork()` failures correctly release the dst. That is absolutely true for the slow path, where `udp_v6_flush_pending_frames()` eventually handles the cleanup. 
> > > 
> > > However, in the fast path, `ip6_make_skb()` calls `ip6_setup_cork()`. Inside `ip6_setup_cork()`, `cork->base.dst` is assigned early. If a subsequent memory allocation fails (e.g., `v6_cork->opt = kzalloc(...)` failing due to failslab), it returns an error. `ip6_make_skb()` then directly returns `ERR_PTR(err)` WITHOUT calling `ip6_cork_release(cork)`.
> > > 
> > > Since `udpv6_sendmsg()` assumes the `dst` reference is stolen by `ip6_make_skb()` and unconditionally jumps to `out_no_dst`, the `dst` is completely leaked.
> > > 
> > > The fix is simply to add `ip6_cork_release(cork)` in the `ip6_setup_cork()` error path inside `ip6_make_skb()`.
> > > 
> > > I will submit a v3 patch shortly addressing this true root cause and using your suggested Fixes tag. Thank you again for steering me in the exact right direction!
> > > 
> > > Best regards,
> > > Mingyu Wang
> > > 
> > > 
> > > > -----原始邮件-----
> > > > 发件人: "Willem de Bruijn" <willemdebruijn.kernel@xxxxxxxxx>
> > > > 发送时间:2026-04-23 22:59:45 (星期四)
> > > > 收件人: "Mingyu Wang" <25181214217@xxxxxxxxxxxxxxxxx>, willemdebruijn.kernel@xxxxxxxxx, davem@xxxxxxxxxxxxx, dsahern@xxxxxxxxxx, edumazet@xxxxxxxxxx, kuba@xxxxxxxxxx, pabeni@xxxxxxxxxx
> > > > 抄送: sd@xxxxxxxxxxxxxxx, horms@xxxxxxxxxx, netdev@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, "Mingyu Wang" <25181214217@xxxxxxxxxxxxxxxxx>, syzbot+e5d6936b9f4545fd88ab@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > > 主题: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
> > > > 
> > > > Mingyu Wang wrote:
> > > > > During fuzzing with failslab enabled, a memory leak was observed in the
> > > > > IPv6 UDP send path.
> > > > > 
> > > > > The root cause resides in __ip6_make_skb(). In extremely rare cases
> > > > > (such as fault injection or specific empty payload conditions),
> > > > 
> > > > Can you elaborate on this? Which fault injection lets
> > > > __ip6_append_data succeed without writing data?
> > > > 
> > > > > __ip6_append_data() may succeed but leave the socket's write queue
> > > > > empty.
> > > > > 
> > > > > When __ip6_make_skb() is subsequently called, __skb_dequeue(queue)
> > > > > returns NULL. The previous logic handled this by executing a 'goto out;',
> > > > > which completely bypassed the call to ip6_cork_release(cork).
> > > > > 
> > > > > Since the 'cork' structure actively holds a reference to the routing
> > > > > entry (dst_entry) and potentially other allocated options, skipping
> > > > > the release cleanly leaks these resources.
> > > > > 
> > > > > Fix this by introducing an 'out_cork_release' label and jumping to it
> > > > > when skb is NULL, ensuring the cork state is always properly cleaned up.
> > > > > The now-unused 'out' label is also removed to prevent compiler warnings.
> > > > > 
> > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > > 
> > > > I think this is 
> > > > 
> > > > Fixes: 6422398c2ab0 ("ipv6: introduce ipv6_make_skb")
> > > > 
> > > > > Reported-by: syzbot+e5d6936b9f4545fd88ab@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > > > Signed-off-by: Mingyu Wang <25181214217@xxxxxxxxxxxxxxxxx>