net: reject a forward path that loops back to the tunnel

[PATCH net] net: reject a forward path that loops back to the tunnel

Posted by Farhad Alemi 2 weeks ago

ipip_fill_forward_path() and ip6_tnl_fill_forward_path() assign the outer
route's device to ctx->dev without checking that it differs from the tunnel
device itself.  When a tunnel's outer route resolves back to that same
tunnel, the walk in dev_fill_forward_path() makes no progress and trips its
loop check WARN_ON_ONCE(last_dev == ctx->dev). Release the route and
return -EOPNOTSUPP when the resolved dst device equals ctx->dev, so the
path walk either advances to a different device or fails cleanly.

Closes: https://lore.kernel.org/all/CA+0ovCgaRvbd0Udj70b2xxG8Cx3CaCpNhnf1V4RWQuDveZYZhA@mail.gmail.com/
Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>
---
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -375,6 +375,12 @@ static int ipip_fill_forward_path(struct
net_device_path_ctx *ctx,
 	if (IS_ERR(rt))
 		return PTR_ERR(rt);

+	/* The path walk must advance: a route back into the tunnel is a loop. */
+	if (rt->dst.dev == ctx->dev) {
+		ip_rt_put(rt);
+		return -EOPNOTSUPP;
+	}
+
 	path->type = DEV_PATH_TUN;
 	path->tun.src_v4.s_addr = tiph->saddr;
 	path->tun.dst_v4.s_addr = tiph->daddr;
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1863,7 +1863,12 @@ static int ip6_tnl_fill_forward_path(struct
net_device_path_ctx *ctx,
 	fl6.flowi6_proto = 0;

 	dst = ip6_route_output(dev_net(ctx->dev), NULL, &fl6);
-	if (!dst->error) {
+	err = dst->error;
+	/* The path walk must advance: a route back into the tunnel is a loop. */
+	if (!err && dst->dev == ctx->dev)
+		err = -EOPNOTSUPP;
+
+	if (!err) {
 		path->type = DEV_PATH_TUN;
 		path->tun.src_v6 = fl6.saddr;
 		path->tun.dst_v6 = fl6.daddr;
@@ -1873,7 +1878,6 @@ static int ip6_tnl_fill_forward_path(struct
net_device_path_ctx *ctx,
 		ctx->dev = dst->dev;
 	}

-	err = dst->error;
 	if (err)
 		dst_release(dst);

Re: [PATCH net] net: reject a forward path that loops back to the tunnel

Posted by Pablo Neira Ayuso 2 weeks ago

Hi,

On Fri, Sep 11, 2026 at 12:17:08AM +0000, Farhad Alemi wrote:
> ipip_fill_forward_path() and ip6_tnl_fill_forward_path() assign the outer
> route's device to ctx->dev without checking that it differs from the tunnel
> device itself.  When a tunnel's outer route resolves back to that same
> tunnel, the walk in dev_fill_forward_path() makes no progress and trips its
> loop check WARN_ON_ONCE(last_dev == ctx->dev). Release the route and
> return -EOPNOTSUPP when the resolved dst device equals ctx->dev, so the
> path walk either advances to a different device or fails cleanly.

I think we can remove this WARN_ON_ONCE() from dev_fill_forward_path(), ie.

diff --git a/net/core/dev.c b/net/core/dev.c
index ecfbd72d5d1a..c67900354fa6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -789,7 +789,7 @@ int dev_fill_forward_path(struct net_device_path_ctx *ctx,
                        goto err_out;
 
                stack->num_paths++;
-               if (WARN_ON_ONCE(last_dev == ctx->dev))
+               if (last_dev == ctx->dev)
                        goto err_out;
        }
 
> Closes: https://lore.kernel.org/all/CA+0ovCgaRvbd0Udj70b2xxG8Cx3CaCpNhnf1V4RWQuDveZYZhA@mail.gmail.com/
> Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>
> ---
> --- a/net/ipv4/ipip.c
> +++ b/net/ipv4/ipip.c
> @@ -375,6 +375,12 @@ static int ipip_fill_forward_path(struct
> net_device_path_ctx *ctx,
>  	if (IS_ERR(rt))
>  		return PTR_ERR(rt);
> 
> +	/* The path walk must advance: a route back into the tunnel is a loop. */
> +	if (rt->dst.dev == ctx->dev) {
> +		ip_rt_put(rt);
> +		return -EOPNOTSUPP;
> +	}
> +
>  	path->type = DEV_PATH_TUN;
>  	path->tun.src_v4.s_addr = tiph->saddr;
>  	path->tun.dst_v4.s_addr = tiph->daddr;
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -1863,7 +1863,12 @@ static int ip6_tnl_fill_forward_path(struct
> net_device_path_ctx *ctx,
>  	fl6.flowi6_proto = 0;
> 
>  	dst = ip6_route_output(dev_net(ctx->dev), NULL, &fl6);
> -	if (!dst->error) {
> +	err = dst->error;
> +	/* The path walk must advance: a route back into the tunnel is a loop. */
> +	if (!err && dst->dev == ctx->dev)
> +		err = -EOPNOTSUPP;
> +
> +	if (!err) {
>  		path->type = DEV_PATH_TUN;
>  		path->tun.src_v6 = fl6.saddr;
>  		path->tun.dst_v6 = fl6.daddr;
> @@ -1873,7 +1878,6 @@ static int ip6_tnl_fill_forward_path(struct
> net_device_path_ctx *ctx,
>  		ctx->dev = dst->dev;
>  	}
> 
> -	err = dst->error;
>  	if (err)
>  		dst_release(dst);

Re: [PATCH net] net: reject a forward path that loops back to the tunnel

Posted by Farhad Alemi 1 week, 6 days ago

Xuanqiang and Pablo, many thanks for the helpful hints. I’ve sent out
a v2 addressing the comments above. Please let me know if any further
modifications are needed. Thanks!

On Fri, Sep 11, 2026 at 7:22 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> Hi,
>
> On Fri, Sep 11, 2026 at 12:17:08AM +0000, Farhad Alemi wrote:
> > ipip_fill_forward_path() and ip6_tnl_fill_forward_path() assign the outer
> > route's device to ctx->dev without checking that it differs from the tunnel
> > device itself.  When a tunnel's outer route resolves back to that same
> > tunnel, the walk in dev_fill_forward_path() makes no progress and trips its
> > loop check WARN_ON_ONCE(last_dev == ctx->dev). Release the route and
> > return -EOPNOTSUPP when the resolved dst device equals ctx->dev, so the
> > path walk either advances to a different device or fails cleanly.
>
> I think we can remove this WARN_ON_ONCE() from dev_fill_forward_path(), ie.
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ecfbd72d5d1a..c67900354fa6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -789,7 +789,7 @@ int dev_fill_forward_path(struct net_device_path_ctx *ctx,
>                         goto err_out;
>
>                 stack->num_paths++;
> -               if (WARN_ON_ONCE(last_dev == ctx->dev))
> +               if (last_dev == ctx->dev)
>                         goto err_out;
>         }
>
> > Closes: https://lore.kernel.org/all/CA+0ovCgaRvbd0Udj70b2xxG8Cx3CaCpNhnf1V4RWQuDveZYZhA@mail.gmail.com/
> > Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>
> > ---
> > --- a/net/ipv4/ipip.c
> > +++ b/net/ipv4/ipip.c
> > @@ -375,6 +375,12 @@ static int ipip_fill_forward_path(struct
> > net_device_path_ctx *ctx,
> >       if (IS_ERR(rt))
> >               return PTR_ERR(rt);
> >
> > +     /* The path walk must advance: a route back into the tunnel is a loop. */
> > +     if (rt->dst.dev == ctx->dev) {
> > +             ip_rt_put(rt);
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> >       path->type = DEV_PATH_TUN;
> >       path->tun.src_v4.s_addr = tiph->saddr;
> >       path->tun.dst_v4.s_addr = tiph->daddr;
> > --- a/net/ipv6/ip6_tunnel.c
> > +++ b/net/ipv6/ip6_tunnel.c
> > @@ -1863,7 +1863,12 @@ static int ip6_tnl_fill_forward_path(struct
> > net_device_path_ctx *ctx,
> >       fl6.flowi6_proto = 0;
> >
> >       dst = ip6_route_output(dev_net(ctx->dev), NULL, &fl6);
> > -     if (!dst->error) {
> > +     err = dst->error;
> > +     /* The path walk must advance: a route back into the tunnel is a loop. */
> > +     if (!err && dst->dev == ctx->dev)
> > +             err = -EOPNOTSUPP;
> > +
> > +     if (!err) {
> >               path->type = DEV_PATH_TUN;
> >               path->tun.src_v6 = fl6.saddr;
> >               path->tun.dst_v6 = fl6.daddr;
> > @@ -1873,7 +1878,6 @@ static int ip6_tnl_fill_forward_path(struct
> > net_device_path_ctx *ctx,
> >               ctx->dev = dst->dev;
> >       }
> >
> > -     err = dst->error;
> >       if (err)
> >               dst_release(dst);

Re: [PATCH net] net: reject a forward path that loops back to the tunnel

Posted by Xuanqiang Luo 2 weeks ago

> ipip_fill_forward_path() and ip6_tnl_fill_forward_path() assign the outer
> route's device to ctx->dev without checking that it differs from the tunnel
> device itself.  When a tunnel's outer route resolves back to that same
> tunnel, the walk in dev_fill_forward_path() makes no progress and trips its
> loop check WARN_ON_ONCE(last_dev == ctx->dev). Release the route and
> return -EOPNOTSUPP when the resolved dst device equals ctx->dev, so the
> path walk either advances to a different device or fails cleanly.
>
> Closes: https://lore.kernel.org/all/CA+0ovCgaRvbd0Udj70b2xxG8Cx3CaCpNhnf1V4RWQuDveZYZhA@mail.gmail.com/
> Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>

Please add the missing Fixes tags. These appear to be the relevant
commits:

Fixes: ab427db178858 ("netfilter: flowtable: Add IPIP rx sw acceleration")
Fixes: d98103575dcdd ("netfilter: flowtable: Add IP6IP6 rx sw acceleration")

Thanks,
Xuanqiang