linux-kernel - Re: [PATCH net] ipv6: sr: fix destroy of seg6_hmac

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89i+UTv8nJ=cc67iKky=MLXOnzF5XyVRsV-TMXz7wUQ6Yvw@mail.gmail.com>
Date: Mon, 25 Aug 2025 12:33:26 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Andrea Mayer <andrea.mayer@...roma2.it>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...nel.org>, Simon Horman <horms@...nel.org>, 
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org, 
	Eric Biggers <ebiggers@...nel.org>, David Lebrun <dlebrun@...gle.com>, 
	Stefano Salsano <stefano.salsano@...roma2.it>, Paolo Lungaroni <paolo.lungaroni@...roma2.it>, 
	Ahmed Abdelsalam <ahabdels.dev@...il.com>, stable@...r.kernel.org
Subject: Re: [PATCH net] ipv6: sr: fix destroy of seg6_hmac_info to prevent
 HMAC data leak

On Mon, Aug 25, 2025 at 12:08 PM Andrea Mayer <andrea.mayer@...roma2.it> wrote:
>
> The seg6_hmac_info structure stores information related to SRv6 HMAC
> configurations, including the secret key, HMAC ID, and hashing algorithm
> used to authenticate and secure SRv6 packets.
>
> When a seg6_hmac_info object is no longer needed, it is destroyed via
> seg6_hmac_info_del(), which eventually calls seg6_hinfo_release(). This
> function uses kfree_rcu() to safely deallocate memory after an RCU grace
> period has elapsed.
> The kfree_rcu() releases memory without sanitization (e.g., zeroing out
> the memory). Consequently, sensitive information such as the HMAC secret
> and its length may remain in freed memory, potentially leading to data
> leaks.
>
> To address this risk, we replaced kfree_rcu() with a custom RCU
> callback, seg6_hinfo_free_callback_rcu(). Within this callback, we
> explicitly sanitize the seg6_hmac_info object before deallocating it
> safely using kfree_sensitive(). This approach ensures the memory is
> securely freed and prevents potential HMAC info leaks.
> Additionally, in the control path, we ensure proper cleanup of
> seg6_hmac_info objects when seg6_hmac_info_add() fails: such objects are
> freed using kfree_sensitive() instead of kfree().
>
> Fixes: 4f4853dc1c9c ("ipv6: sr: implement API to control SR HMAC structure")
> Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support")

Not sure if you are fixing a bug worth backports.

> Cc: stable@...r.kernel.org
> Signed-off-by: Andrea Mayer <andrea.mayer@...roma2.it>
> ---
>  net/ipv6/seg6.c      |  2 +-
>  net/ipv6/seg6_hmac.c | 10 +++++++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv6/seg6.c b/net/ipv6/seg6.c
> index 180da19c148c..88782bdab843 100644
> --- a/net/ipv6/seg6.c
> +++ b/net/ipv6/seg6.c
> @@ -215,7 +215,7 @@ static int seg6_genl_sethmac(struct sk_buff *skb, struct genl_info *info)
>
>         err = seg6_hmac_info_add(net, hmackeyid, hinfo);
>         if (err)
> -               kfree(hinfo);
> +               kfree_sensitive(hinfo);
>
>  out_unlock:
>         mutex_unlock(&sdata->lock);
> diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
> index fd58426f222b..19cdf3791ebf 100644
> --- a/net/ipv6/seg6_hmac.c
> +++ b/net/ipv6/seg6_hmac.c
> @@ -57,9 +57,17 @@ static int seg6_hmac_cmpfn(struct rhashtable_compare_arg *arg, const void *obj)
>         return (hinfo->hmackeyid != *(__u32 *)arg->key);
>  }
>
> +static void seg6_hinfo_free_callback_rcu(struct rcu_head *head)
> +{
> +       struct seg6_hmac_info *hinfo;
> +
> +       hinfo = container_of(head, struct seg6_hmac_info, rcu);
> +       kfree_sensitive(hinfo);
> +}
> +
>  static inline void seg6_hinfo_release(struct seg6_hmac_info *hinfo)
>  {
> -       kfree_rcu(hinfo, rcu);
> +       call_rcu(&hinfo->rcu, seg6_hinfo_free_callback_rcu);
>  }

If we worry a lot about sensitive data waiting too much in RCU land,
perhaps use call_rcu_hurry() here ?