> -----Original Message----- > From: Eric Biggers <ebigg...@kernel.org> > Sent: Tuesday, July 30, 2019 2:17 AM > To: Pascal Van Leeuwen <pvanleeu...@verimatrix.com> > Cc: Pascal van Leeuwen <pascalv...@gmail.com>; linux-crypto@vger.kernel.org; > herb...@gondor.apana.org.au; da...@davemloft.net > Subject: Re: [PATCH] crypto: testmgr - Improve randomization of params for > AEAD fuzz testing > > On Mon, Jul 29, 2019 at 10:16:48PM +0000, Pascal Van Leeuwen wrote: > > > > > Note that the "empty test suite" message shouldn't be printed > > > > > (especially not at > > > > > KERN_ERR level!) if it's working as intended. > > > > > > > > > That's not my code, that was already there. I already got these > > > > messages before my > > > > modifications, for some ciphersuites. Of course if we don't want that, > > > > we can make > > > > it a pr_warn pr_dbg? > > > > > > I didn't get these error messages before this patch. They start showing > > > up > > > because this patch changes alg_test_null to alg_test_aead for algorithms > > > with no > > > test vectors. > > > > > Ok, I guess I caused it for some additional ciphersuites by forcing them > > to be at least fuzz tested. But there were some ciphersuites without test > > vectors already reporting this in my situation because they did not point > > to alg_test_null in the first place. > > Are you sure? I don't see anything that had no test vectors but didn't use > alg_test_null. > > > So it wasn't entirely clear what the > > whole intention was in the first place, as it wasn't consistent. > > If we all agree on the logging level we want for this message, then I can > > make that change. > > I suggest at least downgrading it to KERN_INFO, since that's the level used > for > logging that there wasn't any test description found at all: > > printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver); > Ah ... I think I may have confused those 2 error messages that mean pretty much the same thing to me ... no test ... empty testsuite. If the testsuite is empty, there is no test. So KERN_INFO appears to be what we want here.
> > > > > > > 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 ... > > > > > > I'm simply suggesting adding the fields of 'struct aead_test_params' to > > > 'struct aead_test_suite'. > > > > > My next mail tried to explain why that's not so simple ... > > The only actual issue is that you can't reuse the __VECS() macro because it > adds > an extra level of braces, right? > Yes, not being able to use __LENS within __VECS would be the main problem. And I'm not familiar enough with the C preprocessor to solve that myself. > > Actually, the patch *should* (didn't try yet) make it work for both: if both > > alen and clen are valid (>=0) then it creates a key blob from those ranges. > > If only clen is valid (>=0) but a alen is not (i.e., -1), then it will just > > generate a random key the "normal" way with length clen. > > So, for authenc you define both ranges, for other AEAD you define only a > > cipher key length range with the auth key range count at 0. > > > > Okay, I guess that makes sense. It wasn't obvious to me though. > > - Eric Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com
