tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

crypto: af_alg - fix use-after-free in af_alg_accept() due to bh_lock_sock()

The locking in af_alg_release_parent is broken as the BH socket
lock can only be taken if there is a code-path to handle the case
where the lock is owned by process-context. Instead of adding
such handling, we can fix this by changing the ref counts to
atomic_t.

This patch also modifies the main refcnt to include both normal
and nokey sockets. This way we don't have to fudge the nokey
ref count when a socket changes from nokey to normal.

Credits go to Mauricio Faria de Oliveira who diagnosed this bug
and sent a patch for it:

https://lore.kernel.org/linux-crypto/20200605161657.535043-1-mfo@canonical.com/

Reported-by: Brian Moyles <bmoyles@netflix.com>
Reported-by: Mauricio Faria de Oliveira <mfo@canonical.com>
Fixes: 37f96694cf73 ("crypto: af_alg - Use bh_lock_sock in...")
Cc: <stable@vger.kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Herbert Xu 34c86f4c 819966c0

+22 -35
+11 -15
crypto/af_alg.c
··· 128 128 void af_alg_release_parent(struct sock *sk) 129 129 { 130 130 struct alg_sock *ask = alg_sk(sk); 131 - unsigned int nokey = ask->nokey_refcnt; 132 - bool last = nokey && !ask->refcnt; 131 + unsigned int nokey = atomic_read(&ask->nokey_refcnt); 133 132 134 133 sk = ask->parent; 135 134 ask = alg_sk(sk); 136 135 137 - local_bh_disable(); 138 - bh_lock_sock(sk); 139 - ask->nokey_refcnt -= nokey; 140 - if (!last) 141 - last = !--ask->refcnt; 142 - bh_unlock_sock(sk); 143 - local_bh_enable(); 136 + if (nokey) 137 + atomic_dec(&ask->nokey_refcnt); 144 138 145 - if (last) 139 + if (atomic_dec_and_test(&ask->refcnt)) 146 140 sock_put(sk); 147 141 } 148 142 EXPORT_SYMBOL_GPL(af_alg_release_parent); ··· 181 187 182 188 err = -EBUSY; 183 189 lock_sock(sk); 184 - if (ask->refcnt | ask->nokey_refcnt) 190 + if (atomic_read(&ask->refcnt)) 185 191 goto unlock; 186 192 187 193 swap(ask->type, type); ··· 230 236 int err = -EBUSY; 231 237 232 238 lock_sock(sk); 233 - if (ask->refcnt) 239 + if (atomic_read(&ask->refcnt) != atomic_read(&ask->nokey_refcnt)) 234 240 goto unlock; 235 241 236 242 type = ask->type; ··· 295 301 if (err) 296 302 goto unlock; 297 303 298 - if (nokey || !ask->refcnt++) 304 + if (atomic_inc_return_relaxed(&ask->refcnt) == 1) 299 305 sock_hold(sk); 300 - ask->nokey_refcnt += nokey; 306 + if (nokey) { 307 + atomic_inc(&ask->nokey_refcnt); 308 + atomic_set(&alg_sk(sk2)->nokey_refcnt, 1); 309 + } 301 310 alg_sk(sk2)->parent = sk; 302 311 alg_sk(sk2)->type = type; 303 - alg_sk(sk2)->nokey_refcnt = nokey; 304 312 305 313 newsock->ops = type->ops; 306 314 newsock->state = SS_CONNECTED;
+3 -6
crypto/algif_aead.c
··· 384 384 struct alg_sock *ask = alg_sk(sk); 385 385 386 386 lock_sock(sk); 387 - if (ask->refcnt) 387 + if (!atomic_read(&ask->nokey_refcnt)) 388 388 goto unlock_child; 389 389 390 390 psk = ask->parent; ··· 396 396 if (crypto_aead_get_flags(tfm->aead) & CRYPTO_TFM_NEED_KEY) 397 397 goto unlock; 398 398 399 - if (!pask->refcnt++) 400 - sock_hold(psk); 401 - 402 - ask->refcnt = 1; 403 - sock_put(psk); 399 + atomic_dec(&pask->nokey_refcnt); 400 + atomic_set(&ask->nokey_refcnt, 0); 404 401 405 402 err = 0; 406 403
+3 -6
crypto/algif_hash.c
··· 301 301 struct alg_sock *ask = alg_sk(sk); 302 302 303 303 lock_sock(sk); 304 - if (ask->refcnt) 304 + if (!atomic_read(&ask->nokey_refcnt)) 305 305 goto unlock_child; 306 306 307 307 psk = ask->parent; ··· 313 313 if (crypto_ahash_get_flags(tfm) & CRYPTO_TFM_NEED_KEY) 314 314 goto unlock; 315 315 316 - if (!pask->refcnt++) 317 - sock_hold(psk); 318 - 319 - ask->refcnt = 1; 320 - sock_put(psk); 316 + atomic_dec(&pask->nokey_refcnt); 317 + atomic_set(&ask->nokey_refcnt, 0); 321 318 322 319 err = 0; 323 320
+3 -6
crypto/algif_skcipher.c
··· 211 211 struct alg_sock *ask = alg_sk(sk); 212 212 213 213 lock_sock(sk); 214 - if (ask->refcnt) 214 + if (!atomic_read(&ask->nokey_refcnt)) 215 215 goto unlock_child; 216 216 217 217 psk = ask->parent; ··· 223 223 if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY) 224 224 goto unlock; 225 225 226 - if (!pask->refcnt++) 227 - sock_hold(psk); 228 - 229 - ask->refcnt = 1; 230 - sock_put(psk); 226 + atomic_dec(&pask->nokey_refcnt); 227 + atomic_set(&ask->nokey_refcnt, 0); 231 228 232 229 err = 0; 233 230
+2 -2
include/crypto/if_alg.h
··· 29 29 30 30 struct sock *parent; 31 31 32 - unsigned int refcnt; 33 - unsigned int nokey_refcnt; 32 + atomic_t refcnt; 33 + atomic_t nokey_refcnt; 34 34 35 35 const struct af_alg_type *type; 36 36 void *private;