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

[Pavel Machek]

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: signature.asc
Description: Digital signature