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+Y2VW2QP7NmzjmDg-wjfhB=q-6CUJ6=CeEEt9yxDYomXw@mail.gmail.com>
Date:	Wed, 30 Dec 2015 13:45:15 +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 1:24 PM, Herbert Xu <herbert@...dor.apana.org.au> wrote:
> On Wed, Dec 30, 2015 at 11:58:58AM +0100, Dmitry Vyukov wrote:
>>
>> 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?
>
> OK I see the problem now.  When accept fails we free the socket
> twice.

Great!

This seems to be a zero-day. Should we CC stable@...r.kernel.org ?

Code in 03c8efc1ffeb6b82a22c1af8dd908af349563314 (Oct 19, 2010) contained:

+       sock_init_data(newsock, sk2);
+
+       err = type->accept(ask->private, sk2);
+       if (err) {
+               sk_free(sk2);
+               goto unlock;
+       }

There were no sock_graft call, but sock_init_data also sets
newsock->sk = sk2, which seems to be enough for the double-free.




> ---8<---
> Subject: crypto: af_alg - Fix socket double-free when accept fails
>
> When we fail an accept(2) call we will end up freeing the socket
> twice, once due to the direct sk_free call and once again through
> newsock.
>
> This patch fixes this by removing the sk_free call.
>
> Cc: stable@...r.kernel.org
> Reported-by: Dmitry Vyukov <dvyukov@...gle.com>
> Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
>
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index 7b5b592..eaf98e2 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -285,10 +285,8 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
>         security_sk_clone(sk, sk2);
>
>         err = type->accept(ask->private, sk2);
> -       if (err) {
> -               sk_free(sk2);
> +       if (err)
>                 goto unlock;
> -       }
>
>         sk2->sk_family = PF_ALG;
>
> --
> Email: Herbert Xu <herbert@...dor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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/