* linux-next: build warnings after merge of the crypto tree @ 2014-08-26 6:14 Stephen Rothwell 2014-08-26 6:38 ` Herbert Xu 0 siblings, 1 reply; 24+ messages in thread From: Stephen Rothwell @ 2014-08-26 6:14 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-next, linux-kernel, Stephan Mueller [-- Attachment #1: Type: text/plain, Size: 718 bytes --] Hi Herbert, After merging the crypto tree, today's linux-next build (powerpc ppc44x_defconfig, i386 defconfig and sparc defconfig) produced these warnings: In file included from crypto/testmgr.c:30:0: include/crypto/drbg.h: In function 'drbg_max_addtl': include/crypto/drbg.h:157:2: warning: left shift count >= width of type return (1UL<<35); ^ include/crypto/drbg.h: In function 'drbg_max_requests': include/crypto/drbg.h:163:2: warning: left shift count >= width of type return (1UL<<48); ^ Introduced by commit 05c81ccd9087 ("crypto: drbg - remove configuration of fixed values"). These are all 32 bit builds. -- Cheers, Stephen Rothwell sfr@canb.auug.org.au [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: linux-next: build warnings after merge of the crypto tree 2014-08-26 6:14 linux-next: build warnings after merge of the crypto tree Stephen Rothwell @ 2014-08-26 6:38 ` Herbert Xu 2014-08-26 7:00 ` Stephan Mueller 2014-08-26 7:31 ` [PATCH] DRBG: fix bit shifting on 32 bit systems Stephan Mueller 0 siblings, 2 replies; 24+ messages in thread From: Herbert Xu @ 2014-08-26 6:38 UTC (permalink / raw) To: Stephen Rothwell; +Cc: linux-next, linux-kernel, Stephan Mueller On Tue, Aug 26, 2014 at 04:14:56PM +1000, Stephen Rothwell wrote: > Hi Herbert, > > After merging the crypto tree, today's linux-next build (powerpc > ppc44x_defconfig, i386 defconfig and sparc defconfig) produced these > warnings: > > In file included from crypto/testmgr.c:30:0: > include/crypto/drbg.h: In function 'drbg_max_addtl': > include/crypto/drbg.h:157:2: warning: left shift count >= width of type > return (1UL<<35); > ^ > include/crypto/drbg.h: In function 'drbg_max_requests': > include/crypto/drbg.h:163:2: warning: left shift count >= width of type > return (1UL<<48); > ^ > > Introduced by commit 05c81ccd9087 ("crypto: drbg - remove configuration > of fixed values"). These are all 32 bit builds. Stephan, could you take a look at this? Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: linux-next: build warnings after merge of the crypto tree 2014-08-26 6:38 ` Herbert Xu @ 2014-08-26 7:00 ` Stephan Mueller 2014-08-26 7:31 ` [PATCH] DRBG: fix bit shifting on 32 bit systems Stephan Mueller 1 sibling, 0 replies; 24+ messages in thread From: Stephan Mueller @ 2014-08-26 7:00 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel Am Dienstag, 26. August 2014, 14:38:12 schrieb Herbert Xu: Hi Herbert, >On Tue, Aug 26, 2014 at 04:14:56PM +1000, Stephen Rothwell wrote: >> Hi Herbert, >> >> After merging the crypto tree, today's linux-next build (powerpc >> ppc44x_defconfig, i386 defconfig and sparc defconfig) produced these >> warnings: >> >> In file included from crypto/testmgr.c:30:0: >> include/crypto/drbg.h: In function 'drbg_max_addtl': >> include/crypto/drbg.h:157:2: warning: left shift count >= width of >> type> >> return (1UL<<35); >> ^ >> >> include/crypto/drbg.h: In function 'drbg_max_requests': >> include/crypto/drbg.h:163:2: warning: left shift count >= width of >> type> >> return (1UL<<48); >> ^ >> >> Introduced by commit 05c81ccd9087 ("crypto: drbg - remove >> configuration of fixed values"). These are all 32 bit builds. > >Stephan, could you take a look at this? I am on it. Thanks. > >Thanks, Ciao Stephan ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] DRBG: fix bit shifting on 32 bit systems 2014-08-26 6:38 ` Herbert Xu 2014-08-26 7:00 ` Stephan Mueller @ 2014-08-26 7:31 ` Stephan Mueller 2014-08-26 7:32 ` Herbert Xu 1 sibling, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-08-26 7:31 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel Use ULL to mark a 64 bit value. Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Reported-by: kbuild test robot <fengguang.wu@intel.com> Signed-off-by: Stephan Mueller <smueller@chronox.de> --- include/crypto/drbg.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h index 3d8e73a..dd52aee 100644 --- a/include/crypto/drbg.h +++ b/include/crypto/drbg.h @@ -154,13 +154,13 @@ static inline size_t drbg_max_request_bytes(struct drbg_state *drbg) static inline size_t drbg_max_addtl(struct drbg_state *drbg) { /* SP800-90A requires 2**35 bytes additional info str / pers str */ - return (1UL<<35); + return (1ULL<<35); } static inline size_t drbg_max_requests(struct drbg_state *drbg) { /* SP800-90A requires 2**48 maximum requests before reseeding */ - return (1UL<<48); + return (1ULL<<48); } /* -- 1.9.3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] DRBG: fix bit shifting on 32 bit systems 2014-08-26 7:31 ` [PATCH] DRBG: fix bit shifting on 32 bit systems Stephan Mueller @ 2014-08-26 7:32 ` Herbert Xu 2014-08-26 7:37 ` Stephan Mueller 2014-08-26 8:06 ` [PATCH] DRBG: fix maximum value checks " Stephan Mueller 0 siblings, 2 replies; 24+ messages in thread From: Herbert Xu @ 2014-08-26 7:32 UTC (permalink / raw) To: Stephan Mueller; +Cc: Stephen Rothwell, linux-next, linux-kernel On Tue, Aug 26, 2014 at 09:31:02AM +0200, Stephan Mueller wrote: > Use ULL to mark a 64 bit value. > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Reported-by: kbuild test robot <fengguang.wu@intel.com> > Signed-off-by: Stephan Mueller <smueller@chronox.de> I don't think this works as size_t is still 32-bit which means that you'll return zero. Cheers, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] DRBG: fix bit shifting on 32 bit systems 2014-08-26 7:32 ` Herbert Xu @ 2014-08-26 7:37 ` Stephan Mueller 2014-08-26 8:06 ` [PATCH] DRBG: fix maximum value checks " Stephan Mueller 1 sibling, 0 replies; 24+ messages in thread From: Stephan Mueller @ 2014-08-26 7:37 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel Am Dienstag, 26. August 2014, 15:32:19 schrieb Herbert Xu: Hi Herbert, > On Tue, Aug 26, 2014 at 09:31:02AM +0200, Stephan Mueller wrote: > > Use ULL to mark a 64 bit value. > > > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > > Reported-by: kbuild test robot <fengguang.wu@intel.com> > > Signed-off-by: Stephan Mueller <smueller@chronox.de> > > I don't think this works as size_t is still 32-bit which means > that you'll return zero. Oops, yes. Thank you for catching this. I guess I need __u64 for this value. A patch will come shortly. > > Cheers, -- Ciao Stephan ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] DRBG: fix maximum value checks on 32 bit systems 2014-08-26 7:32 ` Herbert Xu 2014-08-26 7:37 ` Stephan Mueller @ 2014-08-26 8:06 ` Stephan Mueller 2014-08-26 8:08 ` Herbert Xu 1 sibling, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-08-26 8:06 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel The maximum values for additional input string or generated blocks is larger than 1<<32. To ensure a sensible value on 32 bit systems, return SIZE_MAX on 32 bit systems. This value is lower than the maximum allowed values defined in SP800-90A. The standard allow lower maximum values, but not larger values. Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Reported-by: kbuild test robot <fengguang.wu@intel.com> Signed-off-by: Stephan Mueller <smueller@chronox.de> --- include/crypto/drbg.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h index 3d8e73a..5ac482a 100644 --- a/include/crypto/drbg.h +++ b/include/crypto/drbg.h @@ -154,13 +154,21 @@ static inline size_t drbg_max_request_bytes(struct drbg_state *drbg) static inline size_t drbg_max_addtl(struct drbg_state *drbg) { /* SP800-90A requires 2**35 bytes additional info str / pers str */ +#if (__BITS_PER_LONG == 32) + return SIZE_MAX; +#else return (1UL<<35); +#endif } static inline size_t drbg_max_requests(struct drbg_state *drbg) { /* SP800-90A requires 2**48 maximum requests before reseeding */ +#if (__BITS_PER_LONG == 32) + return SIZE_MAX; +#else return (1UL<<48); +#endif } /* -- 1.9.3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] DRBG: fix maximum value checks on 32 bit systems 2014-08-26 8:06 ` [PATCH] DRBG: fix maximum value checks " Stephan Mueller @ 2014-08-26 8:08 ` Herbert Xu 2014-08-26 8:29 ` [PATCH v2] " Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Herbert Xu @ 2014-08-26 8:08 UTC (permalink / raw) To: Stephan Mueller; +Cc: Stephen Rothwell, linux-next, linux-kernel On Tue, Aug 26, 2014 at 10:06:52AM +0200, Stephan Mueller wrote: > The maximum values for additional input string or generated blocks is > larger than 1<<32. To ensure a sensible value on 32 bit systems, return > SIZE_MAX on 32 bit systems. This value is lower than the maximum allowed > values defined in SP800-90A. The standard allow lower maximum values, > but not larger values. > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Reported-by: kbuild test robot <fengguang.wu@intel.com> > Signed-off-by: Stephan Mueller <smueller@chronox.de> Unfortunately there's a place in drbg where you add one to drbg_max_addtl which will cause it to overflow back to zero. Cheers, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2] DRBG: fix maximum value checks on 32 bit systems 2014-08-26 8:08 ` Herbert Xu @ 2014-08-26 8:29 ` Stephan Mueller 2014-08-26 8:43 ` Herbert Xu 0 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-08-26 8:29 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel The maximum values for additional input string or generated blocks is larger than 1<<32. To ensure a sensible value on 32 bit systems, return SIZE_MAX on 32 bit systems. This value is lower than the maximum allowed values defined in SP800-90A. The standard allow lower maximum values, but not larger values. SIZE_MAX - 1 is used for drbg_max_addtl to allow drbg_healthcheck_sanity to check the enforcement of the variable without wrapping. Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Reported-by: kbuild test robot <fengguang.wu@intel.com> Signed-off-by: Stephan Mueller <smueller@chronox.de> --- include/crypto/drbg.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h index 3d8e73a..5186f75 100644 --- a/include/crypto/drbg.h +++ b/include/crypto/drbg.h @@ -154,13 +154,26 @@ static inline size_t drbg_max_request_bytes(struct drbg_state *drbg) static inline size_t drbg_max_addtl(struct drbg_state *drbg) { /* SP800-90A requires 2**35 bytes additional info str / pers str */ +#if (__BITS_PER_LONG == 32) + /* + * SP800-90A allows smaller maximum numbers to be returned -- we + * return SIZE_MAX - 1 to allow the verification of the enforcement + * of this value in drbg_healthcheck_sanity. + */ + return (SIZE_MAX - 1); +#else return (1UL<<35); +#endif } static inline size_t drbg_max_requests(struct drbg_state *drbg) { /* SP800-90A requires 2**48 maximum requests before reseeding */ +#if (__BITS_PER_LONG == 32) + return SIZE_MAX; +#else return (1UL<<48); +#endif } /* -- 1.9.3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: fix maximum value checks on 32 bit systems 2014-08-26 8:29 ` [PATCH v2] " Stephan Mueller @ 2014-08-26 8:43 ` Herbert Xu 2014-08-26 8:52 ` Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Herbert Xu @ 2014-08-26 8:43 UTC (permalink / raw) To: Stephan Mueller; +Cc: Stephen Rothwell, linux-next, linux-kernel On Tue, Aug 26, 2014 at 10:29:45AM +0200, Stephan Mueller wrote: > The maximum values for additional input string or generated blocks is > larger than 1<<32. To ensure a sensible value on 32 bit systems, return > SIZE_MAX on 32 bit systems. This value is lower than the maximum > allowed values defined in SP800-90A. The standard allow lower maximum > values, but not larger values. > > SIZE_MAX - 1 is used for drbg_max_addtl to allow > drbg_healthcheck_sanity to check the enforcement of the variable > without wrapping. This is really ugly but OK. However, I'm not sure how the sanity check ever worked. It would appear that the drbg_generate call in drbg_healthcheck_sanity should always fail because you explicitly set addtl->len to drbg_max_addtl + 1, which should trigger the "DRBG: additional information string too long" error, no? Obviously it's working for you but I'd like to understand why it's working and whether it'll continue to work. Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: fix maximum value checks on 32 bit systems 2014-08-26 8:43 ` Herbert Xu @ 2014-08-26 8:52 ` Stephan Mueller 2014-08-26 8:58 ` Herbert Xu 0 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-08-26 8:52 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel Am Dienstag, 26. August 2014, 16:43:43 schrieb Herbert Xu: Hi Herbert, > On Tue, Aug 26, 2014 at 10:29:45AM +0200, Stephan Mueller wrote: > > The maximum values for additional input string or generated blocks is > > larger than 1<<32. To ensure a sensible value on 32 bit systems, return > > SIZE_MAX on 32 bit systems. This value is lower than the maximum > > allowed values defined in SP800-90A. The standard allow lower maximum > > values, but not larger values. > > > > SIZE_MAX - 1 is used for drbg_max_addtl to allow > > drbg_healthcheck_sanity to check the enforcement of the variable > > without wrapping. > > This is really ugly but OK. However, I'm not sure how the sanity > check ever worked. It would appear that the drbg_generate call in > drbg_healthcheck_sanity should always fail because you explicitly > set addtl->len to drbg_max_addtl + 1, which should trigger the > "DRBG: additional information string too long" error, no? That is exactly what the test shall do: the test is intended to check whether the maximum values are enforced. And it does that by checking whether an error is returned. /* get the maximum value */ max_addtllen = drbg_max_addtl(drbg); /* add one to definitely overflow the maximum value */ drbg_string_fill(&addtl, buf, max_addtllen + 1); /* overflow addtllen with additonal info string */ len = drbg_generate(drbg, buf, OUTBUFLEN, &addtl); /* * check that the drbg_generate does not return a positive * value, i.e. check that drbg_generate does not generate anything */ BUG_ON(0 < len); > > Obviously it's working for you but I'd like to understand why > it's working and whether it'll continue to work. > > Thanks, -- Ciao Stephan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: fix maximum value checks on 32 bit systems 2014-08-26 8:52 ` Stephan Mueller @ 2014-08-26 8:58 ` Herbert Xu 2014-08-26 9:36 ` Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Herbert Xu @ 2014-08-26 8:58 UTC (permalink / raw) To: Stephan Mueller; +Cc: Stephen Rothwell, linux-next, linux-kernel On Tue, Aug 26, 2014 at 10:52:45AM +0200, Stephan Mueller wrote: > Am Dienstag, 26. August 2014, 16:43:43 schrieb Herbert Xu: > > Hi Herbert, > > > On Tue, Aug 26, 2014 at 10:29:45AM +0200, Stephan Mueller wrote: > > > The maximum values for additional input string or generated blocks is > > > larger than 1<<32. To ensure a sensible value on 32 bit systems, return > > > SIZE_MAX on 32 bit systems. This value is lower than the maximum > > > allowed values defined in SP800-90A. The standard allow lower maximum > > > values, but not larger values. > > > > > > SIZE_MAX - 1 is used for drbg_max_addtl to allow > > > drbg_healthcheck_sanity to check the enforcement of the variable > > > without wrapping. > > > > This is really ugly but OK. However, I'm not sure how the sanity > > check ever worked. It would appear that the drbg_generate call in > > drbg_healthcheck_sanity should always fail because you explicitly > > set addtl->len to drbg_max_addtl + 1, which should trigger the > > "DRBG: additional information string too long" error, no? > > That is exactly what the test shall do: the test is intended to check whether > the maximum values are enforced. And it does that by checking whether an error > is returned. OK that makes sense. Patch applied. Thanks! -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: fix maximum value checks on 32 bit systems 2014-08-26 8:58 ` Herbert Xu @ 2014-08-26 9:36 ` Stephan Mueller 2014-08-27 13:35 ` Herbert Xu 0 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-08-26 9:36 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel Am Dienstag, 26. August 2014, 16:58:53 schrieb Herbert Xu: Hi Herbert, > On Tue, Aug 26, 2014 at 10:52:45AM +0200, Stephan Mueller wrote: > > Am Dienstag, 26. August 2014, 16:43:43 schrieb Herbert Xu: > > > > Hi Herbert, > > > > > On Tue, Aug 26, 2014 at 10:29:45AM +0200, Stephan Mueller wrote: > > > > The maximum values for additional input string or generated blocks is > > > > larger than 1<<32. To ensure a sensible value on 32 bit systems, > > > > return > > > > SIZE_MAX on 32 bit systems. This value is lower than the maximum > > > > allowed values defined in SP800-90A. The standard allow lower maximum > > > > values, but not larger values. > > > > > > > > SIZE_MAX - 1 is used for drbg_max_addtl to allow > > > > drbg_healthcheck_sanity to check the enforcement of the variable > > > > without wrapping. > > > > > > This is really ugly but OK. However, I'm not sure how the sanity > > > check ever worked. It would appear that the drbg_generate call in > > > drbg_healthcheck_sanity should always fail because you explicitly > > > set addtl->len to drbg_max_addtl + 1, which should trigger the > > > "DRBG: additional information string too long" error, no? > > > > That is exactly what the test shall do: the test is intended to check > > whether the maximum values are enforced. And it does that by checking > > whether an error is returned. > > OK that makes sense. Patch applied. Thanks! I am wondering about the current code in Linus' tree though considering the applied patch: Linus' code contains: static inline size_t drbg_max_addtl(struct drbg_state *drbg) { return (1UL<<(drbg->core->max_addtllen)); } static inline size_t drbg_max_requests(struct drbg_state *drbg) { return (1UL<<(drbg->core->max_req)); } The max_addtllen and max_req are defined in drbg_cores[] in crypto/drbg.c for each DRBG type. As size_t on a 32 bit system is 32 bit the bit shifts would not work either. Thus, I am wondering whether the just applied patch would need to go to Linus tree too? I would think that the following patch would be in order: static inline size_t drbg_max_addtl(struct drbg_state *drbg) { #if (__BITS_PER_LONG == 32) return (SIZE_MAX - 1); #else return (1UL<<(drbg->core->max_addtllen)); #endif } static inline size_t drbg_max_requests(struct drbg_state *drbg) { #if (__BITS_PER_LONG == 32) return SIZE_MAX; #else return (1UL<<(drbg->core->max_req)); #endif } -- Ciao Stephan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: fix maximum value checks on 32 bit systems 2014-08-26 9:36 ` Stephan Mueller @ 2014-08-27 13:35 ` Herbert Xu 2014-08-27 13:40 ` Stephan Mueller ` (2 more replies) 0 siblings, 3 replies; 24+ messages in thread From: Herbert Xu @ 2014-08-27 13:35 UTC (permalink / raw) To: Stephan Mueller; +Cc: Stephen Rothwell, linux-next, linux-kernel On Tue, Aug 26, 2014 at 11:36:54AM +0200, Stephan Mueller wrote: > > The max_addtllen and max_req are defined in drbg_cores[] in crypto/drbg.c for > each DRBG type. As size_t on a 32 bit system is 32 bit the bit shifts would > not work either. > > Thus, I am wondering whether the just applied patch would need to go to Linus > tree too? I would think that the following patch would be in order: Have you actually tested this on a 32-bit box? If so and it actually works then I'd be happy to push it. Cheers, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: fix maximum value checks on 32 bit systems 2014-08-27 13:35 ` Herbert Xu @ 2014-08-27 13:40 ` Stephan Mueller 2014-08-28 7:13 ` Stephan Mueller 2014-08-28 7:17 ` DRBG: remove test for uninitialized DRBG handle Stephan Mueller 2 siblings, 0 replies; 24+ messages in thread From: Stephan Mueller @ 2014-08-27 13:40 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel Am Mittwoch, 27. August 2014, 21:35:28 schrieb Herbert Xu: Hi Herbert, >On Tue, Aug 26, 2014 at 11:36:54AM +0200, Stephan Mueller wrote: >> The max_addtllen and max_req are defined in drbg_cores[] in >> crypto/drbg.c for each DRBG type. As size_t on a 32 bit system is 32 >> bit the bit shifts would not work either. >> >> Thus, I am wondering whether the just applied patch would need to go >> to Linus >> tree too? I would think that the following patch would be in order: >Have you actually tested this on a 32-bit box? If so and it >actually works then I'd be happy to push it. I will have it tested by tomorrow. > >Cheers, Ciao Stephan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: fix maximum value checks on 32 bit systems 2014-08-27 13:35 ` Herbert Xu 2014-08-27 13:40 ` Stephan Mueller @ 2014-08-28 7:13 ` Stephan Mueller 2014-08-28 7:17 ` DRBG: remove test for uninitialized DRBG handle Stephan Mueller 2 siblings, 0 replies; 24+ messages in thread From: Stephan Mueller @ 2014-08-28 7:13 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel Am Mittwoch, 27. August 2014, 21:35:28 schrieb Herbert Xu: Hi Herbert, > On Tue, Aug 26, 2014 at 11:36:54AM +0200, Stephan Mueller wrote: > > The max_addtllen and max_req are defined in drbg_cores[] in crypto/drbg.c > > for each DRBG type. As size_t on a 32 bit system is 32 bit the bit shifts > > would not work either. > > > > Thus, I am wondering whether the just applied patch would need to go to > > Linus > > tree too? I would think that the following patch would be in order: > Have you actually tested this on a 32-bit box? If so and it > actually works then I'd be happy to push it. I tested the current cryptodev-2.6: - on x86_64 and on i386 - in FIPS mode and without FIPS mode on both arches - executing the CAVS testing in FIPS mode and non-FIPS mode on both arches - executing stress tests (obtain random values at a size randomly selected between 1 through 1 million bytes) in both modes on both arches - adding some debug printks to check for the drbg_max_addtl() on both arches - adding some debug printks to check for the drbg_max_requests() on both arches I found a bug that was introduced with bc034ef5573ef4d81daa666c02a3df1ad28e24a7 when booting in FIPS mode, i.e. it is not in Linus' tree. The patch will be on your desk shortly. Thus, the fix in b9347aff91ce4789619168539f08202d8d6a1177 works. However, this patch is based on 05c81ccd9087d238c10b234eadb55632742e5518. So, if we want to fix Linus' tree with minimal impact, either these two patches are pushed to Linus or I have to port b9347aff91ce4789619168539f08202d8d6a1177 to the current Linus tree. > > Cheers, -- Ciao Stephan ^ permalink raw reply [flat|nested] 24+ messages in thread
* DRBG: remove test for uninitialized DRBG handle 2014-08-27 13:35 ` Herbert Xu 2014-08-27 13:40 ` Stephan Mueller 2014-08-28 7:13 ` Stephan Mueller @ 2014-08-28 7:17 ` Stephan Mueller 2014-09-01 5:11 ` [PATCH v2] DRBG: remove check " Stephan Mueller 2 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-08-28 7:17 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel The drbg_healthcheck() contains a test to call the DRBG with an uninitialized DRBG cipher handle. As this is an inappropriate use of the kernel crypto API to try to generate random numbers before initialization, checks verifying for an initialized DRBG during the generate function have been removed in previous patches. Now, the drbg_healthcheck test trying to generate random numbers with an uninstantiated DRBG must also be removed. Signed-off-by: Stephan Mueller <smueller@chronox.de> --- crypto/drbg.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/crypto/drbg.c b/crypto/drbg.c index 39ed918..54cfd48 100644 --- a/crypto/drbg.c +++ b/crypto/drbg.c @@ -1872,9 +1872,6 @@ static inline int __init drbg_healthcheck_sanity(void) /* overflow max addtllen with personalization string */ ret = drbg_instantiate(drbg, &addtl, coreref, pr); BUG_ON(0 == ret); - /* test uninstantated DRBG */ - len = drbg_generate(drbg, buf, (max_request_bytes + 1), NULL); - BUG_ON(0 < len); /* all tests passed */ rc = 0; -- 1.9.3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2] DRBG: remove check for uninitialized DRBG handle 2014-08-28 7:17 ` DRBG: remove test for uninitialized DRBG handle Stephan Mueller @ 2014-09-01 5:11 ` Stephan Mueller 2014-09-03 1:33 ` Stephan Mueller 2014-09-05 8:13 ` Herbert Xu 0 siblings, 2 replies; 24+ messages in thread From: Stephan Mueller @ 2014-09-01 5:11 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel The drbg_healthcheck() contained a test to call the DRBG with an uninitialized DRBG cipher handle. As this is an inappropriate use of the kernel crypto API to try to generate random numbers before initialization, checks verifying for an initialized DRBG have been removed in previous patches. Now, the drbg_healthcheck test must also be removed. Changes V2: Added patch marker to email subject line. Signed-off-by: Stephan Mueller <smueller@chronox.de> --- crypto/drbg.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/crypto/drbg.c b/crypto/drbg.c index 39ed918..54cfd48 100644 --- a/crypto/drbg.c +++ b/crypto/drbg.c @@ -1872,9 +1872,6 @@ static inline int __init drbg_healthcheck_sanity(void) /* overflow max addtllen with personalization string */ ret = drbg_instantiate(drbg, &addtl, coreref, pr); BUG_ON(0 == ret); - /* test uninstantated DRBG */ - len = drbg_generate(drbg, buf, (max_request_bytes + 1), NULL); - BUG_ON(0 < len); /* all tests passed */ rc = 0; -- 1.9.3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: remove check for uninitialized DRBG handle 2014-09-01 5:11 ` [PATCH v2] DRBG: remove check " Stephan Mueller @ 2014-09-03 1:33 ` Stephan Mueller 2014-09-03 23:21 ` Herbert Xu 2014-09-05 8:13 ` Herbert Xu 1 sibling, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-09-03 1:33 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel Am Montag, 1. September 2014, 07:11:20 schrieb Stephan Mueller: Hi Herbert, may I ask for consideration of this patch as this covers an oops FIPS mode? In addition, may I ask for guidance on how to fix the 32 bit code path in Linus' tree as asked on 28.8? To quote: "Thus, the fix in b9347aff91ce4789619168539f08202d8d6a1177 works. However, this patch is based on 05c81ccd9087d238c10b234eadb55632742e5518. So, if we want to fix Linus' tree with minimal impact, either these two patches are pushed to Linus or I have to port b9347aff91ce4789619168539f08202d8d6a1177 to the current Linus tree." Thanks a lot Stephan > The drbg_healthcheck() contained a test to call the DRBG with an > uninitialized DRBG cipher handle. As this is an inappropriate use of the > kernel crypto API to try to generate random numbers before > initialization, checks verifying for an initialized DRBG have been > removed in previous patches. > > Now, the drbg_healthcheck test must also be removed. > > Changes V2: Added patch marker to email subject line. > > Signed-off-by: Stephan Mueller <smueller@chronox.de> > --- > crypto/drbg.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/crypto/drbg.c b/crypto/drbg.c > index 39ed918..54cfd48 100644 > --- a/crypto/drbg.c > +++ b/crypto/drbg.c > @@ -1872,9 +1872,6 @@ static inline int __init drbg_healthcheck_sanity(void) > /* overflow max addtllen with personalization string */ > ret = drbg_instantiate(drbg, &addtl, coreref, pr); > BUG_ON(0 == ret); > - /* test uninstantated DRBG */ > - len = drbg_generate(drbg, buf, (max_request_bytes + 1), NULL); > - BUG_ON(0 < len); > /* all tests passed */ > rc = 0; -- Ciao Stephan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: remove check for uninitialized DRBG handle 2014-09-03 1:33 ` Stephan Mueller @ 2014-09-03 23:21 ` Herbert Xu 2014-09-03 23:50 ` Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Herbert Xu @ 2014-09-03 23:21 UTC (permalink / raw) To: Stephan Mueller; +Cc: Stephen Rothwell, linux-next, linux-kernel On Wed, Sep 03, 2014 at 03:33:16AM +0200, Stephan Mueller wrote: > Am Montag, 1. September 2014, 07:11:20 schrieb Stephan Mueller: > > Hi Herbert, > > may I ask for consideration of this patch as this covers an oops FIPS mode? > > In addition, may I ask for guidance on how to fix the 32 bit code path in > Linus' tree as asked on 28.8? To quote: "Thus, the fix in > b9347aff91ce4789619168539f08202d8d6a1177 works. However, this > patch is based on 05c81ccd9087d238c10b234eadb55632742e5518. So, if we want to > fix Linus' tree with minimal impact, either these two patches are pushed to > Linus or I have to port b9347aff91ce4789619168539f08202d8d6a1177 to the > current Linus tree." I will take care of this. Cheers, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: remove check for uninitialized DRBG handle 2014-09-03 23:21 ` Herbert Xu @ 2014-09-03 23:50 ` Stephan Mueller 2014-09-05 7:55 ` Herbert Xu 0 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-09-03 23:50 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel Am Donnerstag, 4. September 2014, 07:21:29 schrieb Herbert Xu: Hi Herbert, > On Wed, Sep 03, 2014 at 03:33:16AM +0200, Stephan Mueller wrote: > > Am Montag, 1. September 2014, 07:11:20 schrieb Stephan Mueller: > > > > Hi Herbert, > > > > may I ask for consideration of this patch as this covers an oops FIPS > > mode? > > > > In addition, may I ask for guidance on how to fix the 32 bit code path in > > Linus' tree as asked on 28.8? To quote: "Thus, the fix in > > b9347aff91ce4789619168539f08202d8d6a1177 works. However, this > > patch is based on 05c81ccd9087d238c10b234eadb55632742e5518. So, if we want > > to fix Linus' tree with minimal impact, either these two patches are > > pushed to Linus or I have to port > > b9347aff91ce4789619168539f08202d8d6a1177 to the current Linus tree." > > I will take care of this. Thank you. > > Cheers, -- Ciao Stephan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: remove check for uninitialized DRBG handle 2014-09-03 23:50 ` Stephan Mueller @ 2014-09-05 7:55 ` Herbert Xu 2014-09-05 11:25 ` Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Herbert Xu @ 2014-09-05 7:55 UTC (permalink / raw) To: Stephan Mueller; +Cc: Stephen Rothwell, linux-next, linux-kernel On Thu, Sep 04, 2014 at 01:50:32AM +0200, Stephan Mueller wrote: > Am Donnerstag, 4. September 2014, 07:21:29 schrieb Herbert Xu: > > Hi Herbert, > > > On Wed, Sep 03, 2014 at 03:33:16AM +0200, Stephan Mueller wrote: > > > Am Montag, 1. September 2014, 07:11:20 schrieb Stephan Mueller: > > > > > > Hi Herbert, > > > > > > may I ask for consideration of this patch as this covers an oops FIPS > > > mode? > > > > > > In addition, may I ask for guidance on how to fix the 32 bit code path in > > > Linus' tree as asked on 28.8? To quote: "Thus, the fix in > > > b9347aff91ce4789619168539f08202d8d6a1177 works. However, this > > > patch is based on 05c81ccd9087d238c10b234eadb55632742e5518. So, if we want > > > to fix Linus' tree with minimal impact, either these two patches are > > > pushed to Linus or I have to port > > > b9347aff91ce4789619168539f08202d8d6a1177 to the current Linus tree." > > > > I will take care of this. > > Thank you. Here is the patch I will add for 3.17: commit fb38ab4cd05e11184fd2c3ef916fa106ecc505fc Author: Herbert Xu <herbert@gondor.apana.org.au> Date: Fri Sep 5 15:52:28 2014 +0800 crypto: drbg - backport "fix maximum value checks on 32 bit systems" This is a backport of commit b9347aff91ce4789619168539f08202d8d6a1177. This backport is needed as without it the code will crash on 32-bit systems. The maximum values for additional input string or generated blocks is larger than 1<<32. To ensure a sensible value on 32 bit systems, return SIZE_MAX on 32 bit systems. This value is lower than the maximum allowed values defined in SP800-90A. The standard allow lower maximum values, but not larger values. SIZE_MAX - 1 is used for drbg_max_addtl to allow drbg_healthcheck_sanity to check the enforcement of the variable without wrapping. Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Reported-by: kbuild test robot <fengguang.wu@intel.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h index 831d786..882675e 100644 --- a/include/crypto/drbg.h +++ b/include/crypto/drbg.h @@ -162,12 +162,25 @@ static inline size_t drbg_max_request_bytes(struct drbg_state *drbg) static inline size_t drbg_max_addtl(struct drbg_state *drbg) { +#if (__BITS_PER_LONG == 32) + /* + * SP800-90A allows smaller maximum numbers to be returned -- we + * return SIZE_MAX - 1 to allow the verification of the enforcement + * of this value in drbg_healthcheck_sanity. + */ + return (SIZE_MAX - 1); +#else return (1UL<<(drbg->core->max_addtllen)); +#endif } static inline size_t drbg_max_requests(struct drbg_state *drbg) { +#if (__BITS_PER_LONG == 32) + return SIZE_MAX; +#else return (1UL<<(drbg->core->max_req)); +#endif } /* Cheers, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: remove check for uninitialized DRBG handle 2014-09-05 7:55 ` Herbert Xu @ 2014-09-05 11:25 ` Stephan Mueller 0 siblings, 0 replies; 24+ messages in thread From: Stephan Mueller @ 2014-09-05 11:25 UTC (permalink / raw) To: Herbert Xu; +Cc: Stephen Rothwell, linux-next, linux-kernel Am Freitag, 5. September 2014, 15:55:49 schrieb Herbert Xu: Hi Herbert, >On Thu, Sep 04, 2014 at 01:50:32AM +0200, Stephan Mueller wrote: >> Am Donnerstag, 4. September 2014, 07:21:29 schrieb Herbert Xu: >> >> Hi Herbert, >> >> > On Wed, Sep 03, 2014 at 03:33:16AM +0200, Stephan Mueller wrote: >> > > Am Montag, 1. September 2014, 07:11:20 schrieb Stephan Mueller: >> > > >> > > Hi Herbert, >> > > >> > > may I ask for consideration of this patch as this covers an oops >> > > FIPS >> > > mode? >> > > >> > > In addition, may I ask for guidance on how to fix the 32 bit code >> > > path in Linus' tree as asked on 28.8? To quote: "Thus, the fix >> > > in >> > > b9347aff91ce4789619168539f08202d8d6a1177 works. However, this >> > > patch is based on 05c81ccd9087d238c10b234eadb55632742e5518. So, >> > > if we want to fix Linus' tree with minimal impact, either these >> > > two patches are pushed to Linus or I have to port >> > > b9347aff91ce4789619168539f08202d8d6a1177 to the current Linus >> > > tree." >> > >> > I will take care of this. >> >> Thank you. > >Here is the patch I will add for 3.17: > >commit fb38ab4cd05e11184fd2c3ef916fa106ecc505fc >Author: Herbert Xu <herbert@gondor.apana.org.au> >Date: Fri Sep 5 15:52:28 2014 +0800 > > crypto: drbg - backport "fix maximum value checks on 32 bit >systems" > > This is a backport of commit >b9347aff91ce4789619168539f08202d8d6a1177. This backport is needed as >without it the code will crash on 32-bit systems. The kernel / module will not crash, It will simply refuse to work by always returning an error. I have tested the 3.17-rc1 code on 32 bit which returned always the error unless I apply this patch. > > The maximum values for additional input string or generated blocks >is larger than 1<<32. To ensure a sensible value on 32 bit systems, >return SIZE_MAX on 32 bit systems. This value is lower than the >maximum allowed values defined in SP800-90A. The standard allow lower >maximum values, but not larger values. > > SIZE_MAX - 1 is used for drbg_max_addtl to allow > drbg_healthcheck_sanity to check the enforcement of the variable > without wrapping. > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Reported-by: kbuild test robot <fengguang.wu@intel.com> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > >diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h >index 831d786..882675e 100644 >--- a/include/crypto/drbg.h >+++ b/include/crypto/drbg.h >@@ -162,12 +162,25 @@ static inline size_t >drbg_max_request_bytes(struct drbg_state *drbg) > > static inline size_t drbg_max_addtl(struct drbg_state *drbg) > { >+#if (__BITS_PER_LONG == 32) >+ /* >+ * SP800-90A allows smaller maximum numbers to be returned -- we >+ * return SIZE_MAX - 1 to allow the verification of the enforcement >+ * of this value in drbg_healthcheck_sanity. >+ */ >+ return (SIZE_MAX - 1); >+#else > return (1UL<<(drbg->core->max_addtllen)); >+#endif > } > > static inline size_t drbg_max_requests(struct drbg_state *drbg) > { >+#if (__BITS_PER_LONG == 32) >+ return SIZE_MAX; >+#else > return (1UL<<(drbg->core->max_req)); >+#endif > } > > /* > >Cheers, Thank you very much! Acked-by: Stephan Mueller <smueller@chronox.de> Ciao Stephan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] DRBG: remove check for uninitialized DRBG handle 2014-09-01 5:11 ` [PATCH v2] DRBG: remove check " Stephan Mueller 2014-09-03 1:33 ` Stephan Mueller @ 2014-09-05 8:13 ` Herbert Xu 1 sibling, 0 replies; 24+ messages in thread From: Herbert Xu @ 2014-09-05 8:13 UTC (permalink / raw) To: Stephan Mueller; +Cc: Stephen Rothwell, linux-next, linux-kernel On Mon, Sep 01, 2014 at 07:11:20AM +0200, Stephan Mueller wrote: > The drbg_healthcheck() contained a test to call the DRBG with an > uninitialized DRBG cipher handle. As this is an inappropriate use of the > kernel crypto API to try to generate random numbers before > initialization, checks verifying for an initialized DRBG have been > removed in previous patches. > > Now, the drbg_healthcheck test must also be removed. > > Changes V2: Added patch marker to email subject line. > > Signed-off-by: Stephan Mueller <smueller@chronox.de> Patch applied to crypto. Thanks! -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2014-09-05 11:25 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-08-26 6:14 linux-next: build warnings after merge of the crypto tree Stephen Rothwell 2014-08-26 6:38 ` Herbert Xu 2014-08-26 7:00 ` Stephan Mueller 2014-08-26 7:31 ` [PATCH] DRBG: fix bit shifting on 32 bit systems Stephan Mueller 2014-08-26 7:32 ` Herbert Xu 2014-08-26 7:37 ` Stephan Mueller 2014-08-26 8:06 ` [PATCH] DRBG: fix maximum value checks " Stephan Mueller 2014-08-26 8:08 ` Herbert Xu 2014-08-26 8:29 ` [PATCH v2] " Stephan Mueller 2014-08-26 8:43 ` Herbert Xu 2014-08-26 8:52 ` Stephan Mueller 2014-08-26 8:58 ` Herbert Xu 2014-08-26 9:36 ` Stephan Mueller 2014-08-27 13:35 ` Herbert Xu 2014-08-27 13:40 ` Stephan Mueller 2014-08-28 7:13 ` Stephan Mueller 2014-08-28 7:17 ` DRBG: remove test for uninitialized DRBG handle Stephan Mueller 2014-09-01 5:11 ` [PATCH v2] DRBG: remove check " Stephan Mueller 2014-09-03 1:33 ` Stephan Mueller 2014-09-03 23:21 ` Herbert Xu 2014-09-03 23:50 ` Stephan Mueller 2014-09-05 7:55 ` Herbert Xu 2014-09-05 11:25 ` Stephan Mueller 2014-09-05 8:13 ` Herbert Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).