On Mon, Aug 25, 2025 at 12:33:26PM -0700, Eric Dumazet wrote:
On Mon, Aug 25, 2025 at 12:08 PM Andrea Mayer andrea.mayer@uniroma2.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.
It can be considered a bug fix, or just hardening. There are examples of both ways for this same type of issue. I think the patch is fine as-is, though the commit message is a bit long. Zeroizing crypto keys is a best practice that the kernel tries to follow elsewhere for all crypto keys, so this is nothing new. The patch simply adds zeroization before freeing for a struct that contains a key.
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 ?
No, zeroization doesn't have stringent time constraints. As long as it happens eventually it is fine.
Reviewed-by: Eric Biggers ebiggers@kernel.org
- Eric