| Message ID | 20250209193840.20509-2-justin.iurman@uliege.be (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | several fixes for ioam6, rpl and seg6 lwtunnels | expand |
On 2/9/25 20:38, Justin Iurman wrote: > As a follow up to 92191dd10730 ("net: ipv6: fix dst ref loops in rpl, > seg6 and ioam6 lwtunnels"), we also need a conditional dst cache on > input for seg6_iptunnel and rpl_iptunnel to prevent dst ref loops (i.e., > if the packet destination did not change, we may end up recording a > reference to the lwtunnel in its own cache, and the lwtunnel state will > never be freed). > > Fixes: a7a29f9c361f ("net: ipv6: add rpl sr tunnel") > Fixes: af4a2209b134 ("ipv6: sr: use dst_cache in seg6_input") > Signed-off-by: Justin Iurman <justin.iurman@uliege.be> > Cc: Alexander Aring <aahringo@redhat.com> > Cc: David Lebrun <dlebrun@google.com> > --- > net/ipv6/rpl_iptunnel.c | 14 ++++++++++++-- > net/ipv6/seg6_iptunnel.c | 14 ++++++++++++-- > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/net/ipv6/rpl_iptunnel.c b/net/ipv6/rpl_iptunnel.c > index 0ac4283acdf2..c26bf284459f 100644 > --- a/net/ipv6/rpl_iptunnel.c > +++ b/net/ipv6/rpl_iptunnel.c > @@ -262,10 +262,18 @@ static int rpl_input(struct sk_buff *skb) > { > struct dst_entry *orig_dst = skb_dst(skb); > struct dst_entry *dst = NULL; > + struct lwtunnel_state *lwtst; > struct rpl_lwt *rlwt; > int err; > > - rlwt = rpl_lwt_lwtunnel(orig_dst->lwtstate); > + /* Get the address of lwtstate now, because "orig_dst" may not be there > + * anymore after a call to skb_dst_drop(). Note that ip6_route_input() > + * also calls skb_dst_drop(). Below, we compare the address of lwtstate > + * to detect loops. > + */ > + lwtst = orig_dst->lwtstate; > + > + rlwt = rpl_lwt_lwtunnel(lwtst); > > local_bh_disable(); > dst = dst_cache_get(&rlwt->cache); > @@ -280,7 +288,9 @@ static int rpl_input(struct sk_buff *skb) > if (!dst) { > ip6_route_input(skb); > dst = skb_dst(skb); > - if (!dst->error) { > + > + /* cache only if we don't create a dst reference loop */ > + if (!dst->error && lwtst != dst->lwtstate) { > local_bh_disable(); > dst_cache_set_ip6(&rlwt->cache, dst, > &ipv6_hdr(skb)->saddr); > diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c > index 33833b2064c0..6045e850b4bf 100644 > --- a/net/ipv6/seg6_iptunnel.c > +++ b/net/ipv6/seg6_iptunnel.c > @@ -472,10 +472,18 @@ static int seg6_input_core(struct net *net, struct sock *sk, > { > struct dst_entry *orig_dst = skb_dst(skb); > struct dst_entry *dst = NULL; > + struct lwtunnel_state *lwtst; > struct seg6_lwt *slwt; > int err; > > - slwt = seg6_lwt_lwtunnel(orig_dst->lwtstate); > + /* Get the address of lwtstate now, because "orig_dst" may not be there > + * anymore after a call to skb_dst_drop(). Note that ip6_route_input() > + * also calls skb_dst_drop(). Below, we compare the address of lwtstate > + * to detect loops. > + */ > + lwtst = orig_dst->lwtstate; > + > + slwt = seg6_lwt_lwtunnel(lwtst); > > local_bh_disable(); > dst = dst_cache_get(&slwt->cache); > @@ -490,7 +498,9 @@ static int seg6_input_core(struct net *net, struct sock *sk, > if (!dst) { > ip6_route_input(skb); > dst = skb_dst(skb); > - if (!dst->error) { > + > + /* cache only if we don't create a dst reference loop */ > + if (!dst->error && lwtst != dst->lwtstate) { > local_bh_disable(); > dst_cache_set_ip6(&slwt->cache, dst, > &ipv6_hdr(skb)->saddr); ... which makes me think while we're at it: should we directly drop the packet when dst->error (just like it's already done in output() handlers)? Not sure it's useful to send a DestUnreach in this case, especially considering that it may leak data. At least, we probably need to have consistency between input and output behavior. Thoughts?
diff --git a/net/ipv6/rpl_iptunnel.c b/net/ipv6/rpl_iptunnel.c index 0ac4283acdf2..c26bf284459f 100644 --- a/net/ipv6/rpl_iptunnel.c +++ b/net/ipv6/rpl_iptunnel.c @@ -262,10 +262,18 @@ static int rpl_input(struct sk_buff *skb) { struct dst_entry *orig_dst = skb_dst(skb); struct dst_entry *dst = NULL; + struct lwtunnel_state *lwtst; struct rpl_lwt *rlwt; int err; - rlwt = rpl_lwt_lwtunnel(orig_dst->lwtstate); + /* Get the address of lwtstate now, because "orig_dst" may not be there + * anymore after a call to skb_dst_drop(). Note that ip6_route_input() + * also calls skb_dst_drop(). Below, we compare the address of lwtstate + * to detect loops. + */ + lwtst = orig_dst->lwtstate; + + rlwt = rpl_lwt_lwtunnel(lwtst); local_bh_disable(); dst = dst_cache_get(&rlwt->cache); @@ -280,7 +288,9 @@ static int rpl_input(struct sk_buff *skb) if (!dst) { ip6_route_input(skb); dst = skb_dst(skb); - if (!dst->error) { + + /* cache only if we don't create a dst reference loop */ + if (!dst->error && lwtst != dst->lwtstate) { local_bh_disable(); dst_cache_set_ip6(&rlwt->cache, dst, &ipv6_hdr(skb)->saddr); diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c index 33833b2064c0..6045e850b4bf 100644 --- a/net/ipv6/seg6_iptunnel.c +++ b/net/ipv6/seg6_iptunnel.c @@ -472,10 +472,18 @@ static int seg6_input_core(struct net *net, struct sock *sk, { struct dst_entry *orig_dst = skb_dst(skb); struct dst_entry *dst = NULL; + struct lwtunnel_state *lwtst; struct seg6_lwt *slwt; int err; - slwt = seg6_lwt_lwtunnel(orig_dst->lwtstate); + /* Get the address of lwtstate now, because "orig_dst" may not be there + * anymore after a call to skb_dst_drop(). Note that ip6_route_input() + * also calls skb_dst_drop(). Below, we compare the address of lwtstate + * to detect loops. + */ + lwtst = orig_dst->lwtstate; + + slwt = seg6_lwt_lwtunnel(lwtst); local_bh_disable(); dst = dst_cache_get(&slwt->cache); @@ -490,7 +498,9 @@ static int seg6_input_core(struct net *net, struct sock *sk, if (!dst) { ip6_route_input(skb); dst = skb_dst(skb); - if (!dst->error) { + + /* cache only if we don't create a dst reference loop */ + if (!dst->error && lwtst != dst->lwtstate) { local_bh_disable(); dst_cache_set_ip6(&slwt->cache, dst, &ipv6_hdr(skb)->saddr);
As a follow up to 92191dd10730 ("net: ipv6: fix dst ref loops in rpl, seg6 and ioam6 lwtunnels"), we also need a conditional dst cache on input for seg6_iptunnel and rpl_iptunnel to prevent dst ref loops (i.e., if the packet destination did not change, we may end up recording a reference to the lwtunnel in its own cache, and the lwtunnel state will never be freed). Fixes: a7a29f9c361f ("net: ipv6: add rpl sr tunnel") Fixes: af4a2209b134 ("ipv6: sr: use dst_cache in seg6_input") Signed-off-by: Justin Iurman <justin.iurman@uliege.be> Cc: Alexander Aring <aahringo@redhat.com> Cc: David Lebrun <dlebrun@google.com> --- net/ipv6/rpl_iptunnel.c | 14 ++++++++++++-- net/ipv6/seg6_iptunnel.c | 14 ++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-)