Re: [PATCH 4.19 13/36] crypto: af_alg - fix use-after-free in af_alg_accept() due to bh_lock_sock()

Pavel Machek
2020-07-07T21:25:37Z

[-- Attachment #1: Type: text/plain, Size: 973 bytes --]

Hi!

> 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:

> @@ -308,12 +302,14 @@ int af_alg_accept(struct sock *sk, struc
>
> sk2->sk_family = PF_ALG;
>
> - if (nokey || !ask->refcnt++)
> + if (atomic_inc_return_relaxed(&ask->refcnt) == 1)
> sock_hold(sk);
> - ask->nokey_refcnt += nokey;
> + if (nokey) {
> + atomic_inc(&ask->nokey_refcnt);
> + atomic_set(&alg_sk(sk2)->nokey_refcnt, 1);
> + }

Should we set the nokey_refcnt to 0 using atomic_set, too?
Aternatively, should the nokey_refcnt be initialized using
ATOMIC_INIT()?

Best regards,
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]