[PATCH] ipv6: sr: add validity check for algorithm ID

[PATCH] ipv6: sr: add validity check for algorithm ID

[heminhong]

From: Minhong He <heminhong@kylinos.cn>

The seg6_genl_sethmac() directly uses the algid passed in by the user
without checking whether it is an HMAC algorithm supported by the
system. If the algid is invalid, unknown errors may occur during
subsequent use of the HMAC information.

Signed-off-by: Minhong He <heminhong@kylinos.cn>
---
 include/net/seg6_hmac.h | 1 +
 net/ipv6/seg6.c         | 5 +++++
 net/ipv6/seg6_hmac.c    | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/net/seg6_hmac.h b/include/net/seg6_hmac.h
index 24f733b3e3fe..c34e86c99de3 100644
--- a/include/net/seg6_hmac.h
+++ b/include/net/seg6_hmac.h
@@ -49,6 +49,7 @@ extern int seg6_hmac_info_del(struct net *net, u32 key);
 extern int seg6_push_hmac(struct net *net, struct in6_addr *saddr,
 			  struct ipv6_sr_hdr *srh);
 extern bool seg6_hmac_validate_skb(struct sk_buff *skb);
+extern struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id);
 #ifdef CONFIG_IPV6_SEG6_HMAC
 extern int seg6_hmac_init(void);
 extern void seg6_hmac_exit(void);
diff --git a/net/ipv6/seg6.c b/net/ipv6/seg6.c
index 180da19c148c..33c1481ca50a 100644
--- a/net/ipv6/seg6.c
+++ b/net/ipv6/seg6.c
@@ -152,6 +152,7 @@ static int seg6_genl_sethmac(struct sk_buff *skb, struct genl_info *info)
 	struct net *net = genl_info_net(info);
 	struct seg6_pernet_data *sdata;
 	struct seg6_hmac_info *hinfo;
+	struct seg6_hmac_algo *algo;
 	u32 hmackeyid;
 	char *secret;
 	int err = 0;
@@ -175,6 +176,10 @@ static int seg6_genl_sethmac(struct sk_buff *skb, struct genl_info *info)
 	if (slen > SEG6_HMAC_SECRET_LEN)
 		return -EINVAL;
 
+	algo = __hmac_get_algo(algid);
+	if (!algo)
+		return -EINVAL;
+
 	mutex_lock(&sdata->lock);
 	hinfo = seg6_hmac_info_lookup(net, hmackeyid);
 
diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index f78ecb6ad838..1c4858195613 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -107,7 +107,7 @@ static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
 	return tlv;
 }
 
-static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
+struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
 {
 	struct seg6_hmac_algo *algo;
 	int i, alg_count;
-- 
2.25.1

Re: [PATCH] ipv6: sr: add validity check for algorithm ID

[Ido Schimmel]

On Tue, Aug 12, 2025 at 02:19:44PM +0800, heminhong wrote:
> From: Minhong He <heminhong@kylinos.cn>
> 
> The seg6_genl_sethmac() directly uses the algid passed in by the user
> without checking whether it is an HMAC algorithm supported by the
> system. If the algid is invalid, unknown errors may occur during
> subsequent use of the HMAC information.

You should explain the user visible effects from this bug/fix rather
than saying "unknown errors". AFAICT, an invalid HMAC algorithm will
result in packet drops during encap / decap, but I might have missed a
more serious problem.

Fixes tag seems appropriate:

Fixes: 4f4853dc1c9c ("ipv6: sr: implement API to control SR HMAC structure")

And please read:

https://docs.kernel.org/process/maintainer-netdev.html

> 
> Signed-off-by: Minhong He <heminhong@kylinos.cn>
> ---
>  include/net/seg6_hmac.h | 1 +
>  net/ipv6/seg6.c         | 5 +++++
>  net/ipv6/seg6_hmac.c    | 2 +-
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/seg6_hmac.h b/include/net/seg6_hmac.h
> index 24f733b3e3fe..c34e86c99de3 100644
> --- a/include/net/seg6_hmac.h
> +++ b/include/net/seg6_hmac.h
> @@ -49,6 +49,7 @@ extern int seg6_hmac_info_del(struct net *net, u32 key);
>  extern int seg6_push_hmac(struct net *net, struct in6_addr *saddr,
>  			  struct ipv6_sr_hdr *srh);
>  extern bool seg6_hmac_validate_skb(struct sk_buff *skb);
> +extern struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id);
>  #ifdef CONFIG_IPV6_SEG6_HMAC
>  extern int seg6_hmac_init(void);
>  extern void seg6_hmac_exit(void);
> diff --git a/net/ipv6/seg6.c b/net/ipv6/seg6.c
> index 180da19c148c..33c1481ca50a 100644
> --- a/net/ipv6/seg6.c
> +++ b/net/ipv6/seg6.c
> @@ -152,6 +152,7 @@ static int seg6_genl_sethmac(struct sk_buff *skb, struct genl_info *info)
>  	struct net *net = genl_info_net(info);
>  	struct seg6_pernet_data *sdata;
>  	struct seg6_hmac_info *hinfo;
> +	struct seg6_hmac_algo *algo;
>  	u32 hmackeyid;
>  	char *secret;
>  	int err = 0;
> @@ -175,6 +176,10 @@ static int seg6_genl_sethmac(struct sk_buff *skb, struct genl_info *info)
>  	if (slen > SEG6_HMAC_SECRET_LEN)
>  		return -EINVAL;
>  
> +	algo = __hmac_get_algo(algid);
> +	if (!algo)
> +		return -EINVAL;

Another possibility is to keep the HMAC algorithm logic in seg6_hmac.c
and perform the check there. Something like:

diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index f78ecb6ad838..d77b52523b6a 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -304,6 +304,9 @@ int seg6_hmac_info_add(struct net *net, u32 key, struct seg6_hmac_info *hinfo)
 	struct seg6_pernet_data *sdata = seg6_pernet(net);
 	int err;
 
+	if (!__hmac_get_algo(hinfo->alg_id))
+		return -EINVAL;
+
 	err = rhashtable_lookup_insert_fast(&sdata->hmac_infos, &hinfo->node,
 					    rht_params);

> +
>  	mutex_lock(&sdata->lock);
>  	hinfo = seg6_hmac_info_lookup(net, hmackeyid);
>  
> diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
> index f78ecb6ad838..1c4858195613 100644
> --- a/net/ipv6/seg6_hmac.c
> +++ b/net/ipv6/seg6_hmac.c
> @@ -107,7 +107,7 @@ static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
>  	return tlv;
>  }
>  
> -static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
> +struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
>  {
>  	struct seg6_hmac_algo *algo;
>  	int i, alg_count;
> -- 
> 2.25.1
> 
> 

[PATCH net-next v2] ipv6: sr: validate HMAC algorithm ID in seg6_genl_sethmac

[heminhong]

From: Minhong He <heminhong@kylinos.cn>

The seg6_genl_sethmac() directly uses the algorithm ID provided by the
userspace without verifying whether it is an HMAC algorithm supported
by the system.
If an unsupported HMAC algorithm ID is configured, packets using SRv6 HMAC
will be dropped during encapsulation or decapsulation.

Fixes: 4f4853dc1c9c ("ipv6: sr: implement API to control SR HMAC structure")

Signed-off-by: Minhong He <heminhong@kylinos.cn>
---
 include/net/seg6_hmac.h | 1 +
 net/ipv6/seg6.c         | 5 +++++
 net/ipv6/seg6_hmac.c    | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/net/seg6_hmac.h b/include/net/seg6_hmac.h
index 24f733b3e3fe..c34e86c99de3 100644
--- a/include/net/seg6_hmac.h
+++ b/include/net/seg6_hmac.h
@@ -49,6 +49,7 @@ extern int seg6_hmac_info_del(struct net *net, u32 key);
 extern int seg6_push_hmac(struct net *net, struct in6_addr *saddr,
 			  struct ipv6_sr_hdr *srh);
 extern bool seg6_hmac_validate_skb(struct sk_buff *skb);
+extern struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id);
 #ifdef CONFIG_IPV6_SEG6_HMAC
 extern int seg6_hmac_init(void);
 extern void seg6_hmac_exit(void);
diff --git a/net/ipv6/seg6.c b/net/ipv6/seg6.c
index 180da19c148c..33c1481ca50a 100644
--- a/net/ipv6/seg6.c
+++ b/net/ipv6/seg6.c
@@ -152,6 +152,7 @@ static int seg6_genl_sethmac(struct sk_buff *skb, struct genl_info *info)
 	struct net *net = genl_info_net(info);
 	struct seg6_pernet_data *sdata;
 	struct seg6_hmac_info *hinfo;
+	struct seg6_hmac_algo *algo;
 	u32 hmackeyid;
 	char *secret;
 	int err = 0;
@@ -175,6 +176,10 @@ static int seg6_genl_sethmac(struct sk_buff *skb, struct genl_info *info)
 	if (slen > SEG6_HMAC_SECRET_LEN)
 		return -EINVAL;
 
+	algo = __hmac_get_algo(algid);
+	if (!algo)
+		return -EINVAL;
+
 	mutex_lock(&sdata->lock);
 	hinfo = seg6_hmac_info_lookup(net, hmackeyid);
 
diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index f78ecb6ad838..1c4858195613 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -107,7 +107,7 @@ static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
 	return tlv;
 }
 
-static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
+struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
 {
 	struct seg6_hmac_algo *algo;
 	int i, alg_count;
-- 
2.25.1

Re: [PATCH net-next v2] ipv6: sr: validate HMAC algorithm ID in seg6_genl_sethmac

[Kuniyuki Iwashima]

From: heminhong <heminhong@kylinos.cn>
Date: Wed, 13 Aug 2025 14:57:37 +0800
> From: Minhong He <heminhong@kylinos.cn>
> 
> The seg6_genl_sethmac() directly uses the algorithm ID provided by the
> userspace without verifying whether it is an HMAC algorithm supported
> by the system.
> If an unsupported HMAC algorithm ID is configured, packets using SRv6 HMAC
> will be dropped during encapsulation or decapsulation.
> 
> Fixes: 4f4853dc1c9c ("ipv6: sr: implement API to control SR HMAC structure")
> 

nit: no newline here.

> Signed-off-by: Minhong He <heminhong@kylinos.cn>
> ---
>  include/net/seg6_hmac.h | 1 +
>  net/ipv6/seg6.c         | 5 +++++
>  net/ipv6/seg6_hmac.c    | 2 +-
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/seg6_hmac.h b/include/net/seg6_hmac.h
> index 24f733b3e3fe..c34e86c99de3 100644
> --- a/include/net/seg6_hmac.h
> +++ b/include/net/seg6_hmac.h
> @@ -49,6 +49,7 @@ extern int seg6_hmac_info_del(struct net *net, u32 key);
>  extern int seg6_push_hmac(struct net *net, struct in6_addr *saddr,
>  			  struct ipv6_sr_hdr *srh);
>  extern bool seg6_hmac_validate_skb(struct sk_buff *skb);
> +extern struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id);

nit: extern is old fashioned and should be unnecessary.

https://www.kernel.org/doc/html/latest/process/coding-style.html#function-prototypes
---8<---
Do not use the extern keyword with function declarations as this
makes lines longer and isn’t strictly necessary.
---8<---


Also please follow this next time:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#resending-after-review
---8<---
The new version of patches should be posted as a separate thread,
not as a reply to the previous posting. Change log should include
a link to the previous posting
---8<---

Re: [PATCH net-next v2] ipv6: sr: validate HMAC algorithm ID in seg6_genl_sethmac

[Ido Schimmel]

Given the Fixes tag, patch should be targeted at "net" and not
"net-next".

On Wed, Aug 13, 2025 at 02:57:37PM +0800, heminhong wrote:
> From: Minhong He <heminhong@kylinos.cn>
> 
> The seg6_genl_sethmac() directly uses the algorithm ID provided by the
> userspace without verifying whether it is an HMAC algorithm supported
> by the system.
> If an unsupported HMAC algorithm ID is configured, packets using SRv6 HMAC
> will be dropped during encapsulation or decapsulation.
> 
> Fixes: 4f4853dc1c9c ("ipv6: sr: implement API to control SR HMAC structure")
> 
> Signed-off-by: Minhong He <heminhong@kylinos.cn>
> ---
>  include/net/seg6_hmac.h | 1 +
>  net/ipv6/seg6.c         | 5 +++++
>  net/ipv6/seg6_hmac.c    | 2 +-
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/seg6_hmac.h b/include/net/seg6_hmac.h
> index 24f733b3e3fe..c34e86c99de3 100644
> --- a/include/net/seg6_hmac.h
> +++ b/include/net/seg6_hmac.h
> @@ -49,6 +49,7 @@ extern int seg6_hmac_info_del(struct net *net, u32 key);
>  extern int seg6_push_hmac(struct net *net, struct in6_addr *saddr,
>  			  struct ipv6_sr_hdr *srh);
>  extern bool seg6_hmac_validate_skb(struct sk_buff *skb);
> +extern struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id);

Did you see my comment on v1 about fixing it entirely in seg6_hmac.c ?

If you want to perform the check in seg6_genl_sethmac(), then I would
instead expose a function like 'bool seg6_hmac_algo_is_valid(u8 alg_id)'
which internally calls __hmac_get_algo() rather than exposing
__hmac_get_algo().

>  #ifdef CONFIG_IPV6_SEG6_HMAC
>  extern int seg6_hmac_init(void);
>  extern void seg6_hmac_exit(void);
> diff --git a/net/ipv6/seg6.c b/net/ipv6/seg6.c
> index 180da19c148c..33c1481ca50a 100644
> --- a/net/ipv6/seg6.c
> +++ b/net/ipv6/seg6.c
> @@ -152,6 +152,7 @@ static int seg6_genl_sethmac(struct sk_buff *skb, struct genl_info *info)
>  	struct net *net = genl_info_net(info);
>  	struct seg6_pernet_data *sdata;
>  	struct seg6_hmac_info *hinfo;
> +	struct seg6_hmac_algo *algo;
>  	u32 hmackeyid;
>  	char *secret;
>  	int err = 0;
> @@ -175,6 +176,10 @@ static int seg6_genl_sethmac(struct sk_buff *skb, struct genl_info *info)
>  	if (slen > SEG6_HMAC_SECRET_LEN)
>  		return -EINVAL;
>  
> +	algo = __hmac_get_algo(algid);
> +	if (!algo)
> +		return -EINVAL;
> +
>  	mutex_lock(&sdata->lock);
>  	hinfo = seg6_hmac_info_lookup(net, hmackeyid);
>  
> diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
> index f78ecb6ad838..1c4858195613 100644
> --- a/net/ipv6/seg6_hmac.c
> +++ b/net/ipv6/seg6_hmac.c
> @@ -107,7 +107,7 @@ static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
>  	return tlv;
>  }
>  
> -static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
> +struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
>  {
>  	struct seg6_hmac_algo *algo;
>  	int i, alg_count;
> -- 
> 2.25.1
> 

[PATCH net v3] ipv6: sr: validate HMAC algorithm ID in seg6_hmac_info_add

[heminhong]

From: Minhong He <heminhong@kylinos.cn>

The seg6_genl_sethmac() directly uses the algorithm ID provided by the
userspace without verifying whether it is an HMAC algorithm supported
by the system.
If an unsupported HMAC algorithm ID is configured, packets using SRv6 HMAC
will be dropped during encapsulation or decapsulation.
To keep the HMAC algorithm logic in seg6_hmac.c and perform the check
in seg6_hmac_info_add().

Fixes: 4f4853dc1c9c ("ipv6: sr: implement API to control SR HMAC structure")
Signed-off-by: Minhong He <heminhong@kylinos.cn>
---
 net/ipv6/seg6_hmac.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index f78ecb6ad838..d77b52523b6a 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -304,6 +304,9 @@ int seg6_hmac_info_add(struct net *net, u32 key, struct seg6_hmac_info *hinfo)
 	struct seg6_pernet_data *sdata = seg6_pernet(net);
 	int err;
 
+	if (!__hmac_get_algo(hinfo->alg_id))
+		return -EINVAL;
+
 	err = rhashtable_lookup_insert_fast(&sdata->hmac_infos, &hinfo->node,
 					    rht_params);
 
-- 
2.25.1

Re: [PATCH net v3] ipv6: sr: validate HMAC algorithm ID in seg6_hmac_info_add

[Ido Schimmel]

On Thu, Aug 14, 2025 at 02:33:02PM +0800, heminhong wrote:
> From: Minhong He <heminhong@kylinos.cn>
> 
> The seg6_genl_sethmac() directly uses the algorithm ID provided by the
> userspace without verifying whether it is an HMAC algorithm supported
> by the system.
> If an unsupported HMAC algorithm ID is configured, packets using SRv6 HMAC
> will be dropped during encapsulation or decapsulation.
> To keep the HMAC algorithm logic in seg6_hmac.c and perform the check
> in seg6_hmac_info_add().

The "To" doesn't seem necessary in the last sentence?

> 
> Fixes: 4f4853dc1c9c ("ipv6: sr: implement API to control SR HMAC structure")
> Signed-off-by: Minhong He <heminhong@kylinos.cn>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

[PATCH net v4] ipv6: sr: validate HMAC algorithm ID in seg6_hmac_info_add

[Minhong He]

The seg6_genl_sethmac() directly uses the algorithm ID provided by the
userspace without verifying whether it is an HMAC algorithm supported
by the system.
If an unsupported HMAC algorithm ID is configured, packets using SRv6 HMAC
will be dropped during encapsulation or decapsulation.

Fixes: 4f4853dc1c9c ("ipv6: sr: implement API to control SR HMAC structure")
Signed-off-by: Minhong He <heminhong@kylinos.cn>
---
 net/ipv6/seg6_hmac.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index f78ecb6ad838..d77b52523b6a 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -304,6 +304,9 @@ int seg6_hmac_info_add(struct net *net, u32 key, struct seg6_hmac_info *hinfo)
 	struct seg6_pernet_data *sdata = seg6_pernet(net);
 	int err;
 
+	if (!__hmac_get_algo(hinfo->alg_id))
+		return -EINVAL;
+
 	err = rhashtable_lookup_insert_fast(&sdata->hmac_infos, &hinfo->node,
 					    rht_params);
 
-- 
2.25.1

Re: [PATCH net v4] ipv6: sr: validate HMAC algorithm ID in seg6_hmac_info_add

[Kuniyuki Iwashima]

On Thu, Aug 14, 2025 at 11:39 PM Minhong He <heminhong@kylinos.cn> wrote:
>
> The seg6_genl_sethmac() directly uses the algorithm ID provided by the
> userspace without verifying whether it is an HMAC algorithm supported
> by the system.
> If an unsupported HMAC algorithm ID is configured, packets using SRv6 HMAC
> will be dropped during encapsulation or decapsulation.
>
> Fixes: 4f4853dc1c9c ("ipv6: sr: implement API to control SR HMAC structure")
> Signed-off-by: Minhong He <heminhong@kylinos.cn>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>


> ---
>  net/ipv6/seg6_hmac.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
> index f78ecb6ad838..d77b52523b6a 100644
> --- a/net/ipv6/seg6_hmac.c
> +++ b/net/ipv6/seg6_hmac.c
> @@ -304,6 +304,9 @@ int seg6_hmac_info_add(struct net *net, u32 key, struct seg6_hmac_info *hinfo)
>         struct seg6_pernet_data *sdata = seg6_pernet(net);
>         int err;
>
> +       if (!__hmac_get_algo(hinfo->alg_id))
> +               return -EINVAL;
> +
>         err = rhashtable_lookup_insert_fast(&sdata->hmac_infos, &hinfo->node,
>                                             rht_params);
>
> --
> 2.25.1
>

Re: [PATCH net v4] ipv6: sr: validate HMAC algorithm ID in seg6_hmac_info_add

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 15 Aug 2025 14:38:45 +0800 you wrote:
> The seg6_genl_sethmac() directly uses the algorithm ID provided by the
> userspace without verifying whether it is an HMAC algorithm supported
> by the system.
> If an unsupported HMAC algorithm ID is configured, packets using SRv6 HMAC
> will be dropped during encapsulation or decapsulation.
> 
> Fixes: 4f4853dc1c9c ("ipv6: sr: implement API to control SR HMAC structure")
> Signed-off-by: Minhong He <heminhong@kylinos.cn>
> 
> [...]

Here is the summary with links:
  - [net,v4] ipv6: sr: validate HMAC algorithm ID in seg6_hmac_info_add
    https://git.kernel.org/netdev/net/c/84967deee9d9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html