Re: [kernel-hardening] Re: [PATCH v4 06/13] iscsi: ensure RNG is seeded before use

2017-06-16 Thread Lee Duncan 
On 06/16/2017 05:41 PM, Jason A. Donenfeld wrote:
> Hi Lee,
> 
> On Fri, Jun 16, 2017 at 11:58 PM, Lee Duncan  wrote:
>> It seems like what you are doing is basically "good", i.e. if there is
>> not enough random data, don't use it. But what happens in that case? The
>> authentication fails? How does the user know to wait and try again?
> 
> The process just remains in interruptible (kill-able) sleep until
> there is enough entropy, so the process doesn't need to do anything.
> If the waiting is interrupted by a signal, it returns -ESYSRESTART,
> which follows the usual semantics of restartable syscalls.
> 
> Jason
> 

In your testing, how long might a process have to wait? Are we talking
seconds? Longer? What about timeouts?

Sorry, but your changing something that isn't exactly broken, so I just
want to be sure we're not introducing some regression, like clients
can't connect the first 5 minutes are a reboot.
-- 
Lee Duncan

[PATCH V2] staging: ccree: - style fix, spaces and tabs

2017-06-16 Thread Derek Robson 
Changed code indent to be tabs across whole driver
Found using checkpatch

Signed-off-by: Derek Robson 

V1 had vague subject.
---
 drivers/staging/ccree/ssi_cipher.c | 45 +-
 drivers/staging/ccree/ssi_driver.c |  6 ++---
 drivers/staging/ccree/ssi_driver.h |  6 ++---
 drivers/staging/ccree/ssi_fips.h   |  8 +++---
 drivers/staging/ccree/ssi_fips_ext.c   |  4 +--
 drivers/staging/ccree/ssi_fips_ll.c| 40 +++---
 drivers/staging/ccree/ssi_fips_local.c | 28 ++---
 drivers/staging/ccree/ssi_fips_local.h | 12 -
 8 files changed, 75 insertions(+), 74 deletions(-)

diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index 2dfc6a3bd4c1..34450a5e6573 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -258,45 +258,45 @@ static void ssi_blkcipher_exit(struct crypto_tfm *tfm)
 
 
 typedef struct tdes_keys{
-u8  key1[DES_KEY_SIZE];
-u8  key2[DES_KEY_SIZE];
-u8  key3[DES_KEY_SIZE];
+   u8  key1[DES_KEY_SIZE];
+   u8  key2[DES_KEY_SIZE];
+   u8  key3[DES_KEY_SIZE];
 }tdes_keys_t;
 
-static const u8 zero_buff[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
-   0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
-   0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
-   0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0};
+static const u8 zero_buff[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+   0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+   0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+   0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0};
 
 /* The function verifies that tdes keys are not weak.*/
 static int ssi_fips_verify_3des_keys(const u8 *key, unsigned int keylen)
 {
 #ifdef CCREE_FIPS_SUPPORT
-tdes_keys_t *tdes_key = (tdes_keys_t*)key;
+   tdes_keys_t *tdes_key = (tdes_keys_t*)key;
 
/* verify key1 != key2 and key3 != key2*/
-if (unlikely( (memcmp((u8*)tdes_key->key1, (u8*)tdes_key->key2, 
sizeof(tdes_key->key1)) == 0) ||
+   if (unlikely( (memcmp((u8*)tdes_key->key1, (u8*)tdes_key->key2, 
sizeof(tdes_key->key1)) == 0) ||
  (memcmp((u8*)tdes_key->key3, (u8*)tdes_key->key2, 
sizeof(tdes_key->key3)) == 0) )) {
-return -ENOEXEC;
-}
+   return -ENOEXEC;
+   }
 #endif /* CCREE_FIPS_SUPPORT */
 
-return 0;
+   return 0;
 }
 
 /* The function verifies that xts keys are not weak.*/
 static int ssi_fips_verify_xts_keys(const u8 *key, unsigned int keylen)
 {
 #ifdef CCREE_FIPS_SUPPORT
-/* Weak key is define as key that its first half (128/256 lsb) equals 
its second half (128/256 msb) */
-int singleKeySize = keylen >> 1;
+   /* Weak key is define as key that its first half (128/256 lsb) equals 
its second half (128/256 msb) */
+   int singleKeySize = keylen >> 1;
 
if (unlikely(memcmp(key, [singleKeySize], singleKeySize) == 0)) {
return -ENOEXEC;
}
 #endif /* CCREE_FIPS_SUPPORT */
 
-return 0;
+   return 0;
 }
 
 static enum cc_hw_crypto_key hw_key_to_cc_hw_key(int slot_num)
@@ -720,12 +720,13 @@ ssi_blkcipher_create_data_desc(
 }
 
 static int ssi_blkcipher_complete(struct device *dev,
-  struct ssi_ablkcipher_ctx *ctx_p,
-  struct blkcipher_req_ctx *req_ctx,
-  struct scatterlist *dst, struct scatterlist 
*src,
-  unsigned int ivsize,
-  void *areq,
-  void __iomem *cc_base)
+   struct ssi_ablkcipher_ctx *ctx_p,
+   struct blkcipher_req_ctx *req_ctx,
+   struct scatterlist *dst,
+   struct scatterlist *src,
+   unsigned int ivsize,
+   void *areq,
+   void __iomem *cc_base)
 {
int completion_error = 0;
u32 inflight_counter;
@@ -779,7 +780,7 @@ static int ssi_blkcipher_process(
/* No data to process is valid */
return 0;
}
-/*For CTS in case of data size aligned to 16 use CBC mode*/
+   /*For CTS in case of data size aligned to 16 use CBC mode*/
if (((nbytes % AES_BLOCK_SIZE) == 0) && (ctx_p->cipher_mode == 
DRV_CIPHER_CBC_CTS)){
 
ctx_p->cipher_mode = DRV_CIPHER_CBC;
diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index 190922970bf0..b9d0dd27e853 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -437,9 +437,9 @@ static void

Re: [kernel-hardening] Re: [PATCH v4 06/13] iscsi: ensure RNG is seeded before use

2017-06-16 Thread Jason A. Donenfeld 
Hi Lee,

On Fri, Jun 16, 2017 at 11:58 PM, Lee Duncan  wrote:
> It seems like what you are doing is basically "good", i.e. if there is
> not enough random data, don't use it. But what happens in that case? The
> authentication fails? How does the user know to wait and try again?

The process just remains in interruptible (kill-able) sleep until
there is enough entropy, so the process doesn't need to do anything.
If the waiting is interrupted by a signal, it returns -ESYSRESTART,
which follows the usual semantics of restartable syscalls.

Jason

Re: [PATCH] random: silence compiler warnings and fix race

2017-06-16 Thread Jason A. Donenfeld 
On Fri, Jun 16, 2017 at 4:35 PM, Sebastian Andrzej Siewior
 wrote:
> I wouldn't just push the lock one up as is but move that write part to
> crng_init to remain within the locked section. Like that:

We can't quite do that, because invalidate_batched_entropy() needs to
be called _before_ crng_init. Otherwise a concurrent call to
get_random_u32/u64() will have crng_init being the wrong value when
the batched entropy is still old.

> Are use about that? I am not sure that the gcc will inline "crng_init"
> read twice. It is not a local variable. READ_ONCE() is usually used
> where gcc could cache a memory access but you do not want this. But hey!
> If someone knows better I am here to learn.

The whole purpose is that I _want_ it to cache the memory access so
that it is _not_ inlined. So, based on your understanding, it does
exactly what I intended it to do. The reason is that I'd like to avoid
a lock imbalance, which could happen if the read is inlined.

Jason

Re: [PATCH v3 net-next 3/4] tls: kernel TLS support

2017-06-16 Thread Dave Watson 
On 06/16/17 01:58 PM, Stephen Hemminger wrote:
> On Wed, 14 Jun 2017 11:37:39 -0700
> Dave Watson  wrote:
> 
> > --- /dev/null
> > +++ b/net/tls/Kconfig
> > @@ -0,0 +1,12 @@
> > +#
> > +# TLS configuration
> > +#
> > +config TLS
> > +   tristate "Transport Layer Security support"
> > +   depends on NET
> > +   default m
> > +   ---help---
> > +   Enable kernel support for TLS protocol. This allows symmetric
> > +   encryption handling of the TLS protocol to be done in-kernel.
> > +
> > +   If unsure, say M.
> 
> I understand that this will be useful to lots of people and most distributions
> will enable it. But the defacto policy in kernel configuration has been that
> new features in kernel default to being disabled.

Sure, will send a patch to switch to default n.

Re: [PATCH v3 net-next 1/4] tcp: ULP infrastructure

2017-06-16 Thread Christoph Paasch 
Hello,

On 14/06/17 - 11:37:14, Dave Watson wrote:
> Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP
> sockets. Based on a similar infrastructure in tcp_cong.  The idea is that any
> ULP can add its own logic by changing the TCP proto_ops structure to its own
> methods.
> 
> Example usage:
> 
> setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));
> 
> modules will call:
> tcp_register_ulp(_tls_ulp_ops);
> 
> to register/unregister their ulp, with an init function and name.
> 
> A list of registered ulps will be returned by tcp_get_available_ulp, which is
> hooked up to /proc.  Example:
> 
> $ cat /proc/sys/net/ipv4/tcp_available_ulp
> tls
> 
> There is currently no functionality to remove or chain ULPs, but
> it should be possible to add these in the future if needed.
> 
> Signed-off-by: Boris Pismenny 
> Signed-off-by: Dave Watson 
> ---
>  include/net/inet_connection_sock.h |   4 ++
>  include/net/tcp.h  |  25 +++
>  include/uapi/linux/tcp.h   |   1 +
>  net/ipv4/Makefile  |   2 +-
>  net/ipv4/sysctl_net_ipv4.c |  25 +++
>  net/ipv4/tcp.c |  28 
>  net/ipv4/tcp_ipv4.c|   2 +
>  net/ipv4/tcp_ulp.c | 134 
> +
>  8 files changed, 220 insertions(+), 1 deletion(-)
>  create mode 100644 net/ipv4/tcp_ulp.c

I know I'm pretty late to the game (and maybe this has already been
discussed but I couldn't find anything in the archives), but I am wondering
what the take is on potential races of the setsockopt() vs other system-calls.

For example one might race the setsockopt() with a sendmsg() and the sendmsg
might end up blocking on the lock_sock in tcp_sendmsg, waiting for
tcp_set_ulp() to finish changing sk_prot. When the setsockopt() finishes, we
are then inside tcp_sendmsg() coming directly from sendmsg(), while we
should have been in the ULP's sendmsg.

It seems like TLS-ULP is resilient to this (or at least, won't cause a panic),
but there might be more exotic users of ULP in the future, that change other
callbacks and then things might go wrong.


Thoughts?


Thanks,
Christoph

Re: [kernel-hardening] Re: [PATCH v4 06/13] iscsi: ensure RNG is seeded before use

2017-06-16 Thread Lee Duncan 
On 06/08/2017 05:09 AM, Jason A. Donenfeld wrote:
> On Thu, Jun 8, 2017 at 4:43 AM, Theodore Ts'o  wrote:
>> What was the testing that was done for commit?  It looks safe, but I'm
>> unfamiliar enough with how the iSCSI authentication works that I'd
>> prefer getting an ack'ed by from the iSCSI maintainers or
>> alternativel, information about how to kick off some kind of automated
>> test suite ala xfstests for file systems.
> 
> Only very basic testing from my end.
> 
> I'm thus adding the iSCSI list to see if they'll have a look (patch 
> reattached).
> 
> Jason
> 

It seems like what you are doing is basically "good", i.e. if there is
not enough random data, don't use it. But what happens in that case? The
authentication fails? How does the user know to wait and try again?
-- 
Lee Duncan
SUSE Labs

Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver

2017-06-16 Thread Horia Geantă 
On 6/16/2017 11:00 AM, Herbert Xu wrote:
> On Fri, Jun 16, 2017 at 07:57:00AM +, Horia Geantă wrote:
>>
>> Commit 0605c41cc53ca ("crypto: cts - Convert to skcipher") appends
>> CRYPTO_TFM_REQ_MAY_BACKLOG to the original crypto request flags for the
>> last block - when calling cts_cbc_encrypt().
>> Is it really needed?
> 
> Yes, because at this point we cannot tell the sender to back off.
> 
>> For cts(cbc(aes)) with cbc(aes) offloaded in HW, i.e. running in async
>> mode, we get the below stack for CAAM driver.
>> Driver is told that it can sleep (CRYPTO_TFM_REQ_MAY_BACKLOG flag), so
>> it uses GFP_KERNEL to allocate memory. However, this is incorrect, since
>> driver runs in atomic context (softirq).
> 
> This is wrong.  Whether you can sleep or not is determined by
> MAY_SLEEP, not MAY_BACKLOG.  MAY_BACKLOG only indicates that this
> request must be queued, even if the queue is full.
> 
Indeed, CAAM driver incorrectly decides to use GFP_KERNEL for allocation
when MAY_BACKLOG flag is set. This seems to be a long-standing issue, I
will send a fix (separately).

Still I think we have a problem.
David reported that the user is fscrypt. Looking into fscrypt code, I
see that besides MAY_BACKLOG, MAY_SLEEP flag is also set. So we end up
in the situation I described earlier: the last block is encrypted in
atomic context and with MAY_SLEEP set.

Thanks,
Horia

Re: [PATCH v3 net-next 3/4] tls: kernel TLS support

2017-06-16 Thread Stephen Hemminger 
On Wed, 14 Jun 2017 11:37:39 -0700
Dave Watson  wrote:

> --- /dev/null
> +++ b/net/tls/Kconfig
> @@ -0,0 +1,12 @@
> +#
> +# TLS configuration
> +#
> +config TLS
> + tristate "Transport Layer Security support"
> + depends on NET
> + default m
> + ---help---
> + Enable kernel support for TLS protocol. This allows symmetric
> + encryption handling of the TLS protocol to be done in-kernel.
> +
> + If unsure, say M.

I understand that this will be useful to lots of people and most distributions
will enable it. But the defacto policy in kernel configuration has been that
new features in kernel default to being disabled.

Re: [PATCH v3 net-next 3/4] tls: kernel TLS support

2017-06-16 Thread Stephen Hemminger 
On Wed, 14 Jun 2017 11:37:39 -0700
Dave Watson  wrote:

> +
> +static inline struct tls_context *tls_get_ctx(const struct sock *sk)
> +{
> + struct inet_connection_sock *icsk = inet_csk(sk);
> +
> + return icsk->icsk_ulp_data;
> +}
> +
> +static inline struct tls_sw_context *tls_sw_ctx(
> + const struct tls_context *tls_ctx)
> +{
> + return (struct tls_sw_context *)tls_ctx->priv_ctx;
> +}
> +
> +static inline struct tls_offload_context *tls_offload_ctx(
> + const struct tls_context *tls_ctx)
> +{
> + return (struct tls_offload_context *)tls_ctx->priv_ctx;
> +}
> +

Since priv_ctx is void *, casts here are unnecessary.

Re: Crypto Fixes for 4.12

2017-06-16 Thread David Miller 
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: [PATCH v5 01/13] random: invalidate batched entropy after crng init

2017-06-16 Thread Sebastian Andrzej Siewior 
On 2017-06-16 14:12:42 [+0200], Jason A. Donenfeld wrote:
> I actually figured that out myself after sending the initial email, so
> then I wrote a follow-up patch which I attached to this thread. You
> should have received it. Can you take a look?

replied to the patch.

Sebastian

Re: [PATCH] random: silence compiler warnings and fix race

2017-06-16 Thread Sebastian Andrzej Siewior 
On 2017-06-15 00:45:26 [+0200], Jason A. Donenfeld wrote:
> Odd versions of gcc for the sh4 architecture will actually warn about
> flags being used while uninitialized, so we set them to zero. Non crazy
> gccs will optimize that out again, so it doesn't make a difference.

that is minor

> Next, over aggressive gccs could inline the expression that defines
> use_lock, which could then introduce a race resulting in a lock
> imbalance. By using READ_ONCE, we prevent that fate. Finally, we make
> that assignment const, so that gcc can still optimize a nice amount.

Not sure about that, more below.

> Finally, we fix a potential deadlock between primary_crng.lock and
> batched_entropy_reset_lock, where they could be called in opposite
> order. Moving the call to invalidate_batched_entropy to outside the lock
> rectifies this issue.

and *that* is separate issue which has to pulled in for stable once it
has been addressed in Linus' tree.

> Signed-off-by: Jason A. Donenfeld 
> ---
> Ted -- the first part of this is the fixup patch we discussed earlier.
> Then I added on top a fix for a potentially related race.
> 
> I'm not totally convinced that moving this block to outside the spinlock
> is 100% okay, so please give this a close look before merging.
> 
> 
>  drivers/char/random.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index e870f329db88..01a260f67437 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -803,13 +803,13 @@ static int crng_fast_load(const char *cp, size_t len)
>   p[crng_init_cnt % CHACHA20_KEY_SIZE] ^= *cp;
>   cp++; crng_init_cnt++; len--;
>   }
> + spin_unlock_irqrestore(_crng.lock, flags);
>   if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) {
>   invalidate_batched_entropy();
>   crng_init = 1;
>   wake_up_interruptible(_init_wait);
>   pr_notice("random: fast init done\n");
>   }
> - spin_unlock_irqrestore(_crng.lock, flags);
>   return 1;

I wouldn't just push the lock one up as is but move that write part to
crng_init to remain within the locked section. Like that:

diff --git a/drivers/char/random.c b/drivers/char/random.c
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -804,12 +804,14 @@ static int crng_fast_load(const char *cp, size_t len)
cp++; crng_init_cnt++; len--;
}
if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) {
-   invalidate_batched_entropy();
crng_init = 1;
+   spin_unlock_irqrestore(_crng.lock, flags);
+   invalidate_batched_entropy();
wake_up_interruptible(_init_wait);
pr_notice("random: fast init done\n");
+   } else {
+   spin_unlock_irqrestore(_crng.lock, flags);
}
-   spin_unlock_irqrestore(_crng.lock, flags);
return 1;
 }
 
@@ -842,13 +844,16 @@ static void crng_reseed(struct crng_state *crng, struct 
entropy_store *r)
memzero_explicit(, sizeof(buf));
crng->init_time = jiffies;
if (crng == _crng && crng_init < 2) {
-   invalidate_batched_entropy();
crng_init = 2;
+   spin_unlock_irqrestore(_crng.lock, flags);
+
+   invalidate_batched_entropy();
process_random_ready_list();
wake_up_interruptible(_init_wait);
pr_notice("random: crng init done\n");
+   } else {
+   spin_unlock_irqrestore(_crng.lock, flags);
}
-   spin_unlock_irqrestore(_crng.lock, flags);
 }
 
 static inline void crng_wait_ready(void)

>  }
>  
> @@ -2041,8 +2041,8 @@ static DEFINE_PER_CPU(struct batched_entropy, 
> batched_entropy_u64);
>  u64 get_random_u64(void)
>  {
>   u64 ret;
> - bool use_lock = crng_init < 2;
> - unsigned long flags;
> + bool use_lock = READ_ONCE(crng_init) < 2;

Are use about that? I am not sure that the gcc will inline "crng_init"
read twice. It is not a local variable. READ_ONCE() is usually used
where gcc could cache a memory access but you do not want this. But hey!
If someone knows better I am here to learn.
One thing that this change does for sure is that crng_init is read very
early in the function while without READ_ONCE it is delayed _after_
arch_get_random_XXX(). So if arch_get_random_XXX() is around and works
you have one read for nothing.

> + unsigned long flags = 0;
>   struct batched_entropy *batch;
>  
>  #if BITS_PER_LONG == 64

Sebastian

[PATCH 1/1] cavium: Add firmware for CNN55XX crypto driver.

2017-06-16 Thread Srikanth Jampala 
This patchset adds the firmware for CNN55XX cryto driver,
supports Symmetric crypto operations.

The version of the firmware is v07.

Signed-off-by: Srikanth Jampala 
---
 WHENCE|   9 +
 cnn55xx_se.fw | Bin 0 -> 27698 bytes
 2 files changed, 9 insertions(+)
 create mode 100644 cnn55xx_se.fw

diff --git a/WHENCE b/WHENCE
index fafb1ce..a3142f0 100644
--- a/WHENCE
+++ b/WHENCE
@@ -3257,6 +3257,15 @@ Licence: Redistributable. See LICENCE.cavium for details
 
 --
 
+Driver: nitrox -- Cavium CNN55XX crypto driver
+
+File: cnn55xx_se.fw
+Version: v07
+
+Licence: Redistributable. See LICENCE.cavium for details
+
+--
+
 Driver: i915 -- Intel Integrated Graphics driver
 
 File: i915/skl_dmc_ver1_23.bin
diff --git a/cnn55xx_se.fw b/cnn55xx_se.fw
new file mode 100644
index 
..076e270d383488e7ae67e8f4224a519c4173bedc
GIT binary patch
literal 27698
zcmb?@30zZG_V7!9yhlg~BDhuC5Qrop>>{11w9i_n(&?hOiwh_yD2pQC(pDb{C@NO1
zXlX~=0HWCl)TND6wSfe1V^cu1DoI$a2(Hyu)bHH)q}u7s{QuwY`{0-R-o59p=bn4d
za?gFNNgsX~w$}fHN(`%j<#zW>yyprCNx!3<~xutZ?69AS%_hL02#`7mbUogHHLK0wq?k7|3o#ZH

[PATCH 0/1] Add firmware for CNN55XX crypto driver

2017-06-16 Thread Srikanth Jampala 
CNN55XX driver is accepted and needs firmware to functional.
This patchset adds CNN55XX firmware v07 supports
symmetric crypto operations.

Srikanth Jampala (1):
  cavium: Add firmware for CNN55XX crypto driver.

 WHENCE|   9 +
 cnn55xx_se.fw | Bin 0 -> 27698 bytes
 2 files changed, 9 insertions(+)
 create mode 100644 cnn55xx_se.fw

-- 
2.9.4

Re: Crypto Fixes for 4.12

2017-06-16 Thread Theodore Ts'o 
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: [PATCH v5 01/13] random: invalidate batched entropy after crng init

2017-06-16 Thread Jason A. Donenfeld 
On Fri, Jun 16, 2017 at 10:31 AM, Sebastian Andrzej Siewior
 wrote:
> am talking about. Again:

I actually figured that out myself after sending the initial email, so
then I wrote a follow-up patch which I attached to this thread. You
should have received it. Can you take a look?

https://lkml.org/lkml/2017/6/14/942

[PATCH v2 2/6] crypto: aes - refactor shared routines into separate core module

2017-06-16 Thread Ard Biesheuvel 
In preparation of further refactoring and cleanup of the AES code, move
the implementations of crypto_aes_expand_key() and crypto_aes_set_key()
into a separate module called aes_core, along with the Sboxes and the
GF(2^8) routines that they rely on.

Also, introduce crypto_aes_[en|de]crypt() based on the fixed time code,
which will be used in future patches by time invariant SIMD drivers that
may need to fallback to scalar code in exceptional circumstances. These
fallbacks offer a different tradeoff between time invariance and speed,
but are generally more appropriate due to the smaller size and cache
footprint.

Signed-off-by: Ard Biesheuvel 
---
 arch/arm/crypto/Kconfig   |   2 +-
 arch/arm64/crypto/Kconfig |   2 +-
 crypto/Kconfig|   5 +
 crypto/Makefile   |   1 +
 crypto/aes_core.c | 333 
 crypto/aes_generic.c  | 178 ---
 crypto/aes_ti.c   | 305 ++
 drivers/crypto/Kconfig|   8 +-
 include/crypto/aes.h  |   6 +
 9 files changed, 374 insertions(+), 466 deletions(-)

diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index b9adedcc5b2e..fd77aebcb7a9 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -73,7 +73,7 @@ config CRYPTO_AES_ARM_BS
depends on KERNEL_MODE_NEON
select CRYPTO_BLKCIPHER
select CRYPTO_SIMD
-   select CRYPTO_AES
+   select CRYPTO_AES_CORE
help
  Use a faster and more secure NEON based implementation of AES in CBC,
  CTR and XTS modes
diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
index d92293747d63..db55e069c17b 100644
--- a/arch/arm64/crypto/Kconfig
+++ b/arch/arm64/crypto/Kconfig
@@ -68,7 +68,7 @@ config CRYPTO_AES_ARM64_NEON_BLK
tristate "AES in ECB/CBC/CTR/XTS modes using NEON instructions"
depends on ARM64 && KERNEL_MODE_NEON
select CRYPTO_BLKCIPHER
-   select CRYPTO_AES
+   select CRYPTO_AES_CORE
select CRYPTO_SIMD
 
 config CRYPTO_CHACHA20_NEON
diff --git a/crypto/Kconfig b/crypto/Kconfig
index caa770e535a2..b4edea2aed22 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -894,9 +894,13 @@ config CRYPTO_GHASH_CLMUL_NI_INTEL
 
 comment "Ciphers"
 
+config CRYPTO_AES_CORE
+   tristate
+
 config CRYPTO_AES
tristate "AES cipher algorithms"
select CRYPTO_ALGAPI
+   select CRYPTO_AES_CORE
help
  AES cipher algorithms (FIPS-197). AES uses the Rijndael
  algorithm.
@@ -917,6 +921,7 @@ config CRYPTO_AES
 config CRYPTO_AES_TI
tristate "Fixed time AES cipher"
select CRYPTO_ALGAPI
+   select CRYPTO_AES_CORE
help
  This is a generic implementation of AES that attempts to eliminate
  data dependent latencies as much as possible without affecting
diff --git a/crypto/Makefile b/crypto/Makefile
index d41f0331b085..0979ca461ddb 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -96,6 +96,7 @@ obj-$(CONFIG_CRYPTO_TWOFISH) += twofish_generic.o
 obj-$(CONFIG_CRYPTO_TWOFISH_COMMON) += twofish_common.o
 obj-$(CONFIG_CRYPTO_SERPENT) += serpent_generic.o
 CFLAGS_serpent_generic.o := $(call cc-option,-fsched-pressure)  # 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79149
+obj-$(CONFIG_CRYPTO_AES_CORE) += aes_core.o
 obj-$(CONFIG_CRYPTO_AES) += aes_generic.o
 obj-$(CONFIG_CRYPTO_AES_TI) += aes_ti.o
 obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia_generic.o
diff --git a/crypto/aes_core.c b/crypto/aes_core.c
new file mode 100644
index ..3f3d1f2c813e
--- /dev/null
+++ b/crypto/aes_core.c
@@ -0,0 +1,333 @@
+/*
+ * Shared AES primitives for accelerated and generic implementations
+ *
+ * Copyright (C) 2017 Linaro Ltd 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+static const u8 __cacheline_aligned aes_sbox[] = {
+   0x63, 0x7c, 0x77, 0x7b, 0xf2, 0x6b, 0x6f, 0xc5,
+   0x30, 0x01, 0x67, 0x2b, 0xfe, 0xd7, 0xab, 0x76,
+   0xca, 0x82, 0xc9, 0x7d, 0xfa, 0x59, 0x47, 0xf0,
+   0xad, 0xd4, 0xa2, 0xaf, 0x9c, 0xa4, 0x72, 0xc0,
+   0xb7, 0xfd, 0x93, 0x26, 0x36, 0x3f, 0xf7, 0xcc,
+   0x34, 0xa5, 0xe5, 0xf1, 0x71, 0xd8, 0x31, 0x15,
+   0x04, 0xc7, 0x23, 0xc3, 0x18, 0x96, 0x05, 0x9a,
+   0x07, 0x12, 0x80, 0xe2, 0xeb, 0x27, 0xb2, 0x75,
+   0x09, 0x83, 0x2c, 0x1a, 0x1b, 0x6e, 0x5a, 0xa0,
+   0x52, 0x3b, 0xd6, 0xb3, 0x29, 0xe3, 0x2f, 0x84,
+   0x53, 0xd1, 0x00, 0xed, 0x20, 0xfc, 0xb1, 0x5b,
+   0x6a, 0xcb, 0xbe, 0x39, 0x4a, 0x4c, 0x58, 0xcf,
+   0xd0, 0xef, 0xaa, 0xfb, 0x43, 0x4d, 0x33, 0x85,
+   0x45, 0xf9, 0x02, 0x7f, 0x50, 0x3c, 0x9f, 0xa8,
+   0x51, 0xa3, 0x40, 0x8f, 0x92, 0x9d, 0x38, 0xf5,
+   0xbc, 0xb6, 0xda, 0x21, 0x10, 0xff, 0xf3, 0xd2,
+   0xcd, 0x0c, 0x13, 0xec,

[PATCH v2 6/6] crypto: aes - allow generic AES to be replaced by fixed time AES

2017-06-16 Thread Ard Biesheuvel 
On systems where a small memory footprint is important, the generic
AES code with its 16 KB of lookup tables and fully unrolled encrypt
and decrypt routines may be an unnecessary burden, especially given
that modern SoCs often have dedicated instructions for AES. And even
if they don't, a time invariant implementation may be preferred over
a fast one that may be susceptible to cache timing attacks.

So allow the declared dependency of other subsystems on AES to be
fulfilled by either the generic AES or the much smaller time invariant
implementation.

Signed-off-by: Ard Biesheuvel 
---
 crypto/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index f33c0d9136cf..2958120cdef3 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -899,12 +899,14 @@ config CRYPTO_AES_CORE
 
 config CRYPTO_AES
tristate
-   select CRYPTO_AES_GENERIC
+   select CRYPTO_AES_GENERIC if (CRYPTO_AES=y && CRYPTO_AES_TI != y) || \
+(CRYPTO_AES=m && !CRYPTO_AES_TI)
 
 config CRYPTO_AES_GENERIC
tristate "AES cipher algorithms"
-- 
2.7.4

[PATCH v2 3/6] crypto: x86/aes-ni - switch to generic fallback

2017-06-16 Thread Ard Biesheuvel 
The time invariant AES-NI implementation is SIMD based, and so it needs
a fallback in case the code is called from a context where SIMD is not
allowed. On x86, this is really only when executing in the context of an
interrupt taken while in kernel mode, since SIMD is allowed in all other
cases.

There is very little code in the kernel that actually performs AES in
interrupt context, and the code that does (mac80211) only does so when
using 802.11 devices that have no support for AES in hardware, and those
are rare these days.

So switch to the new AES core code as a fallback. It is much smaller, as
well as more resistant to cache timing attacks, and removing the
dependency allows us to disable the time variant drivers altogether if
desired.

Cc: Johannes Berg 
Signed-off-by: Ard Biesheuvel 
---
 arch/x86/crypto/aesni-intel_glue.c | 4 ++--
 crypto/Kconfig | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c 
b/arch/x86/crypto/aesni-intel_glue.c
index 4a55cdcdc008..1734e6185800 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -334,7 +334,7 @@ static void aes_encrypt(struct crypto_tfm *tfm, u8 *dst, 
const u8 *src)
struct crypto_aes_ctx *ctx = aes_ctx(crypto_tfm_ctx(tfm));
 
if (!irq_fpu_usable())
-   crypto_aes_encrypt_x86(ctx, dst, src);
+   crypto_aes_encrypt(ctx, dst, src);
else {
kernel_fpu_begin();
aesni_enc(ctx, dst, src);
@@ -347,7 +347,7 @@ static void aes_decrypt(struct crypto_tfm *tfm, u8 *dst, 
const u8 *src)
struct crypto_aes_ctx *ctx = aes_ctx(crypto_tfm_ctx(tfm));
 
if (!irq_fpu_usable())
-   crypto_aes_decrypt_x86(ctx, dst, src);
+   crypto_aes_decrypt(ctx, dst, src);
else {
kernel_fpu_begin();
aesni_dec(ctx, dst, src);
diff --git a/crypto/Kconfig b/crypto/Kconfig
index b4edea2aed22..1e6e021fda10 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -984,8 +984,7 @@ config CRYPTO_AES_NI_INTEL
tristate "AES cipher algorithms (AES-NI)"
depends on X86
select CRYPTO_AEAD
-   select CRYPTO_AES_X86_64 if 64BIT
-   select CRYPTO_AES_586 if !64BIT
+   select CRYPTO_AES_CORE
select CRYPTO_ALGAPI
select CRYPTO_BLKCIPHER
select CRYPTO_GLUE_HELPER_X86 if 64BIT
-- 
2.7.4

[PATCH v2 1/6] drivers/crypto/Kconfig: drop bogus CRYPTO_AES dependencies

2017-06-16 Thread Ard Biesheuvel 
In preparation of fine tuning the dependency relations between the
accelerated AES drivers and the core support code, let's remove the
dependency declarations that are false. None of these modules have
link time dependencies on the generic AES code, nor do they declare
any AES algos with CRYPTO_ALG_NEED_FALLBACK, so they can function
perfectly fine without crypto/aes_generic.o loaded.

Signed-off-by: Ard Biesheuvel 
---
 drivers/crypto/Kconfig | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 0528a62a39a6..7a737c1c669e 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -419,7 +419,6 @@ config CRYPTO_DEV_S5P
tristate "Support for Samsung S5PV210/Exynos crypto accelerator"
depends on ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
depends on HAS_IOMEM && HAS_DMA
-   select CRYPTO_AES
select CRYPTO_BLKCIPHER
help
  This option allows you to have support for S5P crypto acceleration.
@@ -473,7 +472,6 @@ config CRYPTO_DEV_ATMEL_AES
tristate "Support for Atmel AES hw accelerator"
depends on HAS_DMA
depends on ARCH_AT91 || COMPILE_TEST
-   select CRYPTO_AES
select CRYPTO_AEAD
select CRYPTO_BLKCIPHER
help
@@ -591,7 +589,6 @@ config CRYPTO_DEV_SUN4I_SS
depends on ARCH_SUNXI && !64BIT
select CRYPTO_MD5
select CRYPTO_SHA1
-   select CRYPTO_AES
select CRYPTO_DES
select CRYPTO_BLKCIPHER
help
@@ -606,7 +603,6 @@ config CRYPTO_DEV_SUN4I_SS
 config CRYPTO_DEV_ROCKCHIP
tristate "Rockchip's Cryptographic Engine driver"
depends on OF && ARCH_ROCKCHIP
-   select CRYPTO_AES
select CRYPTO_DES
select CRYPTO_MD5
select CRYPTO_SHA1
@@ -622,7 +618,6 @@ config CRYPTO_DEV_MEDIATEK
tristate "MediaTek's EIP97 Cryptographic Engine driver"
depends on HAS_DMA
depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST
-   select CRYPTO_AES
select CRYPTO_AEAD
select CRYPTO_BLKCIPHER
select CRYPTO_CTR
-- 
2.7.4

[PATCH v2 5/6] crypto: aes - add meaningful help text to the various AES drivers

2017-06-16 Thread Ard Biesheuvel 
Remove the duplicated boilerplate help text and add a bit of explanation
about the nature of the various AES implementations that exist for ARM
and x86.

Signed-off-by: Ard Biesheuvel 
---
 arch/arm/crypto/Kconfig   |  4 +-
 arch/arm64/crypto/Kconfig |  7 ++
 crypto/Kconfig| 68 +++-
 3 files changed, 18 insertions(+), 61 deletions(-)

diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index 3a6994ada2d1..24d70d74ae51 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -66,7 +66,9 @@ config CRYPTO_AES_ARM
select CRYPTO_ALGAPI
select CRYPTO_AES_GENERIC
help
- Use optimized AES assembler routines for ARM platforms.
+ Use optimized AES assembler routines for ARM platforms. This
+ implementation is table based, and thus not time invariant.
+ It reuses the tables exposed by the generic AES driver.
 
 config CRYPTO_AES_ARM_BS
tristate "Bit sliced AES using NEON instructions"
diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
index 7ffe88267943..48404ae2a11a 100644
--- a/arch/arm64/crypto/Kconfig
+++ b/arch/arm64/crypto/Kconfig
@@ -44,11 +44,18 @@ config CRYPTO_CRC32_ARM64_CE
 config CRYPTO_AES_ARM64
tristate "AES core cipher using scalar instructions"
select CRYPTO_AES_GENERIC
+   help
+ Use optimized AES assembler routines for ARM platforms. This
+ implementation is table based, and thus not time invariant.
+ It reuses the tables exposed by the generic AES driver.
 
 config CRYPTO_AES_ARM64_CE
tristate "AES core cipher using ARMv8 Crypto Extensions"
depends on ARM64 && KERNEL_MODE_NEON
select CRYPTO_ALGAPI
+   help
+ Assembler implementation for arm64 of AES using special dedicated
+ instructions. This implementation is time invariant.
 
 config CRYPTO_AES_ARM64_CE_CCM
tristate "AES in CCM mode using ARMv8 Crypto Extensions"
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 9ae3dade4b2b..f33c0d9136cf 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -906,21 +906,10 @@ config CRYPTO_AES_GENERIC
select CRYPTO_ALGAPI
select CRYPTO_AES_CORE
help
- AES cipher algorithms (FIPS-197). AES uses the Rijndael
- algorithm.
-
- Rijndael appears to be consistently a very good performer in
- both hardware and software across a wide range of computing
- environments regardless of its use in feedback or non-feedback
- modes. Its key setup time is excellent, and its key agility is
- good. Rijndael's very low memory requirements make it very well
- suited for restricted-space environments, in which it also
- demonstrates excellent performance. Rijndael's operations are
- among the easiest to defend against power and timing attacks.
-
- The AES specifies three key sizes: 128, 192 and 256 bits
-
- See  for more information.
+ Generic table based implementation of AES. This is the fastest
+ implementation in C, but may be susceptible to known plaintext
+ attacks on the key due to the correlation between the processing
+ time and the input of the first round.
 
 config CRYPTO_AES_TI
tristate "Fixed time AES cipher"
@@ -946,44 +935,18 @@ config CRYPTO_AES_586
select CRYPTO_ALGAPI
select CRYPTO_AES_GENERIC
help
- AES cipher algorithms (FIPS-197). AES uses the Rijndael
+ Assembler implementation for 32-bit x86 of the table based AES
  algorithm.
 
- Rijndael appears to be consistently a very good performer in
- both hardware and software across a wide range of computing
- environments regardless of its use in feedback or non-feedback
- modes. Its key setup time is excellent, and its key agility is
- good. Rijndael's very low memory requirements make it very well
- suited for restricted-space environments, in which it also
- demonstrates excellent performance. Rijndael's operations are
- among the easiest to defend against power and timing attacks.
-
- The AES specifies three key sizes: 128, 192 and 256 bits
-
- See  for more information.
-
 config CRYPTO_AES_X86_64
tristate "AES cipher algorithms (x86_64)"
depends on (X86 || UML_X86) && 64BIT
select CRYPTO_ALGAPI
select CRYPTO_AES_GENERIC
help
- AES cipher algorithms (FIPS-197). AES uses the Rijndael
+ Assembler implementation for 64-bit x86 of the table based AES
  algorithm.
 
- Rijndael appears to be consistently a very good performer in
- both hardware and software across a wide range of computing
- environments regardless of its use in feedback or non-feedback
- modes.

[PATCH v2 4/6] crypto: aes - repurpose CRYPTO_AES and introduce CRYPTO_AES_GENERIC

2017-06-16 Thread Ard Biesheuvel 
Repurpose the Kconfig symbol CRYPTO_AES to signify that a 'select' or
'depends on' relationship on it can be satisfied by any driver that
exposes a "aes" cipher.

The existing generic AES code is now controlled by a new Kconfig symbol
CRYPTO_AES_GENERIC, and only dependencies on CRYPTO_AES that truly depend
on its exported lookup tables are updated accordingly.

Signed-off-by: Ard Biesheuvel 
---
 arch/arm/crypto/Kconfig   | 2 +-
 arch/arm64/crypto/Kconfig | 2 +-
 crypto/Kconfig| 8 ++--
 crypto/Makefile   | 2 +-
 net/sunrpc/Kconfig| 3 ++-
 5 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index fd77aebcb7a9..3a6994ada2d1 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -64,7 +64,7 @@ config CRYPTO_SHA512_ARM
 config CRYPTO_AES_ARM
tristate "Scalar AES cipher for ARM"
select CRYPTO_ALGAPI
-   select CRYPTO_AES
+   select CRYPTO_AES_GENERIC
help
  Use optimized AES assembler routines for ARM platforms.
 
diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
index db55e069c17b..7ffe88267943 100644
--- a/arch/arm64/crypto/Kconfig
+++ b/arch/arm64/crypto/Kconfig
@@ -43,7 +43,7 @@ config CRYPTO_CRC32_ARM64_CE
 
 config CRYPTO_AES_ARM64
tristate "AES core cipher using scalar instructions"
-   select CRYPTO_AES
+   select CRYPTO_AES_GENERIC
 
 config CRYPTO_AES_ARM64_CE
tristate "AES core cipher using ARMv8 Crypto Extensions"
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 1e6e021fda10..9ae3dade4b2b 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -898,6 +898,10 @@ config CRYPTO_AES_CORE
tristate
 
 config CRYPTO_AES
+   tristate
+   select CRYPTO_AES_GENERIC
+
+config CRYPTO_AES_GENERIC
tristate "AES cipher algorithms"
select CRYPTO_ALGAPI
select CRYPTO_AES_CORE
@@ -940,7 +944,7 @@ config CRYPTO_AES_586
tristate "AES cipher algorithms (i586)"
depends on (X86 || UML_X86) && !64BIT
select CRYPTO_ALGAPI
-   select CRYPTO_AES
+   select CRYPTO_AES_GENERIC
help
  AES cipher algorithms (FIPS-197). AES uses the Rijndael
  algorithm.
@@ -962,7 +966,7 @@ config CRYPTO_AES_X86_64
tristate "AES cipher algorithms (x86_64)"
depends on (X86 || UML_X86) && 64BIT
select CRYPTO_ALGAPI
-   select CRYPTO_AES
+   select CRYPTO_AES_GENERIC
help
  AES cipher algorithms (FIPS-197). AES uses the Rijndael
  algorithm.
diff --git a/crypto/Makefile b/crypto/Makefile
index 0979ca461ddb..73395307bcea 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -97,7 +97,7 @@ obj-$(CONFIG_CRYPTO_TWOFISH_COMMON) += twofish_common.o
 obj-$(CONFIG_CRYPTO_SERPENT) += serpent_generic.o
 CFLAGS_serpent_generic.o := $(call cc-option,-fsched-pressure)  # 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79149
 obj-$(CONFIG_CRYPTO_AES_CORE) += aes_core.o
-obj-$(CONFIG_CRYPTO_AES) += aes_generic.o
+obj-$(CONFIG_CRYPTO_AES_GENERIC) += aes_generic.o
 obj-$(CONFIG_CRYPTO_AES_TI) += aes_ti.o
 obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia_generic.o
 obj-$(CONFIG_CRYPTO_CAST_COMMON) += cast_common.o
diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig
index ac09ca803296..58aa2ada40b3 100644
--- a/net/sunrpc/Kconfig
+++ b/net/sunrpc/Kconfig
@@ -19,7 +19,8 @@ config RPCSEC_GSS_KRB5
tristate "Secure RPC: Kerberos V mechanism"
depends on SUNRPC && CRYPTO
depends on CRYPTO_MD5 && CRYPTO_DES && CRYPTO_CBC && CRYPTO_CTS
-   depends on CRYPTO_ECB && CRYPTO_HMAC && CRYPTO_SHA1 && CRYPTO_AES
+   depends on CRYPTO_ECB && CRYPTO_HMAC && CRYPTO_SHA1
+   select CRYPTO_AES
depends on CRYPTO_ARC4
default y
select SUNRPC_GSS
-- 
2.7.4

[PATCH v2 0/6] crypto: aes - allow generic AES to be omitted

2017-06-16 Thread Ard Biesheuvel 
The generic AES driver uses 16 lookup tables of 1 KB each, and has
encryption and decryption routines that are fully unrolled. Given how
the dependencies between this code and other drivers are declared in
Kconfig files, this code is always pulled into the core kernel, even
if it is usually superseded at runtime by accelerated drivers that
exist for many architectures.

This leaves us with 25 KB of dead code in the kernel, which is negligible
in typical environments, but which is actually a big deal for the IoT
domain, where every kilobyte counts.

Also, the scalar, table based AES routines that exist for ARM, arm64, i586
and x86_64 share the lookup tables with AES generic, and may be invoked
occasionally when the time-invariant AES-NI or other special instruction
drivers are called in interrupt context, at which time the SIMD register
file cannot be used. Pulling 16 KB of code and 9 KB of instructions into
the L1s (and evicting what was already there) when a softirq happens to
be handled in the context of an interrupt taken from kernel mode (which
means no SIMD on x86) is also something that we may like to avoid, by
falling back to a much smaller and moderately less performant driver.
(Note that arm64 will be updated shortly to supply fallbacks for all
SIMD based AES implementations, which will be based on the core routines
[if they are accepted].)

For the reasons above, this series refactors the way the various AES
implementations are wired up, to allow the generic version in
crypto/aes_generic.c to be omitted from the build entirely.

Patch #1 removes some bogus 'select CRYPTO_AES' statement.

Patch #2 introduces CRYPTO_AES_CORE and its implementation crypto/aes_core.c,
which contains the existing key expansion routines, and default encrypt and
decrypt routines that are not exposed as a crypto_cipher themselves, but
can be pulled in by other AES drivers. These routines only depend on the two
256 byte Sboxes

Patch #3 switches the fallback in the AES-NI code to the new, generic encrypt
and decrypt routines so it no longer depends on the x86 scalar code or
[transitively] on AES-generic.

Patch #4 repurposes the CRYPTO_AES Kconfig symbol as an abstract symbol that
indicates whether some implementation of AES needs to be available. The
existing generic code is now controlled by CRYPTO_AES_GENERIC.

Patch #5 updates the Kconfig help text to be more descriptive of what they
actually control, rather than duplicating AES's wikipedia entry a number of
times.

Patch #6 updates the Kconfig logic so CRYPTO_AES_GENERIC can be disabled if
any CRYPTO_AES dependencies are satisfied by the fixed time driver.

v2: - repurpose CRYPTO_AES and avoid HAVE_AES/NEED_AES Kconfig symbols
- don't factor out tables from AES generic to be reused by per arch drivers,
  since the space saving is moderate (the generic code only), and the
  drivers weren't made to be small anyway

Ard Biesheuvel (6):
  drivers/crypto/Kconfig: drop bogus CRYPTO_AES dependencies
  crypto: aes - refactor shared routines into separate core module
  crypto: x86/aes-ni - switch to generic fallback
  crypto: aes - repurpose CRYPTO_AES and introduce CRYPTO_AES_GENERIC
  crypto: aes - add meaningful help text to the various AES drivers
  crypto: aes - allow generic AES to be replaced by fixed time AES

 arch/arm/crypto/Kconfig|   8 +-
 arch/arm64/crypto/Kconfig  |  11 +-
 arch/x86/crypto/aesni-intel_glue.c |   4 +-
 crypto/Kconfig |  85 ++---
 crypto/Makefile|   3 +-
 crypto/aes_core.c  | 333 
 crypto/aes_generic.c   | 178 ---
 crypto/aes_ti.c| 305 ++
 drivers/crypto/Kconfig |  13 +-
 include/crypto/aes.h   |   6 +
 net/sunrpc/Kconfig |   3 +-
 11 files changed, 407 insertions(+), 542 deletions(-)
 create mode 100644 crypto/aes_core.c

-- 
2.7.4

[PATCH] crypto: caam: make of_device_ids const.

2017-06-16 Thread Arvind Yadav 
of_device_ids are not supposed to change at runtime. All functions
working with of_device_ids provided by  work with const
of_device_ids. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
   2376 808 1283312 cf0 drivers/crypto/caam/jr.o

File size after constify caam_jr_match:
   textdata bss dec hex filename
   2976 192 1283296 ce0 drivers/crypto/caam/jr.o

Signed-off-by: Arvind Yadav 
---
 drivers/crypto/caam/jr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 2763100..1ccfb31 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -536,7 +536,7 @@ static int caam_jr_probe(struct platform_device *pdev)
return 0;
 }
 
-static struct of_device_id caam_jr_match[] = {
+static const struct of_device_id caam_jr_match[] = {
{
.compatible = "fsl,sec-v4.0-job-ring",
},
-- 
1.9.1

[PATCH] crypto: vmx: remove unnecessary check

2017-06-16 Thread Tudor Ambarus 
You can't reach init() if parent alg_name is invalid. Moreover,
cypto_alloc_base() will return ENOENT if alg_name is NULL.
Found while grasping the fallback mechanism.

Signed-off-by: Tudor Ambarus 
---
 drivers/crypto/vmx/aes.c | 7 +--
 drivers/crypto/vmx/aes_cbc.c | 7 +--
 drivers/crypto/vmx/aes_ctr.c | 7 +--
 drivers/crypto/vmx/aes_xts.c | 7 +--
 4 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/crypto/vmx/aes.c b/drivers/crypto/vmx/aes.c
index 022c7ab..96072b9 100644
--- a/drivers/crypto/vmx/aes.c
+++ b/drivers/crypto/vmx/aes.c
@@ -37,15 +37,10 @@ struct p8_aes_ctx {
 
 static int p8_aes_init(struct crypto_tfm *tfm)
 {
-   const char *alg;
+   const char *alg = crypto_tfm_alg_name(tfm);
struct crypto_cipher *fallback;
struct p8_aes_ctx *ctx = crypto_tfm_ctx(tfm);
 
-   if (!(alg = crypto_tfm_alg_name(tfm))) {
-   printk(KERN_ERR "Failed to get algorithm name.\n");
-   return -ENOENT;
-   }
-
fallback = crypto_alloc_cipher(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(fallback)) {
printk(KERN_ERR
diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
index 72a26eb..7394d35 100644
--- a/drivers/crypto/vmx/aes_cbc.c
+++ b/drivers/crypto/vmx/aes_cbc.c
@@ -39,15 +39,10 @@ struct p8_aes_cbc_ctx {
 
 static int p8_aes_cbc_init(struct crypto_tfm *tfm)
 {
-   const char *alg;
+   const char *alg = crypto_tfm_alg_name(tfm);
struct crypto_skcipher *fallback;
struct p8_aes_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
 
-   if (!(alg = crypto_tfm_alg_name(tfm))) {
-   printk(KERN_ERR "Failed to get algorithm name.\n");
-   return -ENOENT;
-   }
-
fallback = crypto_alloc_skcipher(alg, 0,
CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
 
diff --git a/drivers/crypto/vmx/aes_ctr.c b/drivers/crypto/vmx/aes_ctr.c
index 7cf6d31..9c26d9e 100644
--- a/drivers/crypto/vmx/aes_ctr.c
+++ b/drivers/crypto/vmx/aes_ctr.c
@@ -36,15 +36,10 @@ struct p8_aes_ctr_ctx {
 
 static int p8_aes_ctr_init(struct crypto_tfm *tfm)
 {
-   const char *alg;
+   const char *alg = crypto_tfm_alg_name(tfm);
struct crypto_blkcipher *fallback;
struct p8_aes_ctr_ctx *ctx = crypto_tfm_ctx(tfm);
 
-   if (!(alg = crypto_tfm_alg_name(tfm))) {
-   printk(KERN_ERR "Failed to get algorithm name.\n");
-   return -ENOENT;
-   }
-
fallback =
crypto_alloc_blkcipher(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(fallback)) {
diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
index 6adc929..8cd6e62 100644
--- a/drivers/crypto/vmx/aes_xts.c
+++ b/drivers/crypto/vmx/aes_xts.c
@@ -41,15 +41,10 @@ struct p8_aes_xts_ctx {
 
 static int p8_aes_xts_init(struct crypto_tfm *tfm)
 {
-   const char *alg;
+   const char *alg = crypto_tfm_alg_name(tfm);
struct crypto_skcipher *fallback;
struct p8_aes_xts_ctx *ctx = crypto_tfm_ctx(tfm);
 
-   if (!(alg = crypto_tfm_alg_name(tfm))) {
-   printk(KERN_ERR "Failed to get algorithm name.\n");
-   return -ENOENT;
-   }
-
fallback = crypto_alloc_skcipher(alg, 0,
CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(fallback)) {
-- 
2.7.4

Re: [PATCH v5 01/13] random: invalidate batched entropy after crng init

2017-06-16 Thread Sebastian Andrzej Siewior 
On 2017-06-15 00:33:12 [+0200], Jason A. Donenfeld wrote:
> There's a potential race that I fixed in my v5 of that patch set, but
> Ted only took v4, and for whatever reason has been to busy to submit
> the additional patch I already posted showing the diff between v4
> Hopefully he actually gets around to it and sends this for the next
> rc. Here it is:
> 
> https://patchwork.kernel.org/patch/9774563/

So you replace "crng_init < 2" with use_lock instead. That is not what I
am talking about. Again:
add_interrupt_randomness()
->   crng_fast_load()   
spin_trylock_irqsave(_crng.lock, )
->invalidate_batched_entropy()  
write_lock_irqsave(_entropy_reset_lock, );

in that order while the code path
get_random_uXX()
read_lock_irqsave(_entropy_reset_lock, );
->   extract_crng()
->_extract_crng()spin_lock_irqsave(>lock, );

which allocates the same lock in opposite order.
That means
  T1T2
  crng_fast_load()  get_random_u64()
extract_crng()
  *dead lock*
invalidate_batched_entropy()
_extract_crng()

So T1 waits for batched_entropy_reset_lock holding primary_crng.lock and
T2 waits for primary_crng.lock holding batched_entropy_reset_lock.

Sebastian

Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver

2017-06-16 Thread Herbert Xu 
On Fri, Jun 16, 2017 at 07:57:00AM +, Horia Geantă wrote:
>
> Commit 0605c41cc53ca ("crypto: cts - Convert to skcipher") appends
> CRYPTO_TFM_REQ_MAY_BACKLOG to the original crypto request flags for the
> last block - when calling cts_cbc_encrypt().
> Is it really needed?

Yes, because at this point we cannot tell the sender to back off.

> For cts(cbc(aes)) with cbc(aes) offloaded in HW, i.e. running in async
> mode, we get the below stack for CAAM driver.
> Driver is told that it can sleep (CRYPTO_TFM_REQ_MAY_BACKLOG flag), so
> it uses GFP_KERNEL to allocate memory. However, this is incorrect, since
> driver runs in atomic context (softirq).

This is wrong.  Whether you can sleep or not is determined by
MAY_SLEEP, not MAY_BACKLOG.  MAY_BACKLOG only indicates that this
request must be queued, even if the queue is full.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver

2017-06-16 Thread Horia Geantă 
On 6/15/2017 5:57 PM, Horia Geantă wrote:
> On 6/2/2017 3:25 PM, David Gstir wrote:
>> Hi!
>>
>> While testing fscrypt's filename encryption, I noticed that the 
>> implementation
>> of cts(cbc(aes)) is broken when the CAAM hardware crypto driver is enabled.
>> Some digging showed that the refactoring of crypto/cts.c in v4.8 
>> (commit 0605c41cc53ca) exposed some problems with CAAM's aes-cbc
>> implementation. This can be tested quite easily by loading the tcrypt
>> module with mode=38. Looks like the cts mode is not used very often
>> and this went unnoticed since 4.8...
>>
>> This patch series is an attempt to fix that and get cts(cbc(aes)) working
>> properly again when the CAAM driver is enabled.
>>
> David, thanks for taking time to fix these.
> 
>> Specifically, the issues are:
[snip]
>> 2) The cts implementation uses aes-cbc twice to perform its job. The second
>>time, it is called from within the callback of the first call to aes-cbc.
>>This usage is not properly handled in the CAAM driver which triggers the
>>BUG below.
>>
> Dan, Radu - have you seen this on i.MX?
> 
>>My current proposal is to use in_interrupt() to detect such cases and set
>>the k*alloc flags accordingly. However, since using in_interrupt() is not
>>really nice, I'm wondering if there is a better way to handle this?
>>
> Nothing else crosses my mind right now, but I'd like to sleep on it.
> 
Herbert,

Commit 0605c41cc53ca ("crypto: cts - Convert to skcipher") appends
CRYPTO_TFM_REQ_MAY_BACKLOG to the original crypto request flags for the
last block - when calling cts_cbc_encrypt().
Is it really needed?

For cts(cbc(aes)) with cbc(aes) offloaded in HW, i.e. running in async
mode, we get the below stack for CAAM driver.
Driver is told that it can sleep (CRYPTO_TFM_REQ_MAY_BACKLOG flag), so
it uses GFP_KERNEL to allocate memory. However, this is incorrect, since
driver runs in atomic context (softirq).

I wouldn't say that:
-David's suggestion - adding in_interrupt()
- or in general: trying to detect in CAAM driver whether we are in
atomic context using anything else than what crypto API provides
(MAY_BACKLOG, MAY_SLEEP flags)

is something we want to do.
IIUC, in general caller (cts / crypto API) is responsible for telling
the callee if it will run in atomic context or not:
https://lwn.net/Articles/274695/

Thanks,
Horia

>> 
>> [  126.252543] BUG: sleeping function called from invalid context at 
>> mm/slab.h:432
>> [  126.260099] in_atomic(): 1, irqs_disabled(): 0, pid: 0, name: swapper/0
>> [  126.266837] no locks held by swapper/0/0.
>> [  126.270969] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
>> 4.12.0-rc3-00052-g0ddec680d395 #287
>> [  126.279226] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
>> [  126.285821] Backtrace:
>> [  126.288406] [] (dump_backtrace) from [] 
>> (show_stack+0x18/0x1c)
>> [  126.296057]  r7: r6:6113 r5: r4:c102ab48
>> [  126.301798] [] (show_stack) from [] 
>> (dump_stack+0xb4/0xe8)
>> [  126.309106] [] (dump_stack) from [] 
>> (___might_sleep+0x17c/0x2a0)
>> [  126.316929]  r9: r8:c0a016dc r7:c101ee3e r6:01b0 r5:c0c12fac 
>> r4:e000
>> [  126.324751] [] (___might_sleep) from [] 
>> (__might_sleep+0x64/0xa4)
>> [  126.332655]  r7:014080c1 r6: r5:01b0 r4:c0c12fac
>> [  126.338397] [] (__might_sleep) from [] 
>> (__kmalloc+0x130/0x1b8)
>> [  126.346039]  r6:c071a8ec r5:014080c1 r4:eec01e00
>> [  126.350742] [] (__kmalloc) from [] 
>> (ablkcipher_edesc_alloc.constprop.1+0x200/0x900)
>> [  126.360213]  r10: r9: r8:c0a016dc r7:c0a016dc r6:ee05ac10 
>> r5:edfdaec0
>> [  126.368109]  r4:0001 r3:0020
>> [  126.371765] [] (ablkcipher_edesc_alloc.constprop.1) from 
>> [] (ablkcipher_encrypt+0x24/0x9c)
>> [  126.381843]  r10: r9:0020 r8:0001 r7:ee05ac10 r6:ed597000 
>> r5:edfdaec0
>> [  126.389738]  r4:ed475d08
>> [  126.392354] [] (ablkcipher_encrypt) from [] 
>> (skcipher_encrypt_ablkcipher+0x68/0x6c)
>> [  126.401822]  r7:ed475d08 r6:ed597000 r5:0400 r4:ed475d08
>> [  126.407566] [] (skcipher_encrypt_ablkcipher) from [] 
>> (cts_cbc_encrypt+0x118/0x124)
>> [  126.416947]  r7:ed475d08 r6:c1001cc0 r5:0010 r4:edfdae00
>> [  126.422686] [] (cts_cbc_encrypt) from [] 
>> (crypto_cts_encrypt_done+0x20/0x54)
>> [  126.431548]  r10: r9:ee05ac10 r8: r7:0010 r6:edc8e6c0 
>> r5:edc8e6d8
>> [  126.439443]  r4:edfdae00
>> [  126.442056] [] (crypto_cts_encrypt_done) from [] 
>> (ablkcipher_encrypt_done+0x88/0x9c)
>> [  126.445180] fec 2188000.ethernet eth0: MDIO read timeout
>> [  126.456948]  r5:edc8e6d8 r4:edfdaec0
>> [  126.460604] [] (ablkcipher_encrypt_done) from [] 
>> (caam_jr_dequeue+0x214/0x2d4)
>> [  126.469639]  r9:0001 r8:ee088010 r7:01ff r6:0001 r5: 
>> r4:edfdaec0
>> [  126.477467] [] (caam_jr_dequeue) from [] 
>> (tasklet_action+0x98/0x154)
>> [  126.485160] fec 2188000.ethernet eth0: MDIO read timeout
>> [  126.490975]  r10:c1080b80