RE: [PATCH] crypto: testmgr - Improve randomization of params for AEAD fuzz testing

[Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>]

Hi Eric,

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of
> Pascal Van Leeuwen
> Sent: Monday, July 29, 2019 11:11 AM
> To: Eric Biggers <ebiggers@kernel.org>; Pascal van Leeuwen <pascalvanl@gmail.com>
> Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au; davem@davemloft.net
> Subject: RE: [PATCH] crypto: testmgr - Improve randomization of params for AEAD fuzz
> testing
> 
> Hi Eric,
> 
> Thanks for your feedback!
> 
> > -----Original Message-----
> > From: Eric Biggers <ebiggers@kernel.org>
> > Sent: Sunday, July 28, 2019 7:31 PM
> > To: Pascal van Leeuwen <pascalvanl@gmail.com>
> > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au; davem@davemloft.net;
> Pascal Van Leeuwen
> > <pvanleeuwen@verimatrix.com>
> > Subject: Re: [PATCH] crypto: testmgr - Improve randomization of params for AEAD fuzz
> testing
> > >
> > > +struct len_range_set {
> > > +	const struct len_range_sel *lensel;
> > > +	unsigned int count;
> > > +};
> > > +
> > >  struct aead_test_suite {
> > >  	const struct aead_testvec *vecs;
> > >  	unsigned int count;
> > >  };
> > >
> > > +struct aead_test_params {
> > > +	struct len_range_set ckeylensel;
> > > +	struct len_range_set akeylensel;
> > > +	struct len_range_set authsizesel;
> > > +	struct len_range_set aadlensel;
> > > +	struct len_range_set ptxtlensel;
> > > +};
> > > +
> > >  struct cipher_test_suite {
> > >  	const struct cipher_testvec *vecs;
> > >  	unsigned int count;
> > > @@ -143,6 +156,10 @@ struct alg_test_desc {
> > >  		struct akcipher_test_suite akcipher;
> > >  		struct kpp_test_suite kpp;
> > >  	} suite;
> > > +
> > > +	union {
> > > +		struct aead_test_params aead;
> > > +	} params;
> > >  };
> >
> > Why not put these new fields in the existing 'struct aead_test_suite'?
> >
> > I don't see the point of the separate 'params' struct.  It just confuses things.
> >
> Mostly because I'm not that familiar with C datastructures (I'm not a programmer
> and this is pretty much my first serious experience with C), so I didn't know how
> to do that / didn't want to break anything else :-)
> 
> So if you can provide some example on how to do that ...
> 
Actually, while looking into some way to combine these fields into 
'struct aead_test_suite':  I really can't think of a way to do that that
would be as convenient as the current approach which allows me to:

- NOT have these params for the other types (cipher, comp, hash etc.), at
  least for now
- NOT have to touch any declarations in the alg_test_desc assignment that
  do not need this
- conveniently use a macro line __LENS (idea shamelessly borrowed from
  __VECS) to assign the struct ptr / list length fields pairs

If you know of a better way to achieve all that, then feel free to teach
me. But, frankly I do not see why having 1 entry defining the testsuite 
and  a seperate entry defining the fuzz test parameters would necessarily
be confusing? Apart from 'params' perhaps not being a really good name, 
being too generic and all, 'fuzz_params' would probably be better?

> > - Eric
> 
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> www.insidesecure.com

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com