Re: [PATCH net 1/1] ip6_tunnel: use skb_cow_head() in ip6_tnl_xmit()
Ido Schimmel <[email protected]>
| Newsgroups | org.kernel.vger.netdev |
|---|---|
| Message-ID | <20260805090721.GA1364862@shredder> |
On Mon, Aug 03, 2026 at 01:34:46PM +0800, Zhiling Zou wrote:
> ip6_tnl_xmit() may need to expand headroom before it can push the
> outer IPv6 and optional encap headers. It currently does that with
> skb_realloc_headroom(), copies skb->sk ownership, consumes the original
> skb, and then continues processing with the replacement skb kept only in
> its local variable.
>
> That is safe only if the helper cannot fail afterwards. But this helper
> still has post-reallocation error exits. collect_md tunnels reject
> non-NONE encap after the replacement, and ip6_tnl_encap() can also fail
> later. In those cases the helper returns an error to its callers while
> the caller still only has the original skb pointer.
>
> Both ip6_tnl_start_xmit() and the IPv6 GRE paths free the caller skb on
> error, so they can end up freeing an skb that ip6_tnl_xmit() already
> consumed.
>
> Use skb_cow_head() instead. It provides the required headroom and
> writability without privately replacing the caller-owned skb, so later
> error returns cannot leave callers with a stale pointer.
>
> Fixes: 058214a4d1df ("ip6_tun: Add infrastructure for doing encapsulation")
> Cc: [email protected]
> Reported-by: Vega <[email protected]>
> Signed-off-by: Zhiling Zou <[email protected]>
I read the feedback from Sashiko [1], but nothing there seems actionable
other than adding a note to the commit message about the removal of the
skb_shared() handling, but I think we can live without it.
__gre6_xmit() and ip6erspan_tunnel_xmit() already call skb_cow_head()
before calling ip6_tnl_xmit().
Reviewed-by: Ido Schimmel <[email protected]>
[1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/7f099879785257f4d57d6caf9b6308fc76c7aaea.1785734738.git.zhilinz%40nebusec.ai