Re: [PATCH v2] tpm: use struct tpm_chip for tpm_chip_find_get()

[Jason Gunthorpe]

On Wed, Oct 25, 2017 at 10:07:46PM +0200, Jarkko Sakkinen wrote:

> The id has a nice feature that it is unique for one boot cycle you can
> even try to get a chip that has been deleted. It has the most stable
> properties in the long run.

It isn't unique, we can re-use ids them via idr_alloc(). We should
never use index inside the kernel.

> Address is a reusable identifier in one boot cycle.

It is invalid to pass in a chip for which the caller does not hold a
kref, so address is the safest argument.

Jason

Re: [PATCH v2] tpm: use struct tpm_chip for tpm_chip_find_get()

[Jarkko Sakkinen]

On Wed, Oct 25, 2017 at 01:46:33PM -0600, Jason Gunthorpe wrote:
> > struct tpm_chip *tpm_chip_find_get(u64 id)
> > {
> > struct tpm_chup *chip;
> > struct tpm_chip *res = NULL;
> > int chip_num = 0;
> > int chip_prev;
> > 
> > mutex_lock(_lock);
> > 
> > do {
> > chip_prev = chip_num;
> > 
> > chip = idr_get_next(_nums_idr, _num);
> > 
> > if (chip && (!id || id == chip->id) && !tpm_try_get_ops(chip)) {
> > res = chip;
> > break;
> > }
> > } while (chip_prev != chip_num);
> > 
> > mutex_unlock(_lock);
> > 
> > return res;
> > }
> 
> ?? The old version was correct, idr_find_slowpath is better than an
> idr_get_next serach if you already know id.
> 
> PrasannaKumar's solution seems right, if we already have chip, then we
> just need to lock it again:
> 
> struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
> {
>   struct tpm_chip *res = NULL;
> 
>   mutex_lock(_lock);
> 
>   if (!chip) {
>   int chip_num = 0;
>   int chip_prev;
> 
>   do {
>   chip_prev = chip_num;
>   chip = idr_get_next(_nums_idr, _num);
>   if (chip && !tpm_try_get_ops(chip)) {
>   res = chip;
>   break;
>   }
>   } while (chip_prev != chip_num);
>   } else {
>   if (!tpm_try_get_ops(chip))
>   res = chip;
>   }
> 
>   mutex_unlock(_lock);
> 
>   return res;
> }
> 
> Jason

The id has a nice feature that it is unique for one boot cycle you can
even try to get a chip that has been deleted. It has the most stable
properties in the long run.

Address is a reusable identifier in one boot cycle.

/Jarkko

Re: [PATCH] tpm: Move Linux RNG connection to hwrng

[Jason Gunthorpe]

On Wed, Oct 25, 2017 at 10:00:30PM +0200, Jarkko Sakkinen wrote:
> A minor tidbit: could the tpm_ prefix removed from those fields?

Yes, in v2

I will send v2 when you land your rework patch as this will depend on it.

Jason

Re: [PATCH] tpm: Move Linux RNG connection to hwrng

[Jarkko Sakkinen]

On Wed, Oct 25, 2017 at 01:37:07PM -0600, Jason Gunthorpe wrote:
> On Wed, Oct 25, 2017 at 08:58:17PM +0200, Jarkko Sakkinen wrote:
> > On Wed, Oct 25, 2017 at 08:15:09PM +0530, PrasannaKumar Muralidharan wrote:
> > > > +   if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
> > > > +   return 0;
> > > 
> > > Can #ifndef CONFIG_HW_RANDOM_TPM be used instead? That way an if
> > > condition can be avoided.
> > 
> > Nope. There is no reason to avoid the if-condition. Compiler will take
> > care of it. IS_ENABLED() macro is available just for the purpose Jason
> > is using it.
> > 
> > > > +   char tpm_hwrng_name[64];
> > > > +   struct hwrng tpm_hwrng;
> > > > +
> > > 
> > > Can this also be put inside the #ifdef?
> > 
> > Yes. It should be inside #ifdef.
> 
> Then we need #idefs in the .c code, IS_ENABLED is not enough :\ I
> don't think the few bytes matters enough to bother.
> 
> Jason

I'll buy that!

A minor tidbit: could the tpm_ prefix removed from those fields?

/Jarkko

Re: [PATCH v2] tpm: use struct tpm_chip for tpm_chip_find_get()

[Jason Gunthorpe]

> struct tpm_chip *tpm_chip_find_get(u64 id)
> {
>   struct tpm_chup *chip;
>   struct tpm_chip *res = NULL;
>   int chip_num = 0;
>   int chip_prev;
> 
>   mutex_lock(_lock);
> 
>   do {
>   chip_prev = chip_num;
> 
>   chip = idr_get_next(_nums_idr, _num);
> 
>   if (chip && (!id || id == chip->id) && !tpm_try_get_ops(chip)) {
>   res = chip;
>   break;
>   }
>   } while (chip_prev != chip_num);
> 
>   mutex_unlock(_lock);
> 
>   return res;
> }

?? The old version was correct, idr_find_slowpath is better than an
idr_get_next serach if you already know id.

PrasannaKumar's solution seems right, if we already have chip, then we
just need to lock it again:

struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
{
struct tpm_chip *res = NULL;

mutex_lock(_lock);

if (!chip) {
int chip_num = 0;
int chip_prev;

do {
chip_prev = chip_num;
chip = idr_get_next(_nums_idr, _num);
if (chip && !tpm_try_get_ops(chip)) {
res = chip;
break;
}
} while (chip_prev != chip_num);
} else {
if (!tpm_try_get_ops(chip))
res = chip;
}

mutex_unlock(_lock);

return res;
}

Jason

Re: [PATCH] tpm: Move Linux RNG connection to hwrng

[Jason Gunthorpe]

On Wed, Oct 25, 2017 at 08:58:17PM +0200, Jarkko Sakkinen wrote:
> On Wed, Oct 25, 2017 at 08:15:09PM +0530, PrasannaKumar Muralidharan wrote:
> > > +   if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
> > > +   return 0;
> > 
> > Can #ifndef CONFIG_HW_RANDOM_TPM be used instead? That way an if
> > condition can be avoided.
> 
> Nope. There is no reason to avoid the if-condition. Compiler will take
> care of it. IS_ENABLED() macro is available just for the purpose Jason
> is using it.
> 
> > > +   char tpm_hwrng_name[64];
> > > +   struct hwrng tpm_hwrng;
> > > +
> > 
> > Can this also be put inside the #ifdef?
> 
> Yes. It should be inside #ifdef.

Then we need #idefs in the .c code, IS_ENABLED is not enough :\ I
don't think the few bytes matters enough to bother.

Jason

Re: [PATCH v2] tpm: use struct tpm_chip for tpm_chip_find_get()

[Jarkko Sakkinen]

On Wed, Oct 25, 2017 at 08:40:26PM +0530, PrasannaKumar Muralidharan wrote:
> > -struct tpm_chip *tpm_chip_find_get(int chip_num)
> > +struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
> >  {
> > -   struct tpm_chip *chip, *res = NULL;
> > +   struct tpm_chip *res = NULL;
> > +   int chip_num = 0;
> > int chip_prev;
> >
> > mutex_lock(_lock);
> >
> > -   if (chip_num == TPM_ANY_NUM) {
> > -   chip_num = 0;
> > +   if (!chip) {
> > do {
> > chip_prev = chip_num;
> > chip = idr_get_next(_nums_idr, _num);
> 
> When chip is not NULL just do tpm_try_get_ops(chip). Current code does
> more things which are not required.

Your observation is right that there is something wrong but conclusions
are incorrect.

It's actually a regression.

If @chip has a value, the code does one iteration of what it is doing in
the first branch of the condition. That is completely bogus semantics to
say the least.

To sort that out I'll introduce a new field to struct tpm_chip:

  u64 id;

This gets a value from a global count every time a chip is created.

The function will become then:

struct tpm_chip *tpm_chip_find_get(u64 id)
{
struct tpm_chup *chip;
struct tpm_chip *res = NULL;
int chip_num = 0;
int chip_prev;

mutex_lock(_lock);

do {
chip_prev = chip_num;

chip = idr_get_next(_nums_idr, _num);

if (chip && (!id || id == chip->id) && !tpm_try_get_ops(chip)) {
res = chip;
break;
}
} while (chip_prev != chip_num);

mutex_unlock(_lock);

return res;
}

Thanks for spotting this out. I'll refine the patch.

/Jarkko

Re: [tpmdd-devel] [PATCH] tpm: remove chip_num parameter from in-kernel API

[Jarkko Sakkinen]

On Wed, Oct 25, 2017 at 08:21:16PM +0530, PrasannaKumar Muralidharan wrote:
> >> > 2. Moving struct tpm_rng to the TPM client is architecturally
> >> >uacceptable.
> >>
> >> As there was no response to the patch there is no way to know whether
> >> it is acceptable or not.
> >
> > I like the idea of removing the tpm rng driver as discussed in other
> > emails in this thread.
> 
> Thank you.

No, thank you.

I didn't first understand the big idea and only looked at the code
change per se. I apologize for that.

The problem that you went to solve was real and it led to a properly
implemented solution. You were not late from the party. Jason's code
change is derivative work of your code change. That's why his code
change has also your signed-off-by.

Thanks for doing awesome work :-)

/Jarkko

Re: [PATCH] tpm: Move Linux RNG connection to hwrng

[Jarkko Sakkinen]

On Wed, Oct 25, 2017 at 08:15:09PM +0530, PrasannaKumar Muralidharan wrote:
> > +   if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
> > +   return 0;
> 
> Can #ifndef CONFIG_HW_RANDOM_TPM be used instead? That way an if
> condition can be avoided.

Nope. There is no reason to avoid the if-condition. Compiler will take
care of it. IS_ENABLED() macro is available just for the purpose Jason
is using it.

> > +   char tpm_hwrng_name[64];
> > +   struct hwrng tpm_hwrng;
> > +
> 
> Can this also be put inside the #ifdef?

Yes. It should be inside #ifdef.

/Jarkko

Re: "BUG: scheduling while atomic" in atmel-aes on Linux v4.14-rc6

[Stephan Mueller]

Am Mittwoch, 25. Oktober 2017, 17:26:31 CEST schrieb Romain Izard:

Hi Romain, Herbert,


> Hello,
> 
> While running the kcapi test suite on a SAMA5D2 Xplained board with a
> v4.14-rc6 kernel, I encountered the following error:

Thank you for the report.
> 
> # kcapi -x 9 -e -c "cbc(aes)" -i  -k
> 000 0 -p
> 1b077a6af4b7f98229de786d7516b639 BUG: scheduling while atomic:
> kcapi/926/0x0100
> CPU: 0 PID: 926 Comm: kcapi Not tainted 4.14.0-rc6 #2
> Hardware name: Atmel SAMA5
> [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
> [] (show_stack) from [] (__schedule_bug+0x60/0x80)
> [] (__schedule_bug) from [] (__schedule+0x368/0x3fc)
> [] (__schedule) from [] (schedule+0x40/0xa0)
> [] (schedule) from [] (__lock_sock+0x78/0xb0)
> [] (__lock_sock) from [] (lock_sock_nested+0x48/0x50)
> [] (lock_sock_nested) from []
> (af_alg_async_cb+0x20/0x80) [] (af_alg_async_cb) from
> []
> (atmel_aes_transfer_complete+0x38/0x68)
> [] (atmel_aes_transfer_complete) from []
> (tasklet_action+0x68/0xb4)
> [] (tasklet_action) from [] (__do_softirq+0xc4/0x250)
> [] (__do_softirq) from [] (irq_exit+0xfc/0x130)
> [] (irq_exit) from [] (__handle_domain_irq+0x58/0xa8)
> [] (__handle_domain_irq) from [] (__irq_svc+0x6c/0x90)
> [] (__irq_svc) from [] (skcipher_recvmsg+0x2d8/0x318)
> [] (skcipher_recvmsg) from [] (sock_read_iter+0x88/0xc8)
> [] (sock_read_iter) from []
> (aio_read.constprop.3+0xcc/0x178)
> [] (aio_read.constprop.3) from []
> (SyS_io_submit+0x540/0x644)
> [] (SyS_io_submit) from [] (ret_fast_syscall+0x0/0x48)
> 
> After bisecting, I determined that it appeared during the 4.14 merge
> window, with the following commit:
> 
> e870456d8e7c crypto: algif_skcipher - overhaul memory management

I think the culprit is the lock_sock in af_alg_async_cb. When going through 
the code protected by the lock, I actually do not see that the struct sock is 
actually accessed in any way other than with an atomic operation. Thus, I 
would infer that lock/unlock in af_alg_async_cb could be safely removed.

Would you agree, Herbert?


Ciao
Stephan

[PATCH v8 2/2] crypto: s5p-sss: Add HASH support for Exynos

[Kamil Konieczny]

Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW.
It uses the crypto framework asynchronous hash api.
It is based on omap-sham.c driver.
S5P has some HW differencies and is not implemented.

Modifications in s5p-sss:

- Add hash supporting structures and functions.

- Modify irq handler to handle both aes and hash signals.

- Resize resource end in probe if EXYNOS_HASH is enabled in
  Kconfig.

- Add new copyright line and new author.

- Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6
  with crypto run-time self test testmgr
  and with tcrypt module with: modprobe tcrypt sec=1 mode=N
  where N=402, 403, 404 (MD5, SHA1, SHA256).

Modifications in drivers/crypto/Kconfig:

- Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG
  and CRYPTO_DEV_S5P

- Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH
  as they are needed for fallback.

Acked-by: Vladimir Zapolskiy 
Reviewed-by: Krzysztof Kozlowski 
Signed-off-by: Kamil Konieczny 
---
 drivers/crypto/Kconfig   |   14 +
 drivers/crypto/s5p-sss.c | 1406 +-
 2 files changed, 1410 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 4b75084fabad..dea4d33d9c7f 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P
  Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES
  algorithms execution.
 
+config CRYPTO_DEV_EXYNOS_HASH
+   bool "Support for Samsung Exynos HASH accelerator"
+   depends on CRYPTO_DEV_S5P
+   depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m
+   select CRYPTO_SHA1
+   select CRYPTO_MD5
+   select CRYPTO_SHA256
+   help
+ Select this to offload Exynos from HASH MD5/SHA1/SHA256.
+ This will select software SHA1, MD5 and SHA256 as they are
+ needed for small and zero-size messages.
+ HASH algorithms will be disabled if EXYNOS_RNG
+ is enabled due to hw conflict.
+
 config CRYPTO_DEV_NX
bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration"
depends on PPC64
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index dfae1865c384..142c6020cec7 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -1,14 +1,16 @@
 /*
  * Cryptographic API.
  *
- * Support for Samsung S5PV210 HW acceleration.
+ * Support for Samsung S5PV210 and Exynos HW acceleration.
  *
  * Copyright (C) 2011 NetUP Inc. All rights reserved.
+ * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved.
  *
  * 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.
  *
+ * Hash part based on omap-sham.c driver.
  */
 
 #include 
@@ -30,28 +32,41 @@
 #include 
 #include 
 
+#include 
+#include 
+#include 
+#include 
+
 #define _SBF(s, v) ((v) << (s))
 
 /* Feed control registers */
 #define SSS_REG_FCINTSTAT  0x
+#define SSS_FCINTSTAT_HPARTINT BIT(7)
+#define SSS_FCINTSTAT_HDONEINT BIT(5)
 #define SSS_FCINTSTAT_BRDMAINT BIT(3)
 #define SSS_FCINTSTAT_BTDMAINT BIT(2)
 #define SSS_FCINTSTAT_HRDMAINT BIT(1)
 #define SSS_FCINTSTAT_PKDMAINT BIT(0)
 
 #define SSS_REG_FCINTENSET 0x0004
+#define SSS_FCINTENSET_HPARTINTENSET   BIT(7)
+#define SSS_FCINTENSET_HDONEINTENSET   BIT(5)
 #define SSS_FCINTENSET_BRDMAINTENSET   BIT(3)
 #define SSS_FCINTENSET_BTDMAINTENSET   BIT(2)
 #define SSS_FCINTENSET_HRDMAINTENSET   BIT(1)
 #define SSS_FCINTENSET_PKDMAINTENSET   BIT(0)
 
 #define SSS_REG_FCINTENCLR 0x0008
+#define SSS_FCINTENCLR_HPARTINTENCLR   BIT(7)
+#define SSS_FCINTENCLR_HDONEINTENCLR   BIT(5)
 #define SSS_FCINTENCLR_BRDMAINTENCLR   BIT(3)
 #define SSS_FCINTENCLR_BTDMAINTENCLR   BIT(2)
 #define SSS_FCINTENCLR_HRDMAINTENCLR   BIT(1)
 #define SSS_FCINTENCLR_PKDMAINTENCLR   BIT(0)
 
 #define SSS_REG_FCINTPEND  0x000C
+#define SSS_FCINTPEND_HPARTINTPBIT(7)
+#define SSS_FCINTPEND_HDONEINTPBIT(5)
 #define SSS_FCINTPEND_BRDMAINTPBIT(3)
 #define SSS_FCINTPEND_BTDMAINTPBIT(2)
 #define SSS_FCINTPEND_HRDMAINTPBIT(1)
@@ -72,6 +87,7 @@
 #define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00)
 #define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01)
 #define SSS_HASHIN_CIPHER_OUTPUT   _SBF(0, 0x02)
+#define SSS_HASHIN_MASK_SBF(0, 0x03)
 
 #define SSS_REG_FCBRDMAS   0x0020
 #define SSS_REG_FCBRDMAL   0x0024
@@ -146,9 +162,80 @@
 #define AES_KEY_LEN16
 #define CRYPTO_QUEUE_LEN   1
 
+/* HASH registers */
+#define SSS_REG_HASH_CTRL  0x00
+
+#define SSS_HASH_USER_IV_EN

[PATCH v8 1/2] crypto: s5p-sss: Change spaces to tabs

[Kamil Konieczny]

Change #define lines to use tabs consistently.

Acked-by: Vladimir Zapolskiy 
Reviewed-by: Krzysztof Kozlowski 
Signed-off-by: Kamil Konieczny 
---
 drivers/crypto/s5p-sss.c | 190 +++
 1 file changed, 95 insertions(+), 95 deletions(-)

diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 7ac657f46d15..dfae1865c384 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -30,98 +30,98 @@
 #include 
 #include 
 
-#define _SBF(s, v)  ((v) << (s))
+#define _SBF(s, v) ((v) << (s))
 
 /* Feed control registers */
-#define SSS_REG_FCINTSTAT   0x
-#define SSS_FCINTSTAT_BRDMAINT  BIT(3)
-#define SSS_FCINTSTAT_BTDMAINT  BIT(2)
-#define SSS_FCINTSTAT_HRDMAINT  BIT(1)
-#define SSS_FCINTSTAT_PKDMAINT  BIT(0)
-
-#define SSS_REG_FCINTENSET  0x0004
-#define SSS_FCINTENSET_BRDMAINTENSETBIT(3)
-#define SSS_FCINTENSET_BTDMAINTENSETBIT(2)
-#define SSS_FCINTENSET_HRDMAINTENSETBIT(1)
-#define SSS_FCINTENSET_PKDMAINTENSETBIT(0)
-
-#define SSS_REG_FCINTENCLR  0x0008
-#define SSS_FCINTENCLR_BRDMAINTENCLRBIT(3)
-#define SSS_FCINTENCLR_BTDMAINTENCLRBIT(2)
-#define SSS_FCINTENCLR_HRDMAINTENCLRBIT(1)
-#define SSS_FCINTENCLR_PKDMAINTENCLRBIT(0)
-
-#define SSS_REG_FCINTPEND   0x000C
-#define SSS_FCINTPEND_BRDMAINTP BIT(3)
-#define SSS_FCINTPEND_BTDMAINTP BIT(2)
-#define SSS_FCINTPEND_HRDMAINTP BIT(1)
-#define SSS_FCINTPEND_PKDMAINTP BIT(0)
-
-#define SSS_REG_FCFIFOSTAT  0x0010
-#define SSS_FCFIFOSTAT_BRFIFOFULBIT(7)
-#define SSS_FCFIFOSTAT_BRFIFOEMPBIT(6)
-#define SSS_FCFIFOSTAT_BTFIFOFULBIT(5)
-#define SSS_FCFIFOSTAT_BTFIFOEMPBIT(4)
-#define SSS_FCFIFOSTAT_HRFIFOFULBIT(3)
-#define SSS_FCFIFOSTAT_HRFIFOEMPBIT(2)
-#define SSS_FCFIFOSTAT_PKFIFOFULBIT(1)
-#define SSS_FCFIFOSTAT_PKFIFOEMPBIT(0)
-
-#define SSS_REG_FCFIFOCTRL  0x0014
-#define SSS_FCFIFOCTRL_DESSEL   BIT(2)
-#define SSS_HASHIN_INDEPENDENT  _SBF(0, 0x00)
-#define SSS_HASHIN_CIPHER_INPUT _SBF(0, 0x01)
-#define SSS_HASHIN_CIPHER_OUTPUT_SBF(0, 0x02)
-
-#define SSS_REG_FCBRDMAS0x0020
-#define SSS_REG_FCBRDMAL0x0024
-#define SSS_REG_FCBRDMAC0x0028
-#define SSS_FCBRDMAC_BYTESWAP   BIT(1)
-#define SSS_FCBRDMAC_FLUSH  BIT(0)
-
-#define SSS_REG_FCBTDMAS0x0030
-#define SSS_REG_FCBTDMAL0x0034
-#define SSS_REG_FCBTDMAC0x0038
-#define SSS_FCBTDMAC_BYTESWAP   BIT(1)
-#define SSS_FCBTDMAC_FLUSH  BIT(0)
-
-#define SSS_REG_FCHRDMAS0x0040
-#define SSS_REG_FCHRDMAL0x0044
-#define SSS_REG_FCHRDMAC0x0048
-#define SSS_FCHRDMAC_BYTESWAP   BIT(1)
-#define SSS_FCHRDMAC_FLUSH  BIT(0)
-
-#define SSS_REG_FCPKDMAS0x0050
-#define SSS_REG_FCPKDMAL0x0054
-#define SSS_REG_FCPKDMAC0x0058
-#define SSS_FCPKDMAC_BYTESWAP   BIT(3)
-#define SSS_FCPKDMAC_DESCENDBIT(2)
-#define SSS_FCPKDMAC_TRANSMIT   BIT(1)
-#define SSS_FCPKDMAC_FLUSH  BIT(0)
-
-#define SSS_REG_FCPKDMAO0x005C
+#define SSS_REG_FCINTSTAT  0x
+#define SSS_FCINTSTAT_BRDMAINT BIT(3)
+#define SSS_FCINTSTAT_BTDMAINT BIT(2)
+#define SSS_FCINTSTAT_HRDMAINT BIT(1)
+#define SSS_FCINTSTAT_PKDMAINT BIT(0)
+
+#define SSS_REG_FCINTENSET 0x0004
+#define SSS_FCINTENSET_BRDMAINTENSET   BIT(3)
+#define SSS_FCINTENSET_BTDMAINTENSET   BIT(2)
+#define SSS_FCINTENSET_HRDMAINTENSET   BIT(1)
+#define SSS_FCINTENSET_PKDMAINTENSET   BIT(0)
+
+#define SSS_REG_FCINTENCLR 0x0008
+#define SSS_FCINTENCLR_BRDMAINTENCLR   BIT(3)
+#define SSS_FCINTENCLR_BTDMAINTENCLR   BIT(2)
+#define SSS_FCINTENCLR_HRDMAINTENCLR   BIT(1)
+#define SSS_FCINTENCLR_PKDMAINTENCLR   BIT(0)
+
+#define SSS_REG_FCINTPEND  0x000C
+#define SSS_FCINTPEND_BRDMAINTPBIT(3)
+#define SSS_FCINTPEND_BTDMAINTPBIT(2)
+#define SSS_FCINTPEND_HRDMAINTPBIT(1)
+#define SSS_FCINTPEND_PKDMAINTPBIT(0)
+
+#define SSS_REG_FCFIFOSTAT 0x0010
+#define SSS_FCFIFOSTAT_BRFIFOFUL   BIT(7)
+#define SSS_FCFIFOSTAT_BRFIFOEMP   BIT(6)
+#define SSS_FCFIFOSTAT_BTFIFOFUL   BIT(5)
+#define SSS_FCFIFOSTAT_BTFIFOEMP   BIT(4)
+#define SSS_FCFIFOSTAT_HRFIFOFUL   BIT(3)
+#define SSS_FCFIFOSTAT_HRFIFOEMP   BIT(2)
+#define SSS_FCFIFOSTAT_PKFIFOFUL   BIT(1)
+#define SSS_FCFIFOSTAT_PKFIFOEMP   BIT(0)
+
+#define SSS_REG_FCFIFOCTRL 0x0014
+#define SSS_FCFIFOCTRL_DESSEL  BIT(2)
+#define 

"BUG: scheduling while atomic" in atmel-aes on Linux v4.14-rc6

[Romain Izard]

Hello,

While running the kcapi test suite on a SAMA5D2 Xplained board with a
v4.14-rc6 kernel, I encountered the following error:

# kcapi -x 9 -e -c "cbc(aes)" -i  -k 000
0 -p 1b077a6af4b7f98229de786d7516b639
BUG: scheduling while atomic: kcapi/926/0x0100
CPU: 0 PID: 926 Comm: kcapi Not tainted 4.14.0-rc6 #2
Hardware name: Atmel SAMA5
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (__schedule_bug+0x60/0x80)
[] (__schedule_bug) from [] (__schedule+0x368/0x3fc)
[] (__schedule) from [] (schedule+0x40/0xa0)
[] (schedule) from [] (__lock_sock+0x78/0xb0)
[] (__lock_sock) from [] (lock_sock_nested+0x48/0x50)
[] (lock_sock_nested) from [] (af_alg_async_cb+0x20/0x80)
[] (af_alg_async_cb) from []
(atmel_aes_transfer_complete+0x38/0x68)
[] (atmel_aes_transfer_complete) from []
(tasklet_action+0x68/0xb4)
[] (tasklet_action) from [] (__do_softirq+0xc4/0x250)
[] (__do_softirq) from [] (irq_exit+0xfc/0x130)
[] (irq_exit) from [] (__handle_domain_irq+0x58/0xa8)
[] (__handle_domain_irq) from [] (__irq_svc+0x6c/0x90)
[] (__irq_svc) from [] (skcipher_recvmsg+0x2d8/0x318)
[] (skcipher_recvmsg) from [] (sock_read_iter+0x88/0xc8)
[] (sock_read_iter) from []
(aio_read.constprop.3+0xcc/0x178)
[] (aio_read.constprop.3) from []
(SyS_io_submit+0x540/0x644)
[] (SyS_io_submit) from [] (ret_fast_syscall+0x0/0x48)

After bisecting, I determined that it appeared during the 4.14 merge
window, with the following commit:

e870456d8e7c crypto: algif_skcipher - overhaul memory management


Best regards,
-- 
Romain Izard

[PATCH v8 0/2] crypto: s5p-sss: Add HASH support for Exynos

[Kamil Konieczny]

First patch change spaces to tabs, second adds HASH support for Exynos.
Changes:

version 8:
- fixes suggested by Vladimir Zapolskiy: drop first condition check in 
  s5p_hash_import, delete unused include delay.h, fix typo in commit
  message, fix descriptions of struct s5p_hash_reqctx and function
  s5p_hash_final()

version 7:
- fix ifdef into if(IS_ENABLED()) as suggested by Krzysztof Kozlowski

version 6:
- fixes suggested by Vladimir Zapolskiy: change HASH_OP enum into bool, fix
  comments, change int into unsigned int in several functions, change some
  functions to return void, remove unnecessary parentheses in s5p_hash_import,
  replace rctx with ctx for request context, drop some dd vars and use tctx->dd
  instead, simplify s5p_hash_rx, s5p_hash_copy_result and s5p_hash_set_flow,
  change int final into bool final, reoder some declarations, split patch into
  two
- rewrite and fix while loop in s5p_hash_copy_sg_lists
- rewrite while loop in s5p_hash_prepare_sgs

version 5:
- fix suggested by Krzysztof Kozlowski: change defines HASH_OP into enum, fix
  comments

version 4:
- fixes suggested by Krzysztof Kozlowski: reformat comments, convert context
  flags into two bool vars, drop SSS_ALIGNED, change name of SSS_DMA_ALIGN and
  SSS_DMA_ALIGN_MASK, split assignments into separate lines, use IS_ENABLED in
  place of ifdef, remove sss_hash_algs_info and simplify register and deregister
  HASH algs

version 3:
- many fixes suggested by Krzysztof Kozlowski: comments, uppercases in const,
  remove unused defines, remove unused variable bs, constify aes_variant,
  remove global var use_hash, remove WARN_ON, improve hash_import(),
  change goto label into 'out' in s5p_hash_handle_queue(), reorder variable
  declarations, add spinlock to protect clearing HASH_FLAGS_BUSY
- simplify code: replace one-line functions s5p_hash_update_req(),
  s5p_hash_final_req() with call to s5p_hash_xmit_dma(), and delete them
- replace call to s5p_hash_hw_init() into s5p_ahash_dma_init() and delete it
- fix clearing shash flag CRYPTO_TFM_REQ_MAY_SLEEP
- fix s5p_hash_set_flow()

version 2:
- change patch format so number of lines drops
- change in Kconfig as suggested by Krzysztof Kozlowski, add
EXYNOS_HASH subsection
- change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH
- remove style fixups in aes, as they should go in separate patch
- remove FLOW_LOG, FLOW_DUMP macros and its uses
- remove #if 0 ... endif
- remove unused function hash_wait and its defines
- fix compiler warning in dev_dbg
- remove some comments
- other minor fixes in comments

Kamil Konieczny (2):
  crypto: s5p-sss: Change spaces to tabs
  crypto: s5p-sss: Add HASH support for Exynos

 drivers/crypto/Kconfig   |   14 +
 drivers/crypto/s5p-sss.c | 1596 +++---
 2 files changed, 1505 insertions(+), 105 deletions(-)

-- 
2.14.1.536.g6867272d5b56

Re: [PATCH] tpm: Move Linux RNG connection to hwrng

[PrasannaKumar Muralidharan]

Hi Jason,

On 25 October 2017 at 20:48, Jason Gunthorpe
 wrote:
> On Wed, Oct 25, 2017 at 08:15:09PM +0530, PrasannaKumar Muralidharan
> wrote:
>
>> > +static int tpm_add_hwrng(struct tpm_chip *chip)
>> > +{
>> > +   if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
>> > +   return 0;
>>
>> Can #ifndef CONFIG_HW_RANDOM_TPM be used instead? That way an if
>> condition can be avoided.
>
> Generally speaking IS_ENABLED is prefered over #ifdef as it reduces the
> set of compilation combinations.

Oh okay. No issues then.

Regards,
PrasannaKumar

Re: [PATCH v2] tpm: use struct tpm_chip for tpm_chip_find_get()

[Jason Gunthorpe]

On Wed, Oct 25, 2017 at 01:55:04PM +0200, Jarkko Sakkinen wrote:
> Device number (the character device index) is not a stable identifier
> for a TPM chip. That is the reason why every call site passes
> TPM_ANY_NUM to tpm_chip_find_get().
> 
> This commit changes the API in a way that instead a struct tpm_chip
> instance is given and NULL means the default chip. In addition, this
> commit refines the documentation to be up to date with the
> implementation.
> 
> Suggested-by: Jason Gunthorpe  (@chip_num -> 
> @chip)
> Signed-off-by: Jarkko Sakkinen 
> v2:
> * Further defined function documentation.
> * Changed @chip_num to @chip instead of removing the parameter as suggested by
>   Jason Gunthorpe.

Reviewed-by: Jason Gunthorpe 

Jason

Re: [PATCH] tpm: Move Linux RNG connection to hwrng

[Jason Gunthorpe]

On Wed, Oct 25, 2017 at 08:15:09PM +0530, PrasannaKumar Muralidharan
wrote:

> > +static int tpm_add_hwrng(struct tpm_chip *chip)
> > +{
> > +   if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
> > +   return 0;
> 
> Can #ifndef CONFIG_HW_RANDOM_TPM be used instead? That way an if
> condition can be avoided.

Generally speaking IS_ENABLED is prefered over #ifdef as it reduces the
set of compilation combinations.

Jason

Re: [PATCH v2] tpm: use struct tpm_chip for tpm_chip_find_get()

[PrasannaKumar Muralidharan]

Hi Jarkko,

On 25 October 2017 at 17:25, Jarkko Sakkinen
 wrote:
> Device number (the character device index) is not a stable identifier
> for a TPM chip. That is the reason why every call site passes
> TPM_ANY_NUM to tpm_chip_find_get().
>
> This commit changes the API in a way that instead a struct tpm_chip
> instance is given and NULL means the default chip. In addition, this
> commit refines the documentation to be up to date with the
> implementation.
>
> Suggested-by: Jason Gunthorpe  (@chip_num -> 
> @chip)
> Signed-off-by: Jarkko Sakkinen 
> ---
> v2:
> * Further defined function documentation.
> * Changed @chip_num to @chip instead of removing the parameter as suggested by
>   Jason Gunthorpe.
>  drivers/char/hw_random/tpm-rng.c|   2 +-
>  drivers/char/tpm/tpm-chip.c |  21 +++---
>  drivers/char/tpm/tpm-interface.c| 135 
> +++-
>  drivers/char/tpm/tpm.h  |   2 +-
>  include/linux/tpm.h |  38 +-
>  security/integrity/ima/ima_crypto.c |   2 +-
>  security/integrity/ima/ima_init.c   |   2 +-
>  security/integrity/ima/ima_queue.c  |   2 +-
>  security/keys/trusted.c |  35 +-
>  9 files changed, 125 insertions(+), 114 deletions(-)
>
> diff --git a/drivers/char/hw_random/tpm-rng.c 
> b/drivers/char/hw_random/tpm-rng.c
> index d6d448266f07..c5e363825af0 100644
> --- a/drivers/char/hw_random/tpm-rng.c
> +++ b/drivers/char/hw_random/tpm-rng.c
> @@ -25,7 +25,7 @@
>
>  static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>  {
> -   return tpm_get_random(TPM_ANY_NUM, data, max);
> +   return tpm_get_random(NULL, data, max);
>  }
>
>  static struct hwrng tpm_rng = {
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index a114e8f7fb90..c7a4e7fb424d 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -81,21 +81,26 @@ void tpm_put_ops(struct tpm_chip *chip)
>  EXPORT_SYMBOL_GPL(tpm_put_ops);
>
>  /**
> - * tpm_chip_find_get() - return tpm_chip for a given chip number
> - * @chip_num: id to find
> + * tpm_chip_find_get() - find and reserve a TPM chip
> + * @chip:  a  tpm_chip instance, %NULL for the default chip
>   *
> - * The return'd chip has been tpm_try_get_ops'd and must be released via
> - * tpm_put_ops
> + * Finds a TPM chip and reserves its class device and operations. The chip 
> must
> + * be released with tpm_chip_put_ops() after use.
> + *
> + * Return:
> + * A reserved  tpm_chip instance.
> + * %NULL if a chip is not found.
> + * %NULL if the chip is not available.
>   */
> -struct tpm_chip *tpm_chip_find_get(int chip_num)
> +struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
>  {
> -   struct tpm_chip *chip, *res = NULL;
> +   struct tpm_chip *res = NULL;
> +   int chip_num = 0;
> int chip_prev;
>
> mutex_lock(_lock);
>
> -   if (chip_num == TPM_ANY_NUM) {
> -   chip_num = 0;
> +   if (!chip) {
> do {
> chip_prev = chip_num;
> chip = idr_get_next(_nums_idr, _num);

When chip is not NULL just do tpm_try_get_ops(chip). Current code does
more things which are not required.

> diff --git a/drivers/char/tpm/tpm-interface.c 
> b/drivers/char/tpm/tpm-interface.c
> index ebe0a1d36d8c..19f820f775b5 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -809,19 +809,20 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int 
> pcr_idx, u8 *res_buf)
>  }
>
>  /**
> - * tpm_is_tpm2 - is the chip a TPM2 chip?
> - * @chip_num:  tpm idx # or ANY
> + * tpm_is_tpm2 - do we a have a TPM2 chip?
> + * @chip:  a  tpm_chip instance, %NULL for the default chip
>   *
> - * Returns < 0 on error, and 1 or 0 on success depending whether the chip
> - * is a TPM2 chip.
> + * Return:
> + * 1 if we have a TPM2 chip.
> + * 0 if we don't have a TPM2 chip.
> + * A negative number for system errors (errno).
>   */
> -int tpm_is_tpm2(u32 chip_num)
> +int tpm_is_tpm2(struct tpm_chip *chip)
>  {
> -   struct tpm_chip *chip;
> int rc;
>
> -   chip = tpm_chip_find_get(chip_num);
> -   if (chip == NULL)
> +   chip = tpm_chip_find_get(chip);
> +   if (!chip)
> return -ENODEV;
>
> rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0;
> @@ -833,23 +834,19 @@ int tpm_is_tpm2(u32 chip_num)
>  EXPORT_SYMBOL_GPL(tpm_is_tpm2);
>
>  /**
> - * tpm_pcr_read - read a pcr value
> - * @chip_num:  tpm idx # or ANY
> - * @pcr_idx:   pcr idx to retrieve
> - * @res_buf:   TPM_PCR value
> - * size of res_buf is 20 bytes (or NULL if you don't care)
> + * tpm_pcr_read - read a PCR value from SHA1 bank
> + * @chip:  a  tpm_chip instance, %NULL for the default chip
> + * @pcr_idx:   the PCR to be retrieved
> + * @res_buf:   the value of the PCR
>   *
> - * 

Re: [tpmdd-devel] [PATCH] tpm: remove chip_num parameter from in-kernel API

[PrasannaKumar Muralidharan]

Hi Jarkko,

On 24 October 2017 at 23:52, Jarkko Sakkinen
 wrote:
> On Tue, Oct 24, 2017 at 10:05:20PM +0530, PrasannaKumar Muralidharan wrote:
>> > 1. Every user in the kernel is using TPM_ANY_NUM, which means there are
>> >no other users.
>>
>> Completely agree that there is no in kernel users yet.
>
> And should never be. It's a bogus parameter that makes no sense.

I understood that after seeing latest patch that uses struct tpm_chip.
Sorry for the noise.

>> > 2. Moving struct tpm_rng to the TPM client is architecturally
>> >uacceptable.
>>
>> As there was no response to the patch there is no way to know whether
>> it is acceptable or not.
>
> I like the idea of removing the tpm rng driver as discussed in other
> emails in this thread.

Thank you.

>> > 3. Using zero deos not give you any better guarantees on anything than
>> >just using TPM_ANY_NUM.
>>
>> Chip id is used, not zero.
>
> Sorry I misread the patch first time. Anyway it's not any kind of ID to
> be trusted.

Okay.

>> > Why this patch is not CC'd to linux-integrity? It modifies the TPM
>> > driver. And in the worst way.
>>
>> TPM list is moderated and the moderator has not approved it yet.
>> get_maintainer script did not say about linux-integrity mailing list.
>>
>> It could be doing things in worst way but it is not known until some
>> one says. If no one tells it is the case I don't think it is possible
>> to fix. Which is what happened.
>
> Understood. We've moved to linux-integr...@vger.kernel.org. MAINTAINERS
> update is in the queue for the next kernel release.

Sorry I never knew this.

>> > Implementing the ideas that Jason explained is the senseful way to
>> > get stable access. modules.dep makes sure that the modules are loaded
>> > in the correct order.
>>
>> If that is sensible then it is the way to go.
>>
>> There must be a reason to believe what is sensible and what is not.
>> Looks like this RFC has helped in judging that.
>>
>> Regards,
>> PrasannaKumar
>
> Would you be interested to work on patch set that would remove the
> existing tpm rng driver and make the TPM driver the customer? It's not
> that far away from the work you've been doing already.
>
> /Jarkko

I am late to the party. Jason has sent a patch doing that by the time
I read this email.

Thanks and regards,
PrasannaKumar

Re: [PATCH] tpm: Move Linux RNG connection to hwrng

[PrasannaKumar Muralidharan]

Hi Jason,

Nice to see this patch. Some minor comments below.

On 25 October 2017 at 00:12, Jason Gunthorpe
 wrote:
> The tpm-rng.c approach is completely inconsistent with how the kernel
> handles hotplug. Instead manage a hwrng device for each TPM. This will
> cause the kernel to read entropy from the TPM when it is plugged in,
> and allow access to the TPM rng via /dev/hwrng.
>
> Signed-off-by: PrasannaKumar Muralidharan 
> Signed-off-by: Jason Gunthorpe 
> Reviewed-by: Jason Gunthorpe 
> ---
>  drivers/char/hw_random/Kconfig   | 13 ---
>  drivers/char/hw_random/Makefile  |  1 -
>  drivers/char/hw_random/tpm-rng.c | 50 
> 
>  drivers/char/tpm/Kconfig | 13 +++
>  drivers/char/tpm/tpm-chip.c  | 41 
>  drivers/char/tpm/tpm-interface.c | 37 -
>  drivers/char/tpm/tpm.h   |  5 
>  7 files changed, 76 insertions(+), 84 deletions(-)
>  delete mode 100644 drivers/char/hw_random/tpm-rng.c
>
> All,
>
> It is such a good idea, I took PrasannaKumar's patch, reviewed and
> revised it to the level it could be merged.

Thank you for this.

> This is compile tested only.
>
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 95a031e9eced07..a20fed182cbcce 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -306,19 +306,6 @@ config HW_RANDOM_POWERNV
>
>   If unsure, say Y.
>
> -config HW_RANDOM_TPM
> -   tristate "TPM HW Random Number Generator support"
> -   depends on TCG_TPM
> -   default HW_RANDOM
> -   ---help---
> - This driver provides kernel-side support for the Random Number
> - Generator in the Trusted Platform Module
> -
> - To compile this driver as a module, choose M here: the
> - module will be called tpm-rng.
> -
> - If unsure, say Y.
> -
>  config HW_RANDOM_HISI
> tristate "Hisilicon Random Number Generator support"
> depends on HW_RANDOM && ARCH_HISI
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index 39a67defac67cb..91cb8e8213e7c1 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -26,7 +26,6 @@ obj-$(CONFIG_HW_RANDOM_NOMADIK) += nomadik-rng.o
>  obj-$(CONFIG_HW_RANDOM_PSERIES) += pseries-rng.o
>  obj-$(CONFIG_HW_RANDOM_POWERNV) += powernv-rng.o
>  obj-$(CONFIG_HW_RANDOM_HISI)   += hisi-rng.o
> -obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
>  obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o
>  obj-$(CONFIG_HW_RANDOM_IPROC_RNG200) += iproc-rng200.o
>  obj-$(CONFIG_HW_RANDOM_MSM) += msm-rng.o
> diff --git a/drivers/char/hw_random/tpm-rng.c 
> b/drivers/char/hw_random/tpm-rng.c
> deleted file mode 100644
> index d6d448266f0752..00
> --- a/drivers/char/hw_random/tpm-rng.c
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -/*
> - * Copyright (C) 2012 Kent Yoder IBM Corporation
> - *
> - * HWRNG interfaces to pull RNG data from a TPM
> - *
> - * 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.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
> - */
> -
> -#include 
> -#include 
> -#include 
> -
> -#define MODULE_NAME "tpm-rng"
> -
> -static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> -{
> -   return tpm_get_random(TPM_ANY_NUM, data, max);
> -}
> -
> -static struct hwrng tpm_rng = {
> -   .name = MODULE_NAME,
> -   .read = tpm_rng_read,
> -};
> -
> -static int __init rng_init(void)
> -{
> -   return hwrng_register(_rng);
> -}
> -module_init(rng_init);
> -
> -static void __exit rng_exit(void)
> -{
> -   hwrng_unregister(_rng);
> -}
> -module_exit(rng_exit);
> -
> -MODULE_LICENSE("GPL v2");
> -MODULE_AUTHOR("Kent Yoder ");
> -MODULE_DESCRIPTION("RNG driver for TPM devices");
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index a30352202f1fdc..a95725fa77789e 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -26,6 +26,19 @@ menuconfig TCG_TPM
>
>  if TCG_TPM
>
> +config HW_RANDOM_TPM
> +   tristate "TPM HW Random Number Generator support"
> +   depends on TCG_TPM && HW_RANDOM
> +   default HW_RANDOM
> +   ---help---
> +   

[PATCH v2] tpm: use struct tpm_chip for tpm_chip_find_get()

[Jarkko Sakkinen]

Device number (the character device index) is not a stable identifier
for a TPM chip. That is the reason why every call site passes
TPM_ANY_NUM to tpm_chip_find_get().

This commit changes the API in a way that instead a struct tpm_chip
instance is given and NULL means the default chip. In addition, this
commit refines the documentation to be up to date with the
implementation.

Suggested-by: Jason Gunthorpe  (@chip_num -> 
@chip)
Signed-off-by: Jarkko Sakkinen 
---
v2:
* Further defined function documentation.
* Changed @chip_num to @chip instead of removing the parameter as suggested by
  Jason Gunthorpe.
 drivers/char/hw_random/tpm-rng.c|   2 +-
 drivers/char/tpm/tpm-chip.c |  21 +++---
 drivers/char/tpm/tpm-interface.c| 135 +++-
 drivers/char/tpm/tpm.h  |   2 +-
 include/linux/tpm.h |  38 +-
 security/integrity/ima/ima_crypto.c |   2 +-
 security/integrity/ima/ima_init.c   |   2 +-
 security/integrity/ima/ima_queue.c  |   2 +-
 security/keys/trusted.c |  35 +-
 9 files changed, 125 insertions(+), 114 deletions(-)

diff --git a/drivers/char/hw_random/tpm-rng.c b/drivers/char/hw_random/tpm-rng.c
index d6d448266f07..c5e363825af0 100644
--- a/drivers/char/hw_random/tpm-rng.c
+++ b/drivers/char/hw_random/tpm-rng.c
@@ -25,7 +25,7 @@
 
 static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
 {
-   return tpm_get_random(TPM_ANY_NUM, data, max);
+   return tpm_get_random(NULL, data, max);
 }
 
 static struct hwrng tpm_rng = {
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index a114e8f7fb90..c7a4e7fb424d 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -81,21 +81,26 @@ void tpm_put_ops(struct tpm_chip *chip)
 EXPORT_SYMBOL_GPL(tpm_put_ops);
 
 /**
- * tpm_chip_find_get() - return tpm_chip for a given chip number
- * @chip_num: id to find
+ * tpm_chip_find_get() - find and reserve a TPM chip
+ * @chip:  a  tpm_chip instance, %NULL for the default chip
  *
- * The return'd chip has been tpm_try_get_ops'd and must be released via
- * tpm_put_ops
+ * Finds a TPM chip and reserves its class device and operations. The chip must
+ * be released with tpm_chip_put_ops() after use.
+ *
+ * Return:
+ * A reserved  tpm_chip instance.
+ * %NULL if a chip is not found.
+ * %NULL if the chip is not available.
  */
-struct tpm_chip *tpm_chip_find_get(int chip_num)
+struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
 {
-   struct tpm_chip *chip, *res = NULL;
+   struct tpm_chip *res = NULL;
+   int chip_num = 0;
int chip_prev;
 
mutex_lock(_lock);
 
-   if (chip_num == TPM_ANY_NUM) {
-   chip_num = 0;
+   if (!chip) {
do {
chip_prev = chip_num;
chip = idr_get_next(_nums_idr, _num);
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index ebe0a1d36d8c..19f820f775b5 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -809,19 +809,20 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, 
u8 *res_buf)
 }
 
 /**
- * tpm_is_tpm2 - is the chip a TPM2 chip?
- * @chip_num:  tpm idx # or ANY
+ * tpm_is_tpm2 - do we a have a TPM2 chip?
+ * @chip:  a  tpm_chip instance, %NULL for the default chip
  *
- * Returns < 0 on error, and 1 or 0 on success depending whether the chip
- * is a TPM2 chip.
+ * Return:
+ * 1 if we have a TPM2 chip.
+ * 0 if we don't have a TPM2 chip.
+ * A negative number for system errors (errno).
  */
-int tpm_is_tpm2(u32 chip_num)
+int tpm_is_tpm2(struct tpm_chip *chip)
 {
-   struct tpm_chip *chip;
int rc;
 
-   chip = tpm_chip_find_get(chip_num);
-   if (chip == NULL)
+   chip = tpm_chip_find_get(chip);
+   if (!chip)
return -ENODEV;
 
rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0;
@@ -833,23 +834,19 @@ int tpm_is_tpm2(u32 chip_num)
 EXPORT_SYMBOL_GPL(tpm_is_tpm2);
 
 /**
- * tpm_pcr_read - read a pcr value
- * @chip_num:  tpm idx # or ANY
- * @pcr_idx:   pcr idx to retrieve
- * @res_buf:   TPM_PCR value
- * size of res_buf is 20 bytes (or NULL if you don't care)
+ * tpm_pcr_read - read a PCR value from SHA1 bank
+ * @chip:  a  tpm_chip instance, %NULL for the default chip
+ * @pcr_idx:   the PCR to be retrieved
+ * @res_buf:   the value of the PCR
  *
- * The TPM driver should be built-in, but for whatever reason it
- * isn't, protect against the chip disappearing, by incrementing
- * the module usage count.
+ * Return: same as with tpm_transmit_cmd()
  */
-int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
+int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 {
-   struct tpm_chip *chip;
int rc;
 
-   chip = tpm_chip_find_get(chip_num);
-   if (chip == NULL)
+

[PATCH] drivers/crypto: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Herbert Xu 
Cc: Jesper Nilsson 
Cc: Lars Persson 
Cc: Niklas Cassel 
Cc: "David S. Miller" 
Cc: Jamie Iles 
Cc: linux-arm-ker...@axis.com
Cc: linux-crypto@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Signed-off-by: Kees Cook 
---
 drivers/crypto/axis/artpec6_crypto.c | 6 +++---
 drivers/crypto/mv_cesa.c | 4 ++--
 drivers/crypto/picoxcell_crypto.c| 7 +++
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/axis/artpec6_crypto.c 
b/drivers/crypto/axis/artpec6_crypto.c
index 0f9754e07719..456278440863 100644
--- a/drivers/crypto/axis/artpec6_crypto.c
+++ b/drivers/crypto/axis/artpec6_crypto.c
@@ -2072,9 +2072,9 @@ static void artpec6_crypto_process_queue(struct 
artpec6_crypto *ac)
del_timer(>timer);
 }
 
-static void artpec6_crypto_timeout(unsigned long data)
+static void artpec6_crypto_timeout(struct timer_list *t)
 {
-   struct artpec6_crypto *ac = (struct artpec6_crypto *) data;
+   struct artpec6_crypto *ac = from_timer(ac, t, timer);
 
dev_info_ratelimited(artpec6_crypto_dev, "timeout\n");
 
@@ -3063,7 +3063,7 @@ static int artpec6_crypto_probe(struct platform_device 
*pdev)
spin_lock_init(>queue_lock);
INIT_LIST_HEAD(>queue);
INIT_LIST_HEAD(>pending);
-   setup_timer(>timer, artpec6_crypto_timeout, (unsigned long) ac);
+   timer_setup(>timer, artpec6_crypto_timeout, 0);
 
ac->base = base;
 
diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
index c3883b49f56e..6a8275f017a8 100644
--- a/drivers/crypto/mv_cesa.c
+++ b/drivers/crypto/mv_cesa.c
@@ -149,7 +149,7 @@ struct mv_req_hash_ctx {
int count_add;
 };
 
-static void mv_completion_timer_callback(unsigned long unused)
+static void mv_completion_timer_callback(struct timer_list *unused)
 {
int active = readl(cpg->reg + SEC_ACCEL_CMD) & SEC_CMD_EN_SEC_ACCL0;
 
@@ -167,7 +167,7 @@ static void mv_completion_timer_callback(unsigned long 
unused)
 
 static void mv_setup_timer(void)
 {
-   setup_timer(>completion_timer, _completion_timer_callback, 0);
+   timer_setup(>completion_timer, mv_completion_timer_callback, 0);
mod_timer(>completion_timer,
jiffies + msecs_to_jiffies(MV_CESA_EXPIRE));
 }
diff --git a/drivers/crypto/picoxcell_crypto.c 
b/drivers/crypto/picoxcell_crypto.c
index b6f14844702e..5a6dc53b2b9d 100644
--- a/drivers/crypto/picoxcell_crypto.c
+++ b/drivers/crypto/picoxcell_crypto.c
@@ -1125,9 +1125,9 @@ static irqreturn_t spacc_spacc_irq(int irq, void *dev)
return IRQ_HANDLED;
 }
 
-static void spacc_packet_timeout(unsigned long data)
+static void spacc_packet_timeout(struct timer_list *t)
 {
-   struct spacc_engine *engine = (struct spacc_engine *)data;
+   struct spacc_engine *engine = from_timer(engine, t, packet_timeout);
 
spacc_process_done(engine);
 }
@@ -1714,8 +1714,7 @@ static int spacc_probe(struct platform_device *pdev)
writel(SPA_IRQ_EN_STAT_EN | SPA_IRQ_EN_GLBL_EN,
   engine->regs + SPA_IRQ_EN_REG_OFFSET);
 
-   setup_timer(>packet_timeout, spacc_packet_timeout,
-   (unsigned long)engine);
+   timer_setup(>packet_timeout, spacc_packet_timeout, 0);
 
INIT_LIST_HEAD(>pending);
INIT_LIST_HEAD(>completed);
-- 
2.7.4


-- 
Kees Cook
Pixel Security