Re: [PATCH net 1/2] ovpn: don't deref NULL key slot in ovpn_crypto_kill_key()

Antonio Quartulli <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Organization OpenVPN Inc.
Message-ID <[email protected]>
Hi Junrui,

On 05/08/2026 15:29, Junrui Luo via B4 Relay wrote:
> From: Junrui Luo <[email protected]>
> 
> ovpn_crypto_kill_key() is reached from ovpn_encrypt_post() when the
> packet ID space of the key in use has been exhausted and
> ovpn_pktid_xmit_next() returns -ERANGE. It locates the slot holding the
> given key ID by reading cs->slots[0] and cs->slots[1] and comparing
> ->key_id, but it dereferences both pointers without first checking them
> for NULL.
> 
> An empty key slot is a perfectly normal state. Both
> ovpn_crypto_key_slot_delete() and ovpn_crypto_state_release() install
> NULL, and the ordinary rekeying sequence - install a new key in the
> secondary slot, swap, then delete the retired one - leaves primary_idx
> at 1 with slots[0] empty. In that state the very first comparison
> dereferences NULL. key_id sits at offset 0 of struct
> ovpn_crypto_key_slot, so this faults on a read of address 0.
> 
> Every other slot accessor in this file already guards the pointer before
> touching it, e.g. ovpn_crypto_key_id_to_slot():
> 
> 	ks = rcu_dereference(cs->slots[idx]);
> 	if (ks && ks->key_id == key_id)
> 
> Use the same NULL-safe form here.
> 
> Fixes: 89d3c0e4612a ("ovpn: kill key and notify userspace in case of IV exhaustion")
> Reported-by: Yuhao Jiang <[email protected]>
> Assisted-by: Claude:claude-opus-5
> Cc: [email protected]
> Signed-off-by: Junrui Luo <[email protected]>

A patch fixing this issue was already sent over the openvpn-devel 
mailing list:

https://patchwork.openvpn.net/project/ovpn/patch/da59b0f39ad7eb59174bd1ffdc2b5ab5e2499e08.1785318038.git.ralf@mandelbit.com/

and it will be sent to net soonish.

Feel Free to test/review that patch if you want.

Thanks!



-- 
Antonio Quartulli
OpenVPN Inc.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.