| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c50014e57e77842e4c710231dff829f3f67bbe59.camel@perches.com>
Date: Wed, 09 Sep 2020 17:20:42 -0700
From: Joe Perches <joe@...ches.com>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: Keith Busch <kbusch@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
linux-crypto@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
Jiri Kosina <trivial@...nel.org>,
Kees Cook <kees.cook@...onical.com>,
Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: [trivial PATCH] treewide: Convert switch/case fallthrough; to
break;
On Thu, 2020-09-10 at 08:35 +1000, Herbert Xu wrote:
> On Wed, Sep 09, 2020 at 02:09:32PM -0700, Joe Perches wrote:
> > On Wed, 2020-09-09 at 13:55 -0700, Keith Busch wrote:
> > > On Wed, Sep 09, 2020 at 01:06:39PM -0700, Joe Perches wrote:
> > > > diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
> > > > index eea0f453cfb6..8aac5bc60f4c 100644
> > > > --- a/crypto/tcrypt.c
> > > > +++ b/crypto/tcrypt.c
> > > > @@ -2464,7 +2464,7 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
> > > > test_hash_speed("streebog512", sec,
> > > > generic_hash_speed_template);
> > > > if (mode > 300 && mode < 400) break;
> > > > - fallthrough;
> > > > + break;
> > > > case 399:
> > > > break;
> > >
> > > Just imho, this change makes the preceding 'if' look even more
> > > pointless. Maybe the fallthrough was a deliberate choice? Not that my
> > > opinion matters here as I don't know this module, but it looked a bit
> > > odd to me.
> >
> > It does look odd to me too.
> >
> > It's also just a test module though so the
> > code isn't particularly crucial.
> >
> > Herbert/David? thoughts?
>
> Please read the function as a whole, that fallthrough (and every other
> one in do_test) needs to stay.
Hi Herbert.
OK, I did. I think it's odd code.
The do_test function could start at any 3xx or 4xx mode
and then do all the next tests to the highest 99
for ranges from 301-399 and 401-499.
ie: Pass mode 312, it'll execute test 312 and every
other test to 399.
Not the same for the 500-599 or 600-699 range.
I probably would've hoisted the code for all of the 0,
any value from 300-399, and any value from 400-499
out of the do_test function and into tcrypt_mod_init
not used fallthrough in do_test at all.
cheers, Joe
Perhaps something like:
---
crypto/tcrypt.c | 190 +++++++++++++++++++++-----------------------------------
1 file changed, 72 insertions(+), 118 deletions(-)
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index eea0f453cfb6..aeb089f6ee27 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1660,7 +1660,6 @@ static inline int tcrypt_test(const char *alg)
static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
{
- int i;
int ret = 0;
switch (m) {
@@ -1672,8 +1671,6 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
break;
}
- for (i = 1; i < 200; i++)
- ret += do_test(NULL, 0, 0, i, num_mb);
break;
case 1:
@@ -2349,123 +2346,93 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
test_hash_speed(alg, sec, generic_hash_speed_template);
break;
}
- fallthrough;
+ break;
case 301:
test_hash_speed("md4", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 302:
test_hash_speed("md5", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 303:
test_hash_speed("sha1", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 304:
test_hash_speed("sha256", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 305:
test_hash_speed("sha384", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 306:
test_hash_speed("sha512", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 307:
test_hash_speed("wp256", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 308:
test_hash_speed("wp384", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 309:
test_hash_speed("wp512", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 310:
test_hash_speed("tgr128", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 311:
test_hash_speed("tgr160", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 312:
test_hash_speed("tgr192", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 313:
test_hash_speed("sha224", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 314:
test_hash_speed("rmd128", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 315:
test_hash_speed("rmd160", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 316:
test_hash_speed("rmd256", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 317:
test_hash_speed("rmd320", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 318:
klen = 16;
test_hash_speed("ghash", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 319:
test_hash_speed("crc32c", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 320:
test_hash_speed("crct10dif", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 321:
test_hash_speed("poly1305", sec, poly1305_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 322:
test_hash_speed("sha3-224", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 323:
test_hash_speed("sha3-256", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 324:
test_hash_speed("sha3-384", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 325:
test_hash_speed("sha3-512", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 326:
test_hash_speed("sm3", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 327:
test_hash_speed("streebog256", sec,
generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 328:
test_hash_speed("streebog512", sec,
generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
- case 399:
break;
case 400:
@@ -2473,122 +2440,93 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
test_ahash_speed(alg, sec, generic_hash_speed_template);
break;
}
- fallthrough;
+ break;
case 401:
test_ahash_speed("md4", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 402:
test_ahash_speed("md5", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 403:
test_ahash_speed("sha1", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 404:
test_ahash_speed("sha256", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 405:
test_ahash_speed("sha384", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 406:
test_ahash_speed("sha512", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 407:
test_ahash_speed("wp256", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 408:
test_ahash_speed("wp384", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 409:
test_ahash_speed("wp512", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 410:
test_ahash_speed("tgr128", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 411:
test_ahash_speed("tgr160", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 412:
test_ahash_speed("tgr192", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 413:
test_ahash_speed("sha224", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 414:
test_ahash_speed("rmd128", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 415:
test_ahash_speed("rmd160", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 416:
test_ahash_speed("rmd256", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 417:
test_ahash_speed("rmd320", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 418:
test_ahash_speed("sha3-224", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 419:
test_ahash_speed("sha3-256", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 420:
test_ahash_speed("sha3-384", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 421:
test_ahash_speed("sha3-512", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 422:
test_mb_ahash_speed("sha1", sec, generic_hash_speed_template,
num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 423:
test_mb_ahash_speed("sha256", sec, generic_hash_speed_template,
num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 424:
test_mb_ahash_speed("sha512", sec, generic_hash_speed_template,
num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 425:
test_mb_ahash_speed("sm3", sec, generic_hash_speed_template,
num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 426:
test_mb_ahash_speed("streebog256", sec,
generic_hash_speed_template, num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 427:
test_mb_ahash_speed("streebog512", sec,
generic_hash_speed_template, num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
- case 499:
break;
case 500:
@@ -3034,7 +2972,23 @@ static int __init tcrypt_mod_init(void)
goto err_free_tv;
}
- err = do_test(alg, type, mask, mode, num_mb);
+ if (mode == 0) {
+ err = do_test(alg, type, mask, mode, num_mb);
+ if (!err) {
+ for (i = 1; i < 200; i++)
+ err |= do_test(NULL, 0, 0, i, num_mb);
+ }
+ } else if (mode >= 300 && mode < 400) {
+ err = 0;
+ for (i = mode; i < 400; i++)
+ err |= do_test(alg, type, mask, i, num_mb);
+ } else if (mode >= 400 && mode < 500) {
+ err = 0;
+ for (i = mode; i < 500; i++)
+ err |= do_test(alg, type, mask, i, num_mb);
+ } else {
+ err = do_test(alg, type, mask, mode, num_mb);
+ }
if (err) {
printk(KERN_ERR "tcrypt: one or more tests failed!\n");
Powered by blists - more mailing lists