ovpn: fix NULL dereference when killing missing key

ovpn_crypto_kill_key assumes both crypto slots are populated and
dereferences each slot before checking it. That is not guaranteed: a
peer can have only one installed key, and the kill path may be asked to
remove a key that is not present.

Read each slot once while holding the crypto state lock, check for NULL
before looking at key_id, and only replace the slot that actually
matches.

Fixes: 89d3c0e4612a ("ovpn: kill key and notify userspace in case of IV exhaustion")
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

diff --git a/drivers/net/ovpn/crypto.c b/drivers/net/ovpn/crypto.c
index 90580e32052fb56c646a6df7816872366133bc75..2e95f29514fc4a527b1bb157092ca98e2bd025f0 100644 (file)
--- a/drivers/net/ovpn/crypto.c
+++ b/drivers/net/ovpn/crypto.c
@@ -58,15 +58,19 @@ void ovpn_crypto_state_release(struct ovpn_crypto_state *cs)
 bool ovpn_crypto_kill_key(struct ovpn_crypto_state *cs, u8 key_id)
 {
        struct ovpn_crypto_key_slot *ks = NULL;
+       struct ovpn_crypto_key_slot *tmp;
+       int slot = 0;
 
        spin_lock_bh(&cs->lock);
-       if (rcu_access_pointer(cs->slots[0])->key_id == key_id) {
-               ks = rcu_replace_pointer(cs->slots[0], NULL,
-                                        lockdep_is_held(&cs->lock));
-       } else if (rcu_access_pointer(cs->slots[1])->key_id == key_id) {
-               ks = rcu_replace_pointer(cs->slots[1], NULL,
-                                        lockdep_is_held(&cs->lock));
+       tmp = rcu_access_pointer(cs->slots[slot]);
+       if (!tmp || tmp->key_id != key_id) {
+               slot = 1;
+               tmp = rcu_access_pointer(cs->slots[slot]);
        }
+
+       if (tmp && tmp->key_id == key_id)
+               ks = rcu_replace_pointer(cs->slots[slot], NULL,
+                                        lockdep_is_held(&cs->lock));
        spin_unlock_bh(&cs->lock);
 
        if (ks)