Re: Crypto Fixes for 4.12
From: Theodore Ts'o Date: Fri, 16 Jun 2017 08:50:07 -0400 > On Thu, Jun 15, 2017 at 11:01:18AM -0400, David Miller wrote: >> As a side note, ext4 does something similar with a private >> implementation, but it doesn't use something the evaluates to an >> alloca. Instead it uses a fixed 4-byte size for the shash context >> value in the on-stack declaration. > > In ext4's case, we're doing it inside an inline function, and then > using the "return" value from inside the calling function. Assuming > that gcc actually inlines the function, are we in danger of tripping > over the bug? Again, the bug can only be triggered if you do a dynamically sized object on the stack. Which ext4 is not doing, since it uses fixed size elements in the on-stack shash context.
Re: Crypto Fixes for 4.12
On Thu, Jun 15, 2017 at 11:01:18AM -0400, David Miller wrote: > As a side note, ext4 does something similar with a private > implementation, but it doesn't use something the evaluates to an > alloca. Instead it uses a fixed 4-byte size for the shash context > value in the on-stack declaration. In ext4's case, we're doing it inside an inline function, and then using the "return" value from inside the calling function. Assuming that gcc actually inlines the function, are we in danger of tripping over the bug? - Ted
Re: Crypto Fixes for 4.12
From: Linus Torvalds Date: Thu, 15 Jun 2017 18:04:44 +0900 > There's a fair number of SHASH_DESC_ON_STACK users, are all the others > safe for some random reason that just happens to be about code > generation? Did people actually verify that? I looked at the code generated in every case. As a side note, ext4 does something similar with a private implementation, but it doesn't use something the evaluates to an alloca. Instead it uses a fixed 4-byte size for the shash context value in the on-stack declaration. We can tidy it up with abstraction macros as a follow-up, thanks for the suggestion. I'll look into it.
Re: Crypto Fixes for 4.12
From: Herbert Xu Date: Thu, 15 Jun 2017 17:42:10 +0800 > On Thu, Jun 15, 2017 at 06:04:44PM +0900, Linus Torvalds wrote: >> There's a fair number of SHASH_DESC_ON_STACK users, are all the others >> safe for some random reason that just happens to be about code >> generation? Did people actually verify that? > > If I understand this correctly this is only an issue if you directly > return a value from the shash_desc struct allocated on the stack. > This is usually rare as normally you'd return an error code and the > hash result would be written directly to some memory passed in from > the caller. Correct.
Re: Crypto Fixes for 4.12
On Thu, Jun 15, 2017 at 06:04:44PM +0900, Linus Torvalds wrote: > > Then you *could* implement SHASH_DESC_ON_STACK() as a kmalloc, and > SHASH_DESC_DEALLOC() would be a kfree - but with an alloca()-like > allocation the SHASH_DESC_DEALLOC() would be that "barrier_data()". > > At that point the interface would make _sense_ at some conceptual > level, rather than being a random hack for a small collection of > random users of this thing. Yes we could probably do that. > There's a fair number of SHASH_DESC_ON_STACK users, are all the others > safe for some random reason that just happens to be about code > generation? Did people actually verify that? If I understand this correctly this is only an issue if you directly return a value from the shash_desc struct allocated on the stack. This is usually rare as normally you'd return an error code and the hash result would be written directly to some memory passed in from the caller. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: Crypto Fixes for 4.12
On Thu, Jun 15, 2017 at 6:04 PM, Linus Torvalds wrote: > > Ugh, that's a particularly ugly fix for a random gcc bug on a random > architecture that almost nobody tests. .. anway, I pulled it, but I don't have to like it. Linus
Re: Crypto Fixes for 4.12
On Thu, Jun 15, 2017 at 9:54 AM, Herbert Xu wrote: > > This push fixes a bug on sparc where we may dereference freed stack > memory. Ugh, that's a particularly ugly fix for a random gcc bug on a random architecture that almost nobody tests. In other words, it's nasty. It's nasty because nobody sane will ever realize this pattern, and the code will either bit-rot or just happen again somewhere else. I'd have been *much* happier if this had been some nicer abstraction that is built up around the use of SHASH_DESC_ON_STACK(), and just have some rule that "SHASH_DESC_ON_STACK()" needs to be paired with retrieving the final value and then a SHASH_DESC_DEALLOC() or whatever. Then you *could* implement SHASH_DESC_ON_STACK() as a kmalloc, and SHASH_DESC_DEALLOC() would be a kfree - but with an alloca()-like allocation the SHASH_DESC_DEALLOC() would be that "barrier_data()". At that point the interface would make _sense_ at some conceptual level, rather than being a random hack for a small collection of random users of this thing. There's a fair number of SHASH_DESC_ON_STACK users, are all the others safe for some random reason that just happens to be about code generation? Did people actually verify that? Linus
Crypto Fixes for 4.12
Hi Linus: This push fixes a bug on sparc where we may dereference freed stack memory. Please pull from git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git linus David Miller (1): crypto: Work around deallocated stack frame reference gcc bug on sparc. drivers/infiniband/sw/rxe/rxe.h |5 - fs/btrfs/hash.c |5 - fs/f2fs/f2fs.h |5 - lib/libcrc32c.c |6 -- 4 files changed, 16 insertions(+), 5 deletions(-) Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: Crypto Fixes for 4.12
On Thu, Jun 08, 2017 at 10:05:02AM -0400, David Miller wrote: > From: Herbert Xu > Date: Thu, 8 Jun 2017 17:23:21 +0800 > > > This push fixes a couple of places in the crypto code that were > > doing interruptible sleeps dangerously. They have been converted > > to use non-interruptible sleeps. This push also fixes a bug in > > asymmetric_keys where it would trigger a use-after-free if a > > request returned EBUSY due to a full device queue. > > Where is the gcc shash miscompile workaround? I'll push it to Linus in a couple of days. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: Crypto Fixes for 4.12
From: Herbert Xu Date: Thu, 8 Jun 2017 17:23:21 +0800 > This push fixes a couple of places in the crypto code that were > doing interruptible sleeps dangerously. They have been converted > to use non-interruptible sleeps. This push also fixes a bug in > asymmetric_keys where it would trigger a use-after-free if a > request returned EBUSY due to a full device queue. Where is the gcc shash miscompile workaround? Thanks.
Crypto Fixes for 4.12
Hi Linus: This push fixes a couple of places in the crypto code that were doing interruptible sleeps dangerously. They have been converted to use non-interruptible sleeps. This push also fixes a bug in asymmetric_keys where it would trigger a use-after-free if a request returned EBUSY due to a full device queue. Please pull from git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git linus Gilad Ben-Yossef (3): crypto: asymmetric_keys - handle EBUSY due to backlog correctly crypto: drbg - wait for crypto op not signal safe crypto: gcm - wait for crypto op not signal safe crypto/asymmetric_keys/public_key.c |2 +- crypto/drbg.c |5 ++--- crypto/gcm.c|6 ++ 3 files changed, 5 insertions(+), 8 deletions(-) Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Crypto Fixes for 4.12
Hi Linus: This push fixes a regression in the skcipher interface that allows bogus key parameters to hit underlying implementations which can cause crashes. Please pull from git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git linus Herbert Xu (1): crypto: skcipher - Add missing API setkey checks crypto/skcipher.c | 40 +++- 1 file changed, 39 insertions(+), 1 deletion(-) Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
