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

[Ido Schimmel <idosch@idosch.org>]

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
>