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
| ||
|
Date: Sun, 18 Sep 2016 07:09:36 +0200 (CEST) From: Julia Lawall <julia.lawall@...6.fr> To: Joe Perches <joe@...ches.com> cc: Dan Carpenter <error27@...il.com>, LKML <linux-kernel@...r.kernel.org> Subject: Re: Possible code defects: macros and precedence On Sat, 17 Sep 2016, Joe Perches wrote: > On Sat, 2016-09-17 at 22:24 +0200, Julia Lawall wrote: > > diff -u -p a/lib/sha1.c b/lib/sha1.c > [] > > @@ -49,18 +49,18 @@ > > * the input data, the next mix it from the 512-bit array. > > */ > > #define SHA_SRC(t) get_unaligned_be32((__u32 *)data + t) > > -#define SHA_MIX(t) rol32(W(t+13) ^ W(t+8) ^ W(t+2) ^ W(t), 1) > > +#define SHA_MIX(t) rol32(W((t)+13) ^ W((t)+8) ^ W((t)+2) ^ W(t), 1) > > > > #define SHA_ROUND(t, input, fn, constant, A, B, C, D, E) do { \ > > __u32 TEMP = input(t); setW(t, TEMP); \ > > E += TEMP + rol32(A,5) + (fn) + (constant); \ > > B = ror32(B, 2); } while (0) > > > > -#define T_0_15(t, A, B, C, D, E) SHA_ROUND(t, SHA_SRC, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E ) > > -#define T_16_19(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E ) > > -#define T_20_39(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0x6ed9eba1, A, B, C, D, E ) > > -#define T_40_59(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B&C)+(D&(B^C))) , 0x8f1bbcdc, A, B, C, D, E ) > > -#define T_60_79(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0xca62c1d6, A, B, C, D, E ) > > +#define T_0_15(t, A, B, C, D, E) SHA_ROUND(t, SHA_SRC, ((((C)^D)&B)^D) , 0x5a827999, A, B, C, D, E ) > > +#define T_16_19(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((((C)^D)&B)^D) , 0x5a827999, A, B, C, D, E ) > > +#define T_20_39(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B)^C^D) , 0x6ed9eba1, A, B, C, D, E ) > > +#define T_40_59(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (((B)&C)+((D)&((B)^C))) , 0x8f1bbcdc, A, B, C, D, E ) > > +#define T_60_79(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B)^C^D) , 0xca62c1d6, A, B, C, D, E ) > > I didn't look at much of the patch. > > It looks as if the coccinelle code doesn't do the > transformation on each possible macro argument. > > In the transform above, only the first referenced arg > is updated with parentheses and subsequent args are not. No, actually it looks like only the left argument is getting transformed. For example, T_40_59 has a term that started as ((B&C)+(D&(B^C))) and ends up as (((B)&C)+((D)&((B)^C))). I can fix this. > Many or most of the uses of these various macros always > pass a single argument to these macros so there isn't a > real possibility of a precedence defect for those uses. > > Is it possible to check the macro users for arguments that > might produce precedence defects and only report those > possible defects? It could be possible. Coccinelle is not always able to associate header files to C files, if the header files are in strange paths, but perhaps one could also rely on the names, since some names may be unique in the kernel. > I also submitted a similar checkpatch addition that looks > for non-comma operators used macro arguments in function > definitions. > > The checkpatch test has the same weakness as the coccinelle > test. It doesn't check uses, just the macro definition. I wonder if it is really a weakness? Does anyone care if a macro definition has more parentheses than what is necessary for the current usage? julia > Commits in -next: > > From 75bd22fe82d1fd698894e4a5d21e33ffdf7d4492 Mon Sep 17 00:00:00 2001 > From: Joe Perches <joe@...ches.com> > Date: Thu, 15 Sep 2016 10:29:22 +1000 > Subject: [PATCH] checkpatch: improve MACRO_ARG_PRECEDENCE test > > It is possible for a multiple line macro definition to have a false positive > report when an argument is used on a line after a continuation \. > > This line might have a leading '+' as the initial character that could be > confused by checkpatch as an operator. > > Avoid the leading character on multiple line macro definitions. > > Link: http://lkml.kernel.org/r/60229d13399f9b6509db5a32e30d4c16951a60cd.1473836073.git.joe@perches.com > Signed-off-by: Joe Perches <joe@...ches.com> > Signed-off-by: Andrew Morton <akpm@...ux-foundation.org> > > From 0158682614df6006752ac932b2e65475384a87b3 Mon Sep 17 00:00:00 2001 > From: Joe Perches <joe@...ches.com> > Date: Thu, 15 Sep 2016 10:29:22 +1000 > Subject: [PATCH] checkpatch: add --strict test for precedence challenged macro arguments > > Add a test for macro arguents that have a non-comma leading or trailing > operator where the argument isn't parenthesized to avoid possible precedence > issues. > > Link: http://lkml.kernel.org/r/47715508972f8d786f435e583ff881dbeee3a114.1473745855.git.joe@perches.com > Signed-off-by: Joe Perches <joe@...ches.com> > Cc: Andy Whitcroft <apw@...onical.com> > Cc: Julia Lawall <julia.lawall@...6.fr> > Cc: Dan Carpenter <dan.carpenter@...cle.com> > Signed-off-by: Andrew Morton <akpm@...ux-foundation.org> > >
Powered by blists - more mailing lists