Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1660976 > unrolled thread

Crypto Fixes for 4.12

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Crypto Fixes for 4.12 Herbert Xu <herbert@gondor.apana.org.au> - 2017-06-08 11:30 +0200
    Re: Crypto Fixes for 4.12 David Miller <davem@davemloft.net> - 2017-06-08 16:10 +0200
      Re: Crypto Fixes for 4.12 Herbert Xu <herbert@gondor.apana.org.au> - 2017-06-09 05:00 +0200
    Crypto Fixes for 4.12 Herbert Xu <herbert@gondor.apana.org.au> - 2017-06-15 03:00 +0200
      Re: Crypto Fixes for 4.12 Linus Torvalds <torvalds@linux-foundation.org> - 2017-06-15 11:10 +0200
        Re: Crypto Fixes for 4.12 Linus Torvalds <torvalds@linux-foundation.org> - 2017-06-15 11:10 +0200
        Re: Crypto Fixes for 4.12 Herbert Xu <herbert@gondor.apana.org.au> - 2017-06-15 11:50 +0200
          Re: Crypto Fixes for 4.12 David Miller <davem@davemloft.net> - 2017-06-15 17:10 +0200
        Re: Crypto Fixes for 4.12 David Miller <davem@davemloft.net> - 2017-06-15 17:10 +0200
          Re: Crypto Fixes for 4.12 Theodore Ts'o <tytso@mit.edu> - 2017-06-16 15:00 +0200
            Re: Crypto Fixes for 4.12 David Miller <davem@davemloft.net> - 2017-06-16 19:00 +0200

#1660976 — 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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [next] | [standalone]

#1661297

From: Herbert Xu <herbert@gondor.apana.org.au>
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.

[toc] | [prev] | [next] | [standalone]

#1661881

On Thu, Jun 08, 2017 at 10:05:02AM -0400, David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> 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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [next] | [standalone]

#1666343

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [next] | [standalone]

#1666586

On Thu, Jun 15, 2017 at 9:54 AM, Herbert Xu <herbert@gondor.apana.org.au> 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

[toc] | [prev] | [next] | [standalone]

#1666587

On Thu, Jun 15, 2017 at 6:04 PM, Linus Torvalds
<torvalds@linux-foundation.org> 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

[toc] | [prev] | [next] | [standalone]

#1666604

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [next] | [standalone]

#1666786

From: Herbert Xu <herbert@gondor.apana.org.au>
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.

[toc] | [prev] | [next] | [standalone]

#1666783

From: Linus Torvalds <torvalds@linux-foundation.org>
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.

[toc] | [prev] | [next] | [standalone]

#1667735

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

[toc] | [prev] | [next] | [standalone]

#1667896

From: Theodore Ts'o <tytso@mit.edu>
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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web