linux-kernel - Re: crypto: use-after-free in alg

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACT4Y+ZG_e2NBTCG3Qa06-F79JwG0BxRHyMwwPsrZRMp80cqug@mail.gmail.com>
Date:	Wed, 30 Dec 2015 11:58:58 +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 11:53 AM, Herbert Xu
<herbert@...dor.apana.org.au> wrote:
> On Wed, Dec 30, 2015 at 11:19:45AM +0100, Dmitry Vyukov wrote:
>>
>> 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
>
> I've tried a few thousand instances of it but still no luck.
>>
>>
>> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
>> index a8e7aa3..82a7dcd 100644
>
> There are a few missing hunks in your patch and the patch to
> if_alg.h is missing.
>
> So please start with the current crypto tree and then apply the
> latest version (v2) of "crypto: af_alg - Disallow bind/setkey/...
> after accept(2)" and try again.


I forgot to diff include/crypto/if_alg.h, but the changes are there
(otherwise all references to refcnt would not compile). Also I moved
ask->refcnt checks to alg_setsockopt to fix the deadlock, I believe
that's the missing chunks you refer to. I can retest if you wish, but
I don't think that my changes can affect the reported use-after-free.
Do you?


diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 018afb2..589716f 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -30,6 +30,8 @@ struct alg_sock {

        struct sock *parent;

+       unsigned int refcnt;
+
        const struct af_alg_type *type;
        void *private;
 };
@@ -67,6 +69,7 @@ int af_alg_register_type(const struct af_alg_type *type);
 int af_alg_unregister_type(const struct af_alg_type *type);

 int af_alg_release(struct socket *sock);
+void af_alg_release_parent(struct sock *sk);
 int af_alg_accept(struct sock *sk, struct socket *newsock);

 int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, int len);
@@ -83,11 +86,6 @@ static inline struct alg_sock *alg_sk(struct sock *sk)
        return (struct alg_sock *)sk;
 }

-static inline void af_alg_release_parent(struct sock *sk)
-{
-       sock_put(alg_sk(sk)->parent);
-}
-
 static inline void af_alg_init_completion(struct af_alg_completion *completion)
 {
        init_completion(&completion->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/