diff mbox series

[net,4/7] net: ipv6: seg6_local: fix lwtunnel_input() loop

Message ID 20250311141238.19862-5-justin.iurman@uliege.be (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: fix lwtunnel reentry loops | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 1 blamed authors not CCed: daniel@iogearbox.net; 1 maintainers not CCed: daniel@iogearbox.net
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 211 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Justin Iurman March 11, 2025, 2:12 p.m. UTC 
Fix the lwtunnel_input() reentry loop in seg6_local when the destination
is the same after transformation. Some configurations leading to this
may be considered pathological, but we don't want the kernel to crash
even for these ones. This patch DOES NOT solve the crash reported in
[1], it'll be addressed separately as it's a different issue.

  [1] https://lore.kernel.org/netdev/2bc9e2079e864a9290561894d2a602d6@akamai.com/

Fixes: 140f04c33bbc ("ipv6: sr: implement several seg6local actions")
Fixes: 891ef8dd2a8d ("ipv6: sr: implement additional seg6local actions")
Fixes: 004d4b274e2a ("ipv6: sr: Add seg6local action End.BPF")
Fixes: 664d6f86868b ("seg6: add support for the SRv6 End.DT4 behavior")
Cc: David Lebrun <dlebrun@google.com>
Cc: Andrea Mayer <andrea.mayer@uniroma2.it>
Cc: Stefano Salsano <stefano.salsano@uniroma2.it>
Cc: Ahmed Abdelsalam <ahabdels.dev@gmail.com>
Cc: Mathieu Xhonneux <m.xhonneux@gmail.com>
Cc: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Justin Iurman <justin.iurman@uliege.be>
---
 net/ipv6/seg6_local.c | 85 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 81 insertions(+), 4 deletions(-)

Comments

Paolo Abeni March 13, 2025, 12:40 p.m. UTC | #1 
On 3/11/25 3:12 PM, Justin Iurman wrote:
> ---
>  net/ipv6/seg6_local.c | 85 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 81 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
> index ac1dbd492c22..15485010cdfb 100644
> --- a/net/ipv6/seg6_local.c
> +++ b/net/ipv6/seg6_local.c
> @@ -378,8 +378,16 @@ static void seg6_next_csid_advance_arg(struct in6_addr *addr,
>  static int input_action_end_finish(struct sk_buff *skb,
>  				   struct seg6_local_lwt *slwt)
>  {
> +	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
> +
>  	seg6_lookup_nexthop(skb, NULL, 0);
>  
> +	/* avoid lwtunnel_input() reentry loop when destination is the same
> +	 * after transformation
> +	 */
> +	if (lwtst == skb_dst(skb)->lwtstate)
> +		return lwtst->orig_input(skb);
> +
>  	return dst_input(skb);

The above few lines are repeted a lot of times below. Please factor them
out in an helper and re-use it.

Thanks,

Paolo
Justin Iurman March 13, 2025, 2:45 p.m. UTC | #2 
On 3/13/25 13:40, Paolo Abeni wrote:
> On 3/11/25 3:12 PM, Justin Iurman wrote:
>> ---
>>   net/ipv6/seg6_local.c | 85 +++++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 81 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
>> index ac1dbd492c22..15485010cdfb 100644
>> --- a/net/ipv6/seg6_local.c
>> +++ b/net/ipv6/seg6_local.c
>> @@ -378,8 +378,16 @@ static void seg6_next_csid_advance_arg(struct in6_addr *addr,
>>   static int input_action_end_finish(struct sk_buff *skb,
>>   				   struct seg6_local_lwt *slwt)
>>   {
>> +	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
>> +
>>   	seg6_lookup_nexthop(skb, NULL, 0);
>>   
>> +	/* avoid lwtunnel_input() reentry loop when destination is the same
>> +	 * after transformation
>> +	 */
>> +	if (lwtst == skb_dst(skb)->lwtstate)
>> +		return lwtst->orig_input(skb);
>> +
>>   	return dst_input(skb);
> 
> The above few lines are repeted a lot of times below. Please factor them
> out in an helper and re-use it.

+1, although this patch (and some others) will be removed from the 
series in -v2. That said, Paolo's remark may be applied to Andrea's 
seg6-related patches that will follow.

> Thanks,
> 
> Paolo
>
diff mbox series

Patch

diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index ac1dbd492c22..15485010cdfb 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -378,8 +378,16 @@  static void seg6_next_csid_advance_arg(struct in6_addr *addr,
 static int input_action_end_finish(struct sk_buff *skb,
 				   struct seg6_local_lwt *slwt)
 {
+	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
+
 	seg6_lookup_nexthop(skb, NULL, 0);
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 }
 
@@ -418,8 +426,16 @@  static int end_next_csid_core(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 static int input_action_end_x_finish(struct sk_buff *skb,
 				     struct seg6_local_lwt *slwt)
 {
+	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
+
 	seg6_lookup_nexthop(skb, &slwt->nh6, 0);
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 }
 
@@ -825,6 +841,7 @@  static int input_action_end_x(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 
 static int input_action_end_t(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 {
+	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
 	struct ipv6_sr_hdr *srh;
 
 	srh = get_and_validate_srh(skb);
@@ -835,6 +852,12 @@  static int input_action_end_t(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 
 	seg6_lookup_nexthop(skb, NULL, slwt->table);
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 
 drop:
@@ -902,11 +925,11 @@  static int input_action_end_dx2(struct sk_buff *skb,
 static int input_action_end_dx6_finish(struct net *net, struct sock *sk,
 				       struct sk_buff *skb)
 {
-	struct dst_entry *orig_dst = skb_dst(skb);
+	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
 	struct in6_addr *nhaddr = NULL;
 	struct seg6_local_lwt *slwt;
 
-	slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
+	slwt = seg6_local_lwtunnel(lwtst);
 
 	/* The inner packet is not associated to any local interface,
 	 * so we do not call netif_rx().
@@ -919,6 +942,12 @@  static int input_action_end_dx6_finish(struct net *net, struct sock *sk,
 
 	seg6_lookup_nexthop(skb, nhaddr, 0);
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 }
 
@@ -953,13 +982,13 @@  static int input_action_end_dx6(struct sk_buff *skb,
 static int input_action_end_dx4_finish(struct net *net, struct sock *sk,
 				       struct sk_buff *skb)
 {
-	struct dst_entry *orig_dst = skb_dst(skb);
+	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
 	enum skb_drop_reason reason;
 	struct seg6_local_lwt *slwt;
 	struct iphdr *iph;
 	__be32 nhaddr;
 
-	slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
+	slwt = seg6_local_lwtunnel(lwtst);
 
 	iph = ip_hdr(skb);
 
@@ -973,6 +1002,12 @@  static int input_action_end_dx4_finish(struct net *net, struct sock *sk,
 		return -EINVAL;
 	}
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 }
 
@@ -1174,6 +1209,7 @@  static struct sk_buff *end_dt_vrf_core(struct sk_buff *skb,
 static int input_action_end_dt4(struct sk_buff *skb,
 				struct seg6_local_lwt *slwt)
 {
+	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
 	enum skb_drop_reason reason;
 	struct iphdr *iph;
 
@@ -1197,6 +1233,12 @@  static int input_action_end_dt4(struct sk_buff *skb,
 	if (unlikely(reason))
 		goto drop;
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 
 drop:
@@ -1255,6 +1297,8 @@  static int seg6_end_dt6_build(struct seg6_local_lwt *slwt, const void *cfg,
 static int input_action_end_dt6(struct sk_buff *skb,
 				struct seg6_local_lwt *slwt)
 {
+	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
+
 	if (!decap_and_validate(skb, IPPROTO_IPV6))
 		goto drop;
 
@@ -1279,6 +1323,12 @@  static int input_action_end_dt6(struct sk_buff *skb,
 	 */
 	seg6_lookup_any_nexthop(skb, NULL, 0, true);
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 
 legacy_mode:
@@ -1287,6 +1337,12 @@  static int input_action_end_dt6(struct sk_buff *skb,
 
 	seg6_lookup_any_nexthop(skb, NULL, slwt->table, true);
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 
 drop:
@@ -1327,6 +1383,7 @@  static int input_action_end_dt46(struct sk_buff *skb,
 /* push an SRH on top of the current one */
 static int input_action_end_b6(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 {
+	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
 	struct ipv6_sr_hdr *srh;
 	int err = -EINVAL;
 
@@ -1342,6 +1399,12 @@  static int input_action_end_b6(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 
 	seg6_lookup_nexthop(skb, NULL, 0);
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 
 drop:
@@ -1353,6 +1416,7 @@  static int input_action_end_b6(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 static int input_action_end_b6_encap(struct sk_buff *skb,
 				     struct seg6_local_lwt *slwt)
 {
+	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
 	struct ipv6_sr_hdr *srh;
 	int err = -EINVAL;
 
@@ -1373,6 +1437,12 @@  static int input_action_end_b6_encap(struct sk_buff *skb,
 
 	seg6_lookup_nexthop(skb, NULL, 0);
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 
 drop:
@@ -1411,6 +1481,7 @@  bool seg6_bpf_has_valid_srh(struct sk_buff *skb)
 static int input_action_end_bpf(struct sk_buff *skb,
 				struct seg6_local_lwt *slwt)
 {
+	struct lwtunnel_state *lwtst = skb_dst(skb)->lwtstate;
 	struct seg6_bpf_srh_state *srh_state;
 	struct ipv6_sr_hdr *srh;
 	int ret;
@@ -1457,6 +1528,12 @@  static int input_action_end_bpf(struct sk_buff *skb,
 	if (ret != BPF_REDIRECT)
 		seg6_lookup_nexthop(skb, NULL, 0);
 
+	/* avoid lwtunnel_input() reentry loop when destination is the same
+	 * after transformation
+	 */
+	if (lwtst == skb_dst(skb)->lwtstate)
+		return lwtst->orig_input(skb);
+
 	return dst_input(skb);
 
 drop: