Re: [PATCH] crypto: algif_aead - Require setkey before accept(2)

["Stephan Müller" <smueller@chronox.de>]

Am Freitag, 21. April 2017, 13:11:27 CEST schrieb Herbert Xu:

Hi Herbert,

> On Mon, Apr 10, 2017 at 01:59:21PM +0200, Stephan Müller wrote:
> > @@ -757,12 +887,14 @@ static void aead_sock_destruct(struct sock *sk)
> > 
> >  	af_alg_release_parent(sk);
> >  
> >  }
> > 
> > -static int aead_accept_parent(void *private, struct sock *sk)
> > +static int aead_accept_parent_nokey(void *private, struct sock *sk)
> > 
> >  {
> >  
> >  	struct aead_ctx *ctx;
> >  	struct alg_sock *ask = alg_sk(sk);
> > 
> > -	unsigned int len = sizeof(*ctx) + crypto_aead_reqsize(private);
> > -	unsigned int ivlen = crypto_aead_ivsize(private);
> > +	struct aead_tfm *tfm = private;
> > +	struct crypto_aead *aead = tfm->aead;
> > +	unsigned int len = sizeof(*ctx) + crypto_aead_reqsize(aead);
> > +	unsigned int ivlen = crypto_aead_ivsize(aead);
> > 
> >  	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
> >  	if (!ctx)
> > 
> > @@ -789,7 +921,7 @@ static int aead_accept_parent(void *private, struct
> > sock *sk)> 
> >  	ask->private = ctx;
> > 
> > -	aead_request_set_tfm(&ctx->aead_req, private);
> > +	aead_request_set_tfm(&ctx->aead_req, aead);
> > 
> >  	aead_request_set_callback(&ctx->aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> >  	
> >  				  af_alg_complete, &ctx->completion);
> 
> Please don't mix unrelated cleanups like this with the real change.
>  It makes reviewing harder than necessary.
> 

After checking again, IMHO that is no unreleated cleanup or even a cleanup at 
all.

void *private used to be struct crypto_aead and is now struct aead_tfm. struct 
crypto_aead is found in private->aead. Hence, the patch assigned private to 
tfm and then obtained the struct crypto_aead pointer. As this was not 
necessary before, it is a required extension IMHO.

Ciao
Stephan