crypto: af_alg - fix use-after-free in af_alg_accept() due to bh_lock_sock()
commit 34c86f4c4a7be3b3e35aa48bd18299d4c756064d upstream. 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> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
a464152c18
commit
9a8ecc6a3e
5 changed files with 22 additions and 35 deletions
|
@ -133,21 +133,15 @@ EXPORT_SYMBOL_GPL(af_alg_release);
|
||||||
void af_alg_release_parent(struct sock *sk)
|
void af_alg_release_parent(struct sock *sk)
|
||||||
{
|
{
|
||||||
struct alg_sock *ask = alg_sk(sk);
|
struct alg_sock *ask = alg_sk(sk);
|
||||||
unsigned int nokey = ask->nokey_refcnt;
|
unsigned int nokey = atomic_read(&ask->nokey_refcnt);
|
||||||
bool last = nokey && !ask->refcnt;
|
|
||||||
|
|
||||||
sk = ask->parent;
|
sk = ask->parent;
|
||||||
ask = alg_sk(sk);
|
ask = alg_sk(sk);
|
||||||
|
|
||||||
local_bh_disable();
|
if (nokey)
|
||||||
bh_lock_sock(sk);
|
atomic_dec(&ask->nokey_refcnt);
|
||||||
ask->nokey_refcnt -= nokey;
|
|
||||||
if (!last)
|
|
||||||
last = !--ask->refcnt;
|
|
||||||
bh_unlock_sock(sk);
|
|
||||||
local_bh_enable();
|
|
||||||
|
|
||||||
if (last)
|
if (atomic_dec_and_test(&ask->refcnt))
|
||||||
sock_put(sk);
|
sock_put(sk);
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(af_alg_release_parent);
|
EXPORT_SYMBOL_GPL(af_alg_release_parent);
|
||||||
|
@ -192,7 +186,7 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
|
||||||
|
|
||||||
err = -EBUSY;
|
err = -EBUSY;
|
||||||
lock_sock(sk);
|
lock_sock(sk);
|
||||||
if (ask->refcnt | ask->nokey_refcnt)
|
if (atomic_read(&ask->refcnt))
|
||||||
goto unlock;
|
goto unlock;
|
||||||
|
|
||||||
swap(ask->type, type);
|
swap(ask->type, type);
|
||||||
|
@ -241,7 +235,7 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
|
||||||
int err = -EBUSY;
|
int err = -EBUSY;
|
||||||
|
|
||||||
lock_sock(sk);
|
lock_sock(sk);
|
||||||
if (ask->refcnt)
|
if (atomic_read(&ask->refcnt) != atomic_read(&ask->nokey_refcnt))
|
||||||
goto unlock;
|
goto unlock;
|
||||||
|
|
||||||
type = ask->type;
|
type = ask->type;
|
||||||
|
@ -308,12 +302,14 @@ int af_alg_accept(struct sock *sk, struct socket *newsock, bool kern)
|
||||||
|
|
||||||
sk2->sk_family = PF_ALG;
|
sk2->sk_family = PF_ALG;
|
||||||
|
|
||||||
if (nokey || !ask->refcnt++)
|
if (atomic_inc_return_relaxed(&ask->refcnt) == 1)
|
||||||
sock_hold(sk);
|
sock_hold(sk);
|
||||||
ask->nokey_refcnt += nokey;
|
if (nokey) {
|
||||||
|
atomic_inc(&ask->nokey_refcnt);
|
||||||
|
atomic_set(&alg_sk(sk2)->nokey_refcnt, 1);
|
||||||
|
}
|
||||||
alg_sk(sk2)->parent = sk;
|
alg_sk(sk2)->parent = sk;
|
||||||
alg_sk(sk2)->type = type;
|
alg_sk(sk2)->type = type;
|
||||||
alg_sk(sk2)->nokey_refcnt = nokey;
|
|
||||||
|
|
||||||
newsock->ops = type->ops;
|
newsock->ops = type->ops;
|
||||||
newsock->state = SS_CONNECTED;
|
newsock->state = SS_CONNECTED;
|
||||||
|
|
|
@ -388,7 +388,7 @@ static int aead_check_key(struct socket *sock)
|
||||||
struct alg_sock *ask = alg_sk(sk);
|
struct alg_sock *ask = alg_sk(sk);
|
||||||
|
|
||||||
lock_sock(sk);
|
lock_sock(sk);
|
||||||
if (ask->refcnt)
|
if (!atomic_read(&ask->nokey_refcnt))
|
||||||
goto unlock_child;
|
goto unlock_child;
|
||||||
|
|
||||||
psk = ask->parent;
|
psk = ask->parent;
|
||||||
|
@ -400,11 +400,8 @@ static int aead_check_key(struct socket *sock)
|
||||||
if (crypto_aead_get_flags(tfm->aead) & CRYPTO_TFM_NEED_KEY)
|
if (crypto_aead_get_flags(tfm->aead) & CRYPTO_TFM_NEED_KEY)
|
||||||
goto unlock;
|
goto unlock;
|
||||||
|
|
||||||
if (!pask->refcnt++)
|
atomic_dec(&pask->nokey_refcnt);
|
||||||
sock_hold(psk);
|
atomic_set(&ask->nokey_refcnt, 0);
|
||||||
|
|
||||||
ask->refcnt = 1;
|
|
||||||
sock_put(psk);
|
|
||||||
|
|
||||||
err = 0;
|
err = 0;
|
||||||
|
|
||||||
|
|
|
@ -306,7 +306,7 @@ static int hash_check_key(struct socket *sock)
|
||||||
struct alg_sock *ask = alg_sk(sk);
|
struct alg_sock *ask = alg_sk(sk);
|
||||||
|
|
||||||
lock_sock(sk);
|
lock_sock(sk);
|
||||||
if (ask->refcnt)
|
if (!atomic_read(&ask->nokey_refcnt))
|
||||||
goto unlock_child;
|
goto unlock_child;
|
||||||
|
|
||||||
psk = ask->parent;
|
psk = ask->parent;
|
||||||
|
@ -318,11 +318,8 @@ static int hash_check_key(struct socket *sock)
|
||||||
if (crypto_ahash_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
|
if (crypto_ahash_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
|
||||||
goto unlock;
|
goto unlock;
|
||||||
|
|
||||||
if (!pask->refcnt++)
|
atomic_dec(&pask->nokey_refcnt);
|
||||||
sock_hold(psk);
|
atomic_set(&ask->nokey_refcnt, 0);
|
||||||
|
|
||||||
ask->refcnt = 1;
|
|
||||||
sock_put(psk);
|
|
||||||
|
|
||||||
err = 0;
|
err = 0;
|
||||||
|
|
||||||
|
|
|
@ -215,7 +215,7 @@ static int skcipher_check_key(struct socket *sock)
|
||||||
struct alg_sock *ask = alg_sk(sk);
|
struct alg_sock *ask = alg_sk(sk);
|
||||||
|
|
||||||
lock_sock(sk);
|
lock_sock(sk);
|
||||||
if (ask->refcnt)
|
if (!atomic_read(&ask->nokey_refcnt))
|
||||||
goto unlock_child;
|
goto unlock_child;
|
||||||
|
|
||||||
psk = ask->parent;
|
psk = ask->parent;
|
||||||
|
@ -227,11 +227,8 @@ static int skcipher_check_key(struct socket *sock)
|
||||||
if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
|
if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
|
||||||
goto unlock;
|
goto unlock;
|
||||||
|
|
||||||
if (!pask->refcnt++)
|
atomic_dec(&pask->nokey_refcnt);
|
||||||
sock_hold(psk);
|
atomic_set(&ask->nokey_refcnt, 0);
|
||||||
|
|
||||||
ask->refcnt = 1;
|
|
||||||
sock_put(psk);
|
|
||||||
|
|
||||||
err = 0;
|
err = 0;
|
||||||
|
|
||||||
|
|
|
@ -34,8 +34,8 @@ struct alg_sock {
|
||||||
|
|
||||||
struct sock *parent;
|
struct sock *parent;
|
||||||
|
|
||||||
unsigned int refcnt;
|
atomic_t refcnt;
|
||||||
unsigned int nokey_refcnt;
|
atomic_t nokey_refcnt;
|
||||||
|
|
||||||
const struct af_alg_type *type;
|
const struct af_alg_type *type;
|
||||||
void *private;
|
void *private;
|
||||||
|
|
Loading…
Reference in a new issue