| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACT4Y+ZUiiXUY06MEGQfz3FN8pugrNpSJb2QjMQ9CyRCP6kojw@mail.gmail.com>
Date: Wed, 30 Dec 2015 11:19:45 +0100
From: Dmitry Vyukov <dvyukov@...gle.com>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: "David S. Miller" <davem@...emloft.net>,
linux-crypto@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
Kostya Serebryany <kcc@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Sasha Levin <sasha.levin@...cle.com>,
Eric Dumazet <edumazet@...gle.com>,
syzkaller <syzkaller@...glegroups.com>
Subject: Re: crypto: use-after-free in alg_bind
On Wed, Dec 30, 2015 at 2:24 AM, Herbert Xu <herbert@...dor.apana.org.au> wrote:
> On Tue, Dec 29, 2015 at 09:19:22PM +0100, Dmitry Vyukov wrote:
>> Hello,
>>
>> On commit 8513342170278468bac126640a5d2d12ffbff106
>> + crypto: algif_skcipher - Use new skcipher interface
>> + crypto: algif_skcipher - Require setkey before accept(2)
>> + crypto: af_alg - Disallow bind/setkey/... after accept(2)
>>
>> The following program causes use-after-free in alg_bind and later
>> terminates kernel:
>
> Please double-check that you have the last patch applied correctly,
> as I cannot reproduce the crash with your program.
I am pretty sure I have the last patch applied. The use-after-frees
that it was supposed to fix have gone. Below are combined changed to
crypto/ files that I have on top of
8513342170278468bac126640a5d2d12ffbff106.
This use-after-free does not reproduce on every run. It seems to be
triggered by some race. Try to run the program in a parallel loop.
I use stress tool for this:
https://github.com/golang/tools/blob/master/cmd/stress/stress.go
If you have Go toolchain installed, then then following will do:
$ go get golang.org/x/tools/cmd/stress
$ stress -p 16 ./a.out
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a8e7aa3..82a7dcd 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -125,6 +125,23 @@ int af_alg_release(struct socket *sock)
}
EXPORT_SYMBOL_GPL(af_alg_release);
+void af_alg_release_parent(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ bool last;
+
+ sk = ask->parent;
+ ask = alg_sk(sk);
+
+ lock_sock(sk);
+ last = !--ask->refcnt;
+ release_sock(sk);
+
+ if (last)
+ sock_put(sk);
+}
+EXPORT_SYMBOL_GPL(af_alg_release_parent);
+
static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
{
const u32 forbidden = CRYPTO_ALG_INTERNAL;
@@ -133,6 +150,7 @@ static int alg_bind(struct socket *sock, struct
sockaddr *uaddr, int addr_len)
struct sockaddr_alg *sa = (void *)uaddr;
const struct af_alg_type *type;
void *private;
+ int err;
if (sock->state == SS_CONNECTED)
return -EINVAL;
@@ -160,16 +178,22 @@ static int alg_bind(struct socket *sock, struct
sockaddr *uaddr, int addr_len)
return PTR_ERR(private);
}
+ err = -EBUSY;
lock_sock(sk);
+ if (ask->refcnt)
+ goto unlock;
swap(ask->type, type);
swap(ask->private, private);
+ err = 0;
+
+unlock:
release_sock(sk);
alg_do_release(type, private);
- return 0;
+ return err;
}
static int alg_setkey(struct sock *sk, char __user *ukey,
@@ -196,6 +220,16 @@ out:
return err;
}
+static int alg_setauthsize(struct sock *sk, unsigned int size)
+{
+ int err;
+ struct alg_sock *ask = alg_sk(sk);
+ const struct af_alg_type *type = ask->type;
+
+ err = type->setauthsize(ask->private, size);
+ return err;
+}
+
static int alg_setsockopt(struct socket *sock, int level, int optname,
char __user *optval, unsigned int optlen)
{
@@ -210,6 +244,11 @@ static int alg_setsockopt(struct socket *sock,
int level, int optname,
if (level != SOL_ALG || !type)
goto unlock;
+ if (ask->refcnt) {
+ err = -EBUSY;
+ goto unlock;
+ }
+
switch (optname) {
case ALG_SET_KEY:
if (sock->state == SS_CONNECTED)
@@ -224,7 +263,7 @@ static int alg_setsockopt(struct socket *sock, int
level, int optname,
goto unlock;
if (!type->setauthsize)
goto unlock;
- err = type->setauthsize(ask->private, optlen);
+ err = alg_setauthsize(sk, optlen);
}
unlock:
@@ -264,7 +303,8 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
sk2->sk_family = PF_ALG;
- sock_hold(sk);
+ if (!ask->refcnt++)
+ sock_hold(sk);
alg_sk(sk2)->parent = sk;
alg_sk(sk2)->type = type;
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 634b4d1..df483f9 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -31,6 +31,11 @@ struct skcipher_sg_list {
struct scatterlist sg[0];
};
+struct skcipher_tfm {
+ struct crypto_skcipher *skcipher;
+ bool has_key;
+};
+
struct skcipher_ctx {
struct list_head tsgl;
struct af_alg_sgl rsgl;
@@ -750,17 +755,41 @@ static struct proto_ops algif_skcipher_ops = {
static void *skcipher_bind(const char *name, u32 type, u32 mask)
{
- return crypto_alloc_skcipher(name, type, mask);
+ struct skcipher_tfm *tfm;
+ struct crypto_skcipher *skcipher;
+
+ tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
+ if (!tfm)
+ return ERR_PTR(-ENOMEM);
+
+ skcipher = crypto_alloc_skcipher(name, type, mask);
+ if (IS_ERR(skcipher)) {
+ kfree(tfm);
+ return ERR_CAST(skcipher);
+ }
+
+ tfm->skcipher = skcipher;
+
+ return tfm;
}
static void skcipher_release(void *private)
{
- crypto_free_skcipher(private);
+ struct skcipher_tfm *tfm = private;
+
+ crypto_free_skcipher(tfm->skcipher);
+ kfree(tfm);
}
static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
{
- return crypto_skcipher_setkey(private, key, keylen);
+ struct skcipher_tfm *tfm = private;
+ int err;
+
+ err = crypto_skcipher_setkey(tfm->skcipher, key, keylen);
+ tfm->has_key = !err;
+
+ return err;
}
static void skcipher_wait(struct sock *sk)
@@ -792,20 +821,25 @@ static int skcipher_accept_parent(void *private,
struct sock *sk)
{
struct skcipher_ctx *ctx;
struct alg_sock *ask = alg_sk(sk);
- unsigned int len = sizeof(*ctx) + crypto_skcipher_reqsize(private);
+ struct skcipher_tfm *tfm = private;
+ struct crypto_skcipher *skcipher = tfm->skcipher;
+ unsigned int len = sizeof(*ctx) + crypto_skcipher_reqsize(skcipher);
+
+ if (!tfm->has_key)
+ return -ENOKEY;
ctx = sock_kmalloc(sk, len, GFP_KERNEL);
if (!ctx)
return -ENOMEM;
- ctx->iv = sock_kmalloc(sk, crypto_skcipher_ivsize(private),
+ ctx->iv = sock_kmalloc(sk, crypto_skcipher_ivsize(skcipher),
GFP_KERNEL);
if (!ctx->iv) {
sock_kfree_s(sk, ctx, len);
return -ENOMEM;
}
- memset(ctx->iv, 0, crypto_skcipher_ivsize(private));
+ memset(ctx->iv, 0, crypto_skcipher_ivsize(skcipher));
INIT_LIST_HEAD(&ctx->tsgl);
ctx->len = len;
@@ -818,7 +852,7 @@ static int skcipher_accept_parent(void *private,
struct sock *sk)
ask->private = ctx;
- skcipher_request_set_tfm(&ctx->req, private);
+ skcipher_request_set_tfm(&ctx->req, skcipher);
skcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
af_alg_complete, &ctx->completion);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists