Re: md5sum (from libkcapi) fails on kernel 4.9 but not on 4.13

[Christophe LEROY]

Hi Stephan,

Le 16/10/2017 à 23:10, Stephan Mueller a écrit :

Am Montag, 16. Oktober 2017, 08:53:00 CEST schrieb Christophe LEROY:

Hi Christophe,


Hi Stephan,

I get an issue with md5sum of a big file with kernel 4.9. It don't get
that issue with kernel 4.13.


The key to the difference in libkcapi is the following code:

static void _kcapi_handle_flags(struct kcapi_handle *handle)
...
 /* older interfaces only processed 16 pages in a row */
 handle->flags.alg_max_pages = _kcapi_kernver_ge(handle, 4, 11, 0) ?
   UINT_MAX : ALG_MAX_PAGES;
...

This check is mainly for skcipher but it affects all cipher types. Thus, on
older kernels, only 16 pages are injected into the kernel with splice and then
it is reverted to sendmsg. Again, this logic is not so much of an issue for
algif_hash, but rather for algif_skcipher and algif_aead.


When I do an strace, I see a difference in the calls: at the end of the
file, with 4.9 md5sum uses sendmsg() for the last block while with 4.13
it uses splice() as for all the previous blocks.

The problem is that the last block has a size over 32kbytes, which is
the maximum size the talitos driver accepts to hash at once.
It looks like sendmsg() sends the entire block to the crypto driver
while splice() calls the crypto driver with blocks of page size.


I understand that your driver has this 32kb limit. But I am not sure what the
difference between the handling of the sendmsg and the splice invocation is.
According to the strace, the splice accepted 256kb of data. Looking into the
hash_sendpage function implementing the backend of this splice system call, it
simply performs an update invocation with this buffer size. I.e. it invokes
your driver with 256kb of data. The sendmsg first performs a copy_from_user
with the maximum limit of 16 * PAGE_SIZE (see the limit variable in
hash_sendmsg) and then invokes the update function.


As far as I can see, hash_sendpage() is called with one single page, of 
size 16kb in my setup. If I understand correct, the loop over every page 
is implemented in __splice_from_pipe().




So, I am not sure why the sendmsg call chokes where the sendpage call
succeeds.


As far as I can see, the sendmsg() calls the update function with the 
full buffer, which in my case is over the 32kb limit of the driver.




If you tamper with the code shown above from libkcapi and set alg_max_pages to
a low value, the library reverts to sendmsg after the given number of pages.


Couldn't we get the libkcapi to splice until the last full page, then 
only use sendmsg() for the last chunk once it is smaller than an entire 
page ?


Thanks
Christophe

Re: md5sum (from libkcapi) fails on kernel 4.9 but not on 4.13

[Christophe LEROY]

Hi Again Stephan

Le 17/10/2017 à 09:58, Christophe LEROY a écrit :

Hi Stephan,

Le 16/10/2017 à 23:10, Stephan Mueller a écrit :

Am Montag, 16. Oktober 2017, 08:53:00 CEST schrieb Christophe LEROY:

Hi Christophe,


Hi Stephan,

I get an issue with md5sum of a big file with kernel 4.9. It don't get
that issue with kernel 4.13.


The key to the difference in libkcapi is the following code:

static void _kcapi_handle_flags(struct kcapi_handle *handle)
...
 /* older interfaces only processed 16 pages in a row */
 handle->flags.alg_max_pages = _kcapi_kernver_ge(handle, 4, 
11, 0) ?

   UINT_MAX : ALG_MAX_PAGES;
...

This check is mainly for skcipher but it affects all cipher types. 
Thus, on
older kernels, only 16 pages are injected into the kernel with splice 
and then
it is reverted to sendmsg. Again, this logic is not so much of an 
issue for

algif_hash, but rather for algif_skcipher and algif_aead.


When I do an strace, I see a difference in the calls: at the end of the
file, with 4.9 md5sum uses sendmsg() for the last block while with 4.13
it uses splice() as for all the previous blocks.

The problem is that the last block has a size over 32kbytes, which is
the maximum size the talitos driver accepts to hash at once.
It looks like sendmsg() sends the entire block to the crypto driver
while splice() calls the crypto driver with blocks of page size.


I understand that your driver has this 32kb limit. But I am not sure 
what the
difference between the handling of the sendmsg and the splice 
invocation is.
According to the strace, the splice accepted 256kb of data. Looking 
into the
hash_sendpage function implementing the backend of this splice system 
call, it
simply performs an update invocation with this buffer size. I.e. it 
invokes
your driver with 256kb of data. The sendmsg first performs a 
copy_from_user

with the maximum limit of 16 * PAGE_SIZE (see the limit variable in
hash_sendmsg) and then invokes the update function.


As far as I can see, hash_sendpage() is called with one single page, of 
size 16kb in my setup. If I understand correct, the loop over every page 
is implemented in __splice_from_pipe().




So, I am not sure why the sendmsg call chokes where the sendpage call
succeeds.


As far as I can see, the sendmsg() calls the update function with the 
full buffer, which in my case is over the 32kb limit of the driver.




If you tamper with the code shown above from libkcapi and set 
alg_max_pages to
a low value, the library reverts to sendmsg after the given number of 
pages.


Couldn't we get the libkcapi to splice until the last full page, then 
only use sendmsg() for the last chunk once it is smaller than an entire 
page ?





Note that the test reported above is done with version 0.14.0

I've now tried a test with 1.0.0, and there seems to be another big 
issue: the error returned by sendmsg() is not taken into account anymore:


...
vmsplice(5, 
[{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
558080}], 1, SPLICE_F_MORE|SPLICE_F_GIFT) = 262144

splice(4, NULL, 7, NULL, 262144, SPLICE_F_MORE) = 262144
vmsplice(5, 
[{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
295936}], 1, SPLICE_F_MORE|SPLICE_F_GIFT) = 262144

splice(4, NULL, 7, NULL, 262144, SPLICE_F_MORE) = 262144
sendmsg(7, {msg_name(0)=NULL, 
msg_iov(1)=[{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
33792}], msg_controllen=0, msg_flags=0}, MSG_MORE) = -1 EINVAL (Invalid 
argument)
sendmsg(7, {msg_name(0)=NULL, 
msg_iov(1)=[{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
33814}], msg_controllen=0, msg_flags=0}, MSG_MORE) = -1 EINVAL (Invalid 
argument)
sendmsg(7, {msg_name(0)=NULL, 
msg_iov(1)=[{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
33836}], msg_controllen=0, msg_flags=0}, MSG_MORE) = -1 EINVAL (Invalid 
argument)
sendmsg(7, {msg_name(0)=NULL, 
msg_iov(1)=[{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
33858}], msg_controllen=0, msg_flags=0}, MSG_MORE) = -1 EINVAL (Invalid 
argument)
sendmsg(7, {msg_name(0)=NULL, 
msg_iov(1)=[{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
33880}], msg_controllen=0, msg_flags=0}, MSG_MORE) = -1 EINVAL (Invalid 
argument)
sendmsg(7, {msg_name(0)=NULL, 
msg_iov(1)=[{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
33902}], msg_controllen=0, msg_flags=0}, MSG_MORE) = -1 EINVAL (Invalid 
argument)
sendmsg(7, {msg_name(0)=NULL, 
msg_iov(1)=[{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
33924}], msg_controllen=0, msg_flags=0}, MSG_MORE) = -1 EINVAL (Invalid 
argument)
sendmsg(7, {msg_name(0)=NULL, 
msg_iov(1)=[{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
33946}], msg_controllen=0, msg_flags=0}, MSG_MORE) = -1 EINVAL (Invalid 
argument)
sendmsg(7, {msg_name(0)=NULL, 

Re: [PATCH v2 1/8] lib/scatterlist: Introduce sgl_alloc() and sgl_free()

[Johannes Thumshirn]

Thanks Bart,
Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v9 17/20] crypto: talitos: move to generic async completion

[Christophe LEROY]

Le 15/10/2017 à 11:20, Gilad Ben-Yossef a écrit :

The talitos driver starts several async crypto ops and  waits for their
completions. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef 


Tested-by: Christophe Leroy 


---
  drivers/crypto/talitos.c | 38 +-
  1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 5bd8191..9c80e0c 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2160,22 +2160,6 @@ static int ahash_import(struct ahash_request *areq, 
const void *in)
return 0;
  }
  
-struct keyhash_result {

-   struct completion completion;
-   int err;
-};
-
-static void keyhash_complete(struct crypto_async_request *req, int err)
-{
-   struct keyhash_result *res = req->data;
-
-   if (err == -EINPROGRESS)
-   return;
-
-   res->err = err;
-   complete(>completion);
-}
-
  static int keyhash(struct crypto_ahash *tfm, const u8 *key, unsigned int 
keylen,
   u8 *hash)
  {
@@ -2183,10 +2167,10 @@ static int keyhash(struct crypto_ahash *tfm, const u8 
*key, unsigned int keylen,
  
  	struct scatterlist sg[1];

struct ahash_request *req;
-   struct keyhash_result hresult;
+   struct crypto_wait wait;
int ret;
  
-	init_completion();

+   crypto_init_wait();
  
  	req = ahash_request_alloc(tfm, GFP_KERNEL);

if (!req)
@@ -2195,25 +2179,13 @@ static int keyhash(struct crypto_ahash *tfm, const u8 
*key, unsigned int keylen,
/* Keep tfm keylen == 0 during hash of the long key */
ctx->keylen = 0;
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-  keyhash_complete, );
+  crypto_req_done, );
  
  	sg_init_one([0], key, keylen);
  
  	ahash_request_set_crypt(req, sg, hash, keylen);

-   ret = crypto_ahash_digest(req);
-   switch (ret) {
-   case 0:
-   break;
-   case -EINPROGRESS:
-   case -EBUSY:
-   ret = wait_for_completion_interruptible(
-   );
-   if (!ret)
-   ret = hresult.err;
-   break;
-   default:
-   break;
-   }
+   ret = crypto_wait_req(crypto_ahash_digest(req), );
+
ahash_request_free(req);
  
  	return ret;

[PATCH v7 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 nedded for fallback.

Signed-off-by: Kamil Konieczny 
---
 drivers/crypto/Kconfig   |   14 +
 drivers/crypto/s5p-sss.c | 1407 +-
 2 files changed, 1411 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..c9feccf21efe 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -1,18 +1,21 @@
 /*
  * 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 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -30,28 +33,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 +88,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 +163,80 @@
 #define AES_KEY_LEN16
 #define CRYPTO_QUEUE_LEN   1
 
+/* HASH registers */
+#define SSS_REG_HASH_CTRL  0x00
+
+#define SSS_HASH_USER_IV_ENBIT(5)
+#define SSS_HASH_INIT_BIT 

[PATCH v7 1/2] crypto: s5p-sss: change spaces into tabs in defines

[Kamil Konieczny]

change spaces into tabs in defines

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 SSS_HASHIN_INDEPENDENT _SBF(0, 0x00)
+#define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01)
+#define 

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

[Kamil Konieczny]

First patch cleans up spaces in defines, second adds HASH support for Exynos.
Changes:

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):
  change spaces into tabs in defines
  Add HASH support for Exynos

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

-- 
2.14.1.536.g6867272d5b56

Re: [PATCH v9 00/20] simplify crypto wait for async op

[Gilad Ben-Yossef]

On Sun, Oct 15, 2017 at 6:38 PM, Herbert Xu  wrote:
>
> On Sun, Oct 15, 2017 at 10:19:45AM +0100, Gilad Ben-Yossef wrote:
> >
> > Changes from v8:
> > - Remove the translation of EAGAIN return code to the
> >   previous return code of EBUSY for the user space
> >   interface of algif as no one seems to rely on it as
> >   requested by Herbert Xu.
>
> Sorry, but I forgot to mention that EAGAIN is not a good value
> to use because it's used by the network system calls for other
> meanings (interrupted by a signal).
>
> So if we stop doing the translation then we also need to pick
> a different value, perhaps E2BIG or something similar that have
> no current use within the crypto API or network API.

Yes, I see what you mean. With a netlink based interface this can
be confusing to debug.

Would you mind if we used ENOSPC instead of E2BIG?

"No space left on device" seems more appropriate than
"Argument list too long".

Thanks,
Gilad




-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

Re: [PATCH v9 00/20] simplify crypto wait for async op

[Russell King - ARM Linux]

On Sun, Oct 15, 2017 at 10:19:45AM +0100, Gilad Ben-Yossef wrote:
> Many users of kernel async. crypto services have a pattern of
> starting an async. crypto op and than using a completion
> to wait for it to end.
> 
> This patch set simplifies this common use case in two ways:
> 
> First, by separating the return codes of the case where a
> request is queued to a backlog due to the provider being
> busy (-EBUSY) from the case the request has failed due
> to the provider being busy and backlogging is not enabled
> (-EAGAIN).
> 
> Next, this change is than built on to create a generic API
> to wait for a async. crypto operation to complete.
> 
> The end result is a smaller code base and an API that is
> easier to use and more difficult to get wrong.
> 
> The patch set was boot tested on x86_64 and arm64 which
> at the very least tests the crypto users via testmgr and
> tcrypt but I do note that I do not have access to some
> of the HW whose drivers are modified nor do I claim I was
> able to test all of the corner cases.
> 
> The patch set is based upon linux-next release tagged
> next-20171013.

Has there been any performance impact analysis of these changes?  I
ended up with patches for one of the crypto drivers which converted
its interrupt handling to threaded interrupts being reverted because
it caused a performance degredation.

Moving code to latest APIs to simplify it is not always beneficial.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH v2 7/8] scsi/pmcraid: Remove an unused structure member

[Bart Van Assche]

On Tue, 2017-10-17 at 08:21 +0200, Hannes Reinecke wrote:
> On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> > Signed-off-by: Bart Van Assche 
> > Reviewed-by: Johannes Thumshirn 
> > Cc: linux-s...@vger.kernel.org
> > Cc: Martin K. Petersen 
> > Cc: Anil Ravindranath 
> > ---
> >  drivers/scsi/pmcraid.h | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/scsi/pmcraid.h b/drivers/scsi/pmcraid.h
> > index 8bfac72a242b..44da91712115 100644
> > --- a/drivers/scsi/pmcraid.h
> > +++ b/drivers/scsi/pmcraid.h
> > @@ -542,7 +542,6 @@ struct pmcraid_sglist {
> > u32 order;
> > u32 num_sg;
> > u32 num_dma_sg;
> > -   u32 buffer_len;
> > struct scatterlist scatterlist[1];
> >  };
> >  
> > 
> 
> This actually is the same story that we've had with ipr (and, looking at
> the code, those two drivers look awfully similar ...).
> pmcraid_sglist looks as if it's a hardware-dependent structure, so just
> removing one entry from the middle of a structure might not be a good idea.
> But this is something for the pmcraid folks to clarify.

Hello Hannes,

Sorry but I don't see how a structure that contains a struct scatterlist
could be hardware-dependent?

Thanks,

Bart.

Re: [PATCH v2 5/8] target: Use sgl_alloc_order() and sgl_free()

[Bart Van Assche]

On Tue, 2017-10-17 at 08:14 +0200, Hannes Reinecke wrote:
> On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> > [ ... ]
> >  void target_free_sgl(struct scatterlist *sgl, int nents)
> >  {
> > -   struct scatterlist *sg;
> > -   int count;
> > -
> > -   for_each_sg(sgl, sg, nents, count)
> > -   __free_page(sg_page(sg));
> > -
> > -   kfree(sgl);
> > +   sgl_free(sgl);
> >  }
> >  EXPORT_SYMBOL(target_free_sgl);
> >  
> > @@ -2423,42 +2417,10 @@ int
> >  target_alloc_sgl(struct scatterlist **sgl, unsigned int *nents, u32 length,
> >  bool zero_page, bool chainable)
> >  {
> > -   [ ... ]
> > +   *sgl = sgl_alloc_order(length, 0, chainable, gfp, nents);
> > +   return *sgl ? 0 : -ENOMEM;
> >  }
> >  EXPORT_SYMBOL(target_alloc_sgl);
> >  
> > The calling convention from target_alloc_sgl() is decidedly dodgy.
> 
> Can't we convert it into returning the sgl, and remove the first parameter?

Hello Hannes,

Another option is to remove the target_alloc_sgl() and target_free_sgl() 
functions
and to make LIO target drivers call sgl_alloc_order() and sgl_free_order() 
directly.
Do you perhaps have a preference for one of these two approaches?

Thanks,

Bart.

Re: New Linux accelerators discussion list [was: Re: Fostering linux community collaboration on hardware accelerators]

[Jonathan Cameron]

+ linux-accelerat...@lists.ozlabs.org

Seems sensible to have this email actually go to the new list so
at least it appears in the archive.

Sorry all, I should have thought of this before pressing send,

Jonathan

On Tue, 17 Oct 2017 13:48:10 +0100
Jonathan Cameron  wrote:

> On Tue, 17 Oct 2017 11:00:40 +1100
> Andrew Donnellan  wrote:
> 
> > On 17/10/17 01:07, Jonathan Cameron wrote:  
> > > 
> > > 
> > >>> So as ever with a linux community focusing on a particular topic, the
> > >>> obvious solution is a mailing list. There are a number of options on how
> > >>> do this.
> > >>>
> > >>> 1) Ask one of the industry bodies to host? Who?
> > >>>
> > >>> 2) Put together a compelling argument for 
> > >>> linux-accelerat...@vger.kernel.org
> > >>> as probably the most generic location for such a list.
> > >>
> > >> Happy to offer linux-accelerat...@lists.ozlabs.org, which I can get set
> > >> up immediately (and if we want patchwork, patchwork.ozlabs.org is
> > >> available as always, no matter where the list is hosted).
> > >>
> > > 
> > > That would be great! Thanks for doing this. Much easier to find out what
> > > such a list is useful for by the practical option of having a list and
> > > see what people do with it.
> > 
> > [+ LKML]
> > 
> > We now have a mailing list:
> > 
> >List:  linux-accelerat...@lists.ozlabs.org
> >Info:  https://lists.ozlabs.org/listinfo/linux-accelerators
> >Archives:  https://lists.ozlabs.org/pipermail/linux-accelerators
> > 
> > I haven't set up Patchwork as yet, but if people think that's a good 
> > idea I can get that done too.
> > 
> > 
> > Andrew
> >   
> 
> Thanks Andrew.
> 
> A quick summary of initial thoughts on scope of this list for anyone
> entering the discussion at this point.
> Note it will hopefully evolve in whatever direction people find helpful.
> This contains some suggestions not a barrier to wider scope!
> 
> There are a number of projects / applications involving what are
> termed hardware accelerators.  These include:
> * Traditional offload engines
>   - Crypto, compression, media transcoding and similar accelerators
>   - usecases including kTLS, Storage Encryption etc.
> * Dynamic FPGA type accelerators
> * ODP, DPDK and similar networking data plane - particularly dual stack
>   solutions where the kernel 'plays nicely' with userspace drivers.
> * AI Accelerators
> * Shared Virtual Memory (SVM) bus systems including Open-CAPI, CCIX etc
> * Fast data flow to/from userspace applications.
> 
> A number of existing project focus on these:
> * Mainline kernel drivers
>   - Numerous crypto drivers etc
>   - Open-CAPI
> * Various networking data plane activities
> * VFIO based and similar userspace drivers
> Hopefully this list can provide a broader forum where more general
> questions are being considered.
> 
> The discussion that lead to this list was that a number of people would
> like a general open forum on which to discuss ideas with scope beyond
> simply one kernel subsystem or one particular userspace framework.
> 
> Topics might include
> * RFCs and early reviews of new approaches.
> * Similar hardware - who is trying to solve the same problems?
> * What would we ideally want from new hardware iterations?
> * Hardware description - the question of how to chose a particular
>   crypto engine is very dependent on the particular data flows.
>   Sometimes hardware accelerators don't actually help due to overheads.
>   Understanding those barriers would be very useful.
> * Upstream paths - blockers and how to work with the communities to
>   overcome them.
> * Fostering stronger userspace communities to allow these accelerators to 
>   be easily harnessed.
>   - A number of projects have been highlighted in this thread
> OpenStack (cyborg project), openMP accelerator support 
> * Robustness / security of userspace frameworks.
> * Virtualisation of accelerators
> 
> Anyhow, this email was just meant to draw together some thoughts.
> It will be interesting to see what the list actually gets used for :)
> 
> Jonathan

Re: New Linux accelerators discussion list [was: Re: Fostering linux community collaboration on hardware accelerators]

[Jonathan Cameron]

On Tue, 17 Oct 2017 11:00:40 +1100
Andrew Donnellan  wrote:

> On 17/10/17 01:07, Jonathan Cameron wrote:
> > 
> >   
> >>> So as ever with a linux community focusing on a particular topic, the
> >>> obvious solution is a mailing list. There are a number of options on how
> >>> do this.
> >>>
> >>> 1) Ask one of the industry bodies to host? Who?
> >>>
> >>> 2) Put together a compelling argument for 
> >>> linux-accelerat...@vger.kernel.org
> >>> as probably the most generic location for such a list.  
> >>
> >> Happy to offer linux-accelerat...@lists.ozlabs.org, which I can get set
> >> up immediately (and if we want patchwork, patchwork.ozlabs.org is
> >> available as always, no matter where the list is hosted).
> >>  
> > 
> > That would be great! Thanks for doing this. Much easier to find out what
> > such a list is useful for by the practical option of having a list and
> > see what people do with it.  
> 
> [+ LKML]
> 
> We now have a mailing list:
> 
>List:  linux-accelerat...@lists.ozlabs.org
>Info:  https://lists.ozlabs.org/listinfo/linux-accelerators
>Archives:  https://lists.ozlabs.org/pipermail/linux-accelerators
> 
> I haven't set up Patchwork as yet, but if people think that's a good 
> idea I can get that done too.
> 
> 
> Andrew
> 

Thanks Andrew.

A quick summary of initial thoughts on scope of this list for anyone
entering the discussion at this point.
Note it will hopefully evolve in whatever direction people find helpful.
This contains some suggestions not a barrier to wider scope!

There are a number of projects / applications involving what are
termed hardware accelerators.  These include:
* Traditional offload engines
- Crypto, compression, media transcoding and similar accelerators
- usecases including kTLS, Storage Encryption etc.
* Dynamic FPGA type accelerators
* ODP, DPDK and similar networking data plane - particularly dual stack
  solutions where the kernel 'plays nicely' with userspace drivers.
* AI Accelerators
* Shared Virtual Memory (SVM) bus systems including Open-CAPI, CCIX etc
* Fast data flow to/from userspace applications.

A number of existing project focus on these:
* Mainline kernel drivers
- Numerous crypto drivers etc
- Open-CAPI
* Various networking data plane activities
* VFIO based and similar userspace drivers
Hopefully this list can provide a broader forum where more general
questions are being considered.

The discussion that lead to this list was that a number of people would
like a general open forum on which to discuss ideas with scope beyond
simply one kernel subsystem or one particular userspace framework.

Topics might include
* RFCs and early reviews of new approaches.
* Similar hardware - who is trying to solve the same problems?
* What would we ideally want from new hardware iterations?
* Hardware description - the question of how to chose a particular
  crypto engine is very dependent on the particular data flows.
  Sometimes hardware accelerators don't actually help due to overheads.
  Understanding those barriers would be very useful.
* Upstream paths - blockers and how to work with the communities to
  overcome them.
* Fostering stronger userspace communities to allow these accelerators to 
  be easily harnessed.
- A number of projects have been highlighted in this thread
  OpenStack (cyborg project), openMP accelerator support 
* Robustness / security of userspace frameworks.
* Virtualisation of accelerators

Anyhow, this email was just meant to draw together some thoughts.
It will be interesting to see what the list actually gets used for :)

Jonathan

Re: [PATCH v2 2/8] crypto: scompress - use sgl_alloc() and sgl_free()

[Hannes Reinecke]

On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> Use the sgl_alloc() and sgl_free() functions instead of open coding
> these functions.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Ard Biesheuvel 
> Cc: Herbert Xu 
> ---
>  crypto/Kconfig |  1 +
>  crypto/scompress.c | 51 ++-
>  2 files changed, 3 insertions(+), 49 deletions(-)
> Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 1/8] lib/scatterlist: Introduce sgl_alloc() and sgl_free()

[Hannes Reinecke]

On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> Many kernel drivers contain code that allocates and frees both a
> scatterlist and the pages that populate that scatterlist.
> Introduce functions in lib/scatterlist.c that perform these tasks
> instead of duplicating this functionality in multiple drivers.
> Only include these functions in the build if CONFIG_SGL_ALLOC=y
> to avoid that the kernel size increases if this functionality is
> not used.
> 
> Signed-off-by: Bart Van Assche 
> ---
>  include/linux/scatterlist.h |  10 +
>  lib/Kconfig |   4 ++
>  lib/scatterlist.c   | 105 
> 
>  3 files changed, 119 insertions(+)
> 

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 3/8] nvmet/fc: Use sgl_alloc() and sgl_free()

[Hannes Reinecke]

On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> Use the sgl_alloc() and sgl_free() functions instead of open coding
> these functions.
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Johannes Thumshirn 
> Cc: Keith Busch 
> Cc: Christoph Hellwig 
> Cc: James Smart 
> Cc: Sagi Grimberg 
> ---
>  drivers/nvme/target/Kconfig |  1 +
>  drivers/nvme/target/fc.c| 36 ++--
>  2 files changed, 3 insertions(+), 34 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 4/8] nvmet/rdma: Use sgl_alloc() and sgl_free()

[Hannes Reinecke]

On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> Use the sgl_alloc() and sgl_free() functions instead of open coding
> these functions.
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Johannes Thumshirn 
> Cc: Keith Busch 
> Cc: Christoph Hellwig 
> Cc: Sagi Grimberg 
> ---
>  drivers/nvme/target/Kconfig |  1 +
>  drivers/nvme/target/rdma.c  | 63 
> +++--
>  2 files changed, 5 insertions(+), 59 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 5/8] target: Use sgl_alloc_order() and sgl_free()

[Hannes Reinecke]

On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> Use the sgl_alloc_order() and sgl_free() functions instead of open
> coding these functions.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Nicholas A. Bellinger 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Sagi Grimberg 
> ---
>  drivers/target/Kconfig |  1 +
>  drivers/target/target_core_transport.c | 46 
> +++---
>  2 files changed, 5 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig
> index e2bc99980f75..4c44d7bed01a 100644
> --- a/drivers/target/Kconfig
> +++ b/drivers/target/Kconfig
> @@ -5,6 +5,7 @@ menuconfig TARGET_CORE
>   select CONFIGFS_FS
>   select CRC_T10DIF
>   select BLK_SCSI_REQUEST # only for scsi_command_size_tbl..
> + select SGL_ALLOC
>   default n
>   help
>   Say Y or M here to enable the TCM Storage Engine and ConfigFS enabled
> diff --git a/drivers/target/target_core_transport.c 
> b/drivers/target/target_core_transport.c
> index 836d552b0385..9bbd08be9d60 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -2293,13 +2293,7 @@ static void target_complete_ok_work(struct work_struct 
> *work)
>  
>  void target_free_sgl(struct scatterlist *sgl, int nents)
>  {
> - struct scatterlist *sg;
> - int count;
> -
> - for_each_sg(sgl, sg, nents, count)
> - __free_page(sg_page(sg));
> -
> - kfree(sgl);
> + sgl_free(sgl);
>  }
>  EXPORT_SYMBOL(target_free_sgl);
>  
> @@ -2423,42 +2417,10 @@ int
>  target_alloc_sgl(struct scatterlist **sgl, unsigned int *nents, u32 length,
>bool zero_page, bool chainable)
>  {
> - struct scatterlist *sg;
> - struct page *page;
> - gfp_t zero_flag = (zero_page) ? __GFP_ZERO : 0;
> - unsigned int nalloc, nent;
> - int i = 0;
> -
> - nalloc = nent = DIV_ROUND_UP(length, PAGE_SIZE);
> - if (chainable)
> - nalloc++;
> - sg = kmalloc_array(nalloc, sizeof(struct scatterlist), GFP_KERNEL);
> - if (!sg)
> - return -ENOMEM;
> + gfp_t gfp = GFP_KERNEL | (zero_page ? __GFP_ZERO : 0);
>  
> - sg_init_table(sg, nalloc);
> -
> - while (length) {
> - u32 page_len = min_t(u32, length, PAGE_SIZE);
> - page = alloc_page(GFP_KERNEL | zero_flag);
> - if (!page)
> - goto out;
> -
> - sg_set_page([i], page, page_len, 0);
> - length -= page_len;
> - i++;
> - }
> - *sgl = sg;
> - *nents = nent;
> - return 0;
> -
> -out:
> - while (i > 0) {
> - i--;
> - __free_page(sg_page([i]));
> - }
> - kfree(sg);
> - return -ENOMEM;
> + *sgl = sgl_alloc_order(length, 0, chainable, gfp, nents);
> + return *sgl ? 0 : -ENOMEM;
>  }
>  EXPORT_SYMBOL(target_alloc_sgl);
>  
> The calling convention from target_alloc_sgl() is decidedly dodgy.
Can't we convert it into returning the sgl, and remove the first parameter?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 6/8] scsi/ipr: Use sgl_alloc_order() and sgl_free_order()

[Hannes Reinecke]

On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> Use the sgl_alloc_order() and sgl_free_order() functions instead
> of open coding these functions.
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Johannes Thumshirn 
> Cc: linux-s...@vger.kernel.org
> Cc: Martin K. Petersen 
> Cc: Brian King 
> ---
>  drivers/scsi/Kconfig |  1 +
>  drivers/scsi/ipr.c   | 49 -
>  drivers/scsi/ipr.h   |  2 +-
>  3 files changed, 10 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 41366339b950..d11e75e76195 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -1058,6 +1058,7 @@ config SCSI_IPR
>   depends on PCI && SCSI && ATA
>   select FW_LOADER
>   select IRQ_POLL
> + select SGL_ALLOC
>   ---help---
> This driver supports the IBM Power Linux family RAID adapters.
> This includes IBM pSeries 5712, 5703, 5709, and 570A, as well
> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index f838bd73befa..6fed5db6255e 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -3815,10 +3815,8 @@ static struct device_attribute ipr_iopoll_weight_attr 
> = {
>   **/
>  static struct ipr_sglist *ipr_alloc_ucode_buffer(int buf_len)
>  {
> - int sg_size, order, bsize_elem, num_elem, i, j;
> + int sg_size, order;
>   struct ipr_sglist *sglist;
> - struct scatterlist *scatterlist;
> - struct page *page;
>  
>   /* Get the minimum size per scatter/gather element */
>   sg_size = buf_len / (IPR_MAX_SGLIST - 1);
> @@ -3826,45 +3824,18 @@ static struct ipr_sglist *ipr_alloc_ucode_buffer(int 
> buf_len)
>   /* Get the actual size per element */
>   order = get_order(sg_size);
>  
> - /* Determine the actual number of bytes per element */
> - bsize_elem = PAGE_SIZE * (1 << order);
> -
> - /* Determine the actual number of sg entries needed */
> - if (buf_len % bsize_elem)
> - num_elem = (buf_len / bsize_elem) + 1;
> - else
> - num_elem = buf_len / bsize_elem;
> -
>   /* Allocate a scatter/gather list for the DMA */
> - sglist = kzalloc(sizeof(struct ipr_sglist) +
> -  (sizeof(struct scatterlist) * (num_elem - 1)),
> -  GFP_KERNEL);
> -
> + sglist = kzalloc(sizeof(struct ipr_sglist), GFP_KERNEL);
>   if (sglist == NULL) {
>   ipr_trace;
>   return NULL;
>   }
> -
> - scatterlist = sglist->scatterlist;
> - sg_init_table(scatterlist, num_elem);
> -
>   sglist->order = order;
> - sglist->num_sg = num_elem;
> -
> - /* Allocate a bunch of sg elements */
> - for (i = 0; i < num_elem; i++) {
> - page = alloc_pages(GFP_KERNEL, order);
> - if (!page) {
> - ipr_trace;
> -
> - /* Free up what we already allocated */
> - for (j = i - 1; j >= 0; j--)
> - __free_pages(sg_page([j]), order);
> - kfree(sglist);
> - return NULL;
> - }
> -
> - sg_set_page([i], page, 0, 0);
> + sglist->scatterlist = sgl_alloc_order(buf_len, order, false, GFP_KERNEL,
> +   >num_sg);
> + if (!sglist->scatterlist) {
> + kfree(sglist);
> + return NULL;
>   }
>  
>   return sglist;
> @@ -3882,11 +3853,7 @@ static struct ipr_sglist *ipr_alloc_ucode_buffer(int 
> buf_len)
>   **/
>  static void ipr_free_ucode_buffer(struct ipr_sglist *sglist)
>  {
> - int i;
> -
> - for (i = 0; i < sglist->num_sg; i++)
> - __free_pages(sg_page(>scatterlist[i]), sglist->order);
> -
> + sgl_free_order(sglist->scatterlist, sglist->order);
>   kfree(sglist);
>  }
>  
> diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h
> index c7f0e9e3cd7d..93570734cbfb 100644
> --- a/drivers/scsi/ipr.h
> +++ b/drivers/scsi/ipr.h
> @@ -1454,7 +1454,7 @@ struct ipr_sglist {
>   u32 num_sg;
>   u32 num_dma_sg;
>   u32 buffer_len;
> - struct scatterlist scatterlist[1];
> + struct scatterlist *scatterlist;
>  };
>  
>  enum ipr_sdt_state {
> 
Not sure if this is a valid conversion.
Originally the driver would allocate a single buffer; with this buffer
we have two distinct buffers.
Given that this is used to download the microcode I'm not sure if this
isn't a hardware-dependent structure which requires a single buffer
including the sglist.
Brian, can you shed some light here?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 7/8] scsi/pmcraid: Remove an unused structure member

[Hannes Reinecke]

On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Johannes Thumshirn 
> Cc: linux-s...@vger.kernel.org
> Cc: Martin K. Petersen 
> Cc: Anil Ravindranath 
> ---
>  drivers/scsi/pmcraid.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/scsi/pmcraid.h b/drivers/scsi/pmcraid.h
> index 8bfac72a242b..44da91712115 100644
> --- a/drivers/scsi/pmcraid.h
> +++ b/drivers/scsi/pmcraid.h
> @@ -542,7 +542,6 @@ struct pmcraid_sglist {
>   u32 order;
>   u32 num_sg;
>   u32 num_dma_sg;
> - u32 buffer_len;
>   struct scatterlist scatterlist[1];
>  };
>  
> 
This actually is the same story that we've had with ipr (and, looking at
the code, those two drivers look awfully similar ...).
pmcraid_sglist looks as if it's a hardware-dependent structure, so just
removing one entry from the middle of a structure might not be a good idea.
But this is something for the pmcraid folks to clarify.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 8/8] scsi/pmcraid: Use sgl_alloc_order() and sgl_free_order()

[Hannes Reinecke]

On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> Use the sgl_alloc_order() and sgl_free_order() functions instead
> of open coding these functions.
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Johannes Thumshirn 
> Cc: linux-s...@vger.kernel.org
> Cc: Martin K. Petersen 
> Cc: Anil Ravindranath 
> ---
>  drivers/scsi/Kconfig   |  1 +
>  drivers/scsi/pmcraid.c | 43 ---
>  drivers/scsi/pmcraid.h |  2 +-
>  3 files changed, 6 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index d11e75e76195..632200ec36a6 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -1576,6 +1576,7 @@ config ZFCP
>  config SCSI_PMCRAID
>   tristate "PMC SIERRA Linux MaxRAID adapter support"
>   depends on PCI && SCSI && NET
> + select SGL_ALLOC
>   ---help---
> This driver supports the PMC SIERRA MaxRAID adapters.
>  
> diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
> index b4d6cd8cd1ad..95fc0352f9bb 100644
> --- a/drivers/scsi/pmcraid.c
> +++ b/drivers/scsi/pmcraid.c
> @@ -3232,12 +3232,7 @@ static int pmcraid_build_ioadl(
>   */
>  static void pmcraid_free_sglist(struct pmcraid_sglist *sglist)
>  {
> - int i;
> -
> - for (i = 0; i < sglist->num_sg; i++)
> - __free_pages(sg_page(&(sglist->scatterlist[i])),
> -  sglist->order);
> -
> + sgl_free_order(sglist->scatterlist, sglist->order);
>   kfree(sglist);
>  }
>  
> @@ -3254,50 +3249,20 @@ static void pmcraid_free_sglist(struct pmcraid_sglist 
> *sglist)
>  static struct pmcraid_sglist *pmcraid_alloc_sglist(int buflen)
>  {
>   struct pmcraid_sglist *sglist;
> - struct scatterlist *scatterlist;
> - struct page *page;
> - int num_elem, i, j;
>   int sg_size;
>   int order;
> - int bsize_elem;
>  
>   sg_size = buflen / (PMCRAID_MAX_IOADLS - 1);
>   order = (sg_size > 0) ? get_order(sg_size) : 0;
> - bsize_elem = PAGE_SIZE * (1 << order);
> -
> - /* Determine the actual number of sg entries needed */
> - if (buflen % bsize_elem)
> - num_elem = (buflen / bsize_elem) + 1;
> - else
> - num_elem = buflen / bsize_elem;
>  
>   /* Allocate a scatter/gather list for the DMA */
> - sglist = kzalloc(sizeof(struct pmcraid_sglist) +
> -  (sizeof(struct scatterlist) * (num_elem - 1)),
> -  GFP_KERNEL);
> -
> + sglist = kzalloc(sizeof(struct pmcraid_sglist), GFP_KERNEL);
>   if (sglist == NULL)
>   return NULL;
>  
> - scatterlist = sglist->scatterlist;
> - sg_init_table(scatterlist, num_elem);
>   sglist->order = order;
> - sglist->num_sg = num_elem;
> - sg_size = buflen;
> -
> - for (i = 0; i < num_elem; i++) {
> - page = alloc_pages(GFP_KERNEL|GFP_DMA|__GFP_ZERO, order);
> - if (!page) {
> - for (j = i - 1; j >= 0; j--)
> - __free_pages(sg_page([j]), order);
> - kfree(sglist);
> - return NULL;
> - }
> -
> - sg_set_page([i], page,
> - sg_size < bsize_elem ? sg_size : bsize_elem, 0);
> - sg_size -= bsize_elem;
> - }
> + sgl_alloc_order(buflen, order, false,
> + GFP_KERNEL | GFP_DMA | __GFP_ZERO, >num_sg);
>  
>   return sglist;
>  }
> diff --git a/drivers/scsi/pmcraid.h b/drivers/scsi/pmcraid.h
> index 44da91712115..754ef30927e2 100644
> --- a/drivers/scsi/pmcraid.h
> +++ b/drivers/scsi/pmcraid.h
> @@ -542,7 +542,7 @@ struct pmcraid_sglist {
>   u32 order;
>   u32 num_sg;
>   u32 num_dma_sg;
> - struct scatterlist scatterlist[1];
> + struct scatterlist *scatterlist;
>  };
>  
>  /* page D0 inquiry data of focal point resource */
> 
The comment to ipr applied here, too.
We need input from the pmcraid folks if this is a valid conversion.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 1/2] x86/crypto/sha256-mb: fix panic due to unaligned access

[Tim Chen]

On 10/16/2017 08:51 AM, Andrey Ryabinin wrote:
> struct sha256_ctx_mgr allocated in sha256_mb_mod_init() via kzalloc()
> and later passed in sha256_mb_flusher_mgr_flush_avx2() function where
> instructions vmovdqa used to access the struct. vmovdqa requires
> 16-bytes aligned argument, but nothing guarantees that struct
> sha256_ctx_mgr will have that alignment. Unaligned vmovdqa will
> generate GP fault.
> 
> Fix this by replacing vmovdqa with vmovdqu which doesn't have alignment
> requirements.

Using vmovdqu will be a bit slower if the structure is unaligned.
However, flush is done on the non performance critical path so
I don't expect this will be an issue to performance.  

Thanks.

Tim

Acked-by: Tim Chen 

> 
> Fixes: a377c6b1876e ("crypto: sha256-mb - submit/flush routines for AVX2")
> Reported-by: Josh Poimboeuf 
> Signed-off-by: Andrey Ryabinin 
> Cc: 
> ---
>  arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S 
> b/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S
> index 8fe6338bcc84..16c4ccb1f154 100644
> --- a/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S
> +++ b/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S
> @@ -155,8 +155,8 @@ LABEL skip_ %I
>  .endr
>  
>   # Find min length
> - vmovdqa _lens+0*16(state), %xmm0
> - vmovdqa _lens+1*16(state), %xmm1
> + vmovdqu _lens+0*16(state), %xmm0
> + vmovdqu _lens+1*16(state), %xmm1
>  
>   vpminud %xmm1, %xmm0, %xmm2 # xmm2 has {D,C,B,A}
>   vpalignr $8, %xmm2, %xmm3, %xmm3# xmm3 has {x,x,D,C}
> @@ -176,8 +176,8 @@ LABEL skip_ %I
>   vpsubd  %xmm2, %xmm0, %xmm0
>   vpsubd  %xmm2, %xmm1, %xmm1
>  
> - vmovdqa %xmm0, _lens+0*16(state)
> - vmovdqa %xmm1, _lens+1*16(state)
> + vmovdqu %xmm0, _lens+0*16(state)
> + vmovdqu %xmm1, _lens+1*16(state)
>  
>   # "state" and "args" are the same address, arg1
>   # len is arg2
> @@ -234,8 +234,8 @@ ENTRY(sha256_mb_mgr_get_comp_job_avx2)
>   jc  .return_null
>  
>   # Find min length
> - vmovdqa _lens(state), %xmm0
> - vmovdqa _lens+1*16(state), %xmm1
> + vmovdqu _lens(state), %xmm0
> + vmovdqu _lens+1*16(state), %xmm1
>  
>   vpminud %xmm1, %xmm0, %xmm2 # xmm2 has {D,C,B,A}
>   vpalignr $8, %xmm2, %xmm3, %xmm3# xmm3 has {x,x,D,C}
> 

Re: md5sum (from libkcapi) fails on kernel 4.9 but not on 4.13

[Stephan Mueller]

Am Dienstag, 17. Oktober 2017, 09:58:31 CEST schrieb Christophe LEROY:

Hi Christophe,

> 
> > If you tamper with the code shown above from libkcapi and set
> > alg_max_pages to a low value, the library reverts to sendmsg after the
> > given number of pages.
> Couldn't we get the libkcapi to splice until the last full page, then
> only use sendmsg() for the last chunk once it is smaller than an entire
> page ?

Adding such loop to the libkcapi invocation of sendmsg to send page-wise is 
easy. Yet, I am wondering whether this is the right thing to do. The 
algif_hash uses the standard in-kernel interface to invoke the cipher 
implementation.

Thus, when fixing the issue in user space, the issue is still present in the 
kernel. Thus, if for some reason the kernel wants to hash more than 32kb in a 
row, it will fall with the Talitos driver. Thus, wouldn't it make more sense 
that the loop to process the data in chunks inside the Talitos driver?

Ciao
Stephan

Re: [PATCH] crypto: ccp: remove unused variable qim

[Gary R Hook]

On 10/12/2017 11:55 AM, Colin King wrote:

From: Colin Ian King 

Variable qim is assigned but never read, it is redundant and can
be removed.

Cleans up clang warning: Value stored to 'qim' is never read

Fixes: 4b394a232df7 ("crypto: ccp - Let a v5 CCP provide the same function as 
v3")
Signed-off-by: Colin Ian King 


Acked-by: Gary R Hook 


---
 drivers/crypto/ccp/ccp-dev-v5.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c
index 65604fc65e8f..44a4d2779b15 100644
--- a/drivers/crypto/ccp/ccp-dev-v5.c
+++ b/drivers/crypto/ccp/ccp-dev-v5.c
@@ -788,13 +788,12 @@ static int ccp5_init(struct ccp_device *ccp)
struct ccp_cmd_queue *cmd_q;
struct dma_pool *dma_pool;
char dma_pool_name[MAX_DMAPOOL_NAME_LEN];
-   unsigned int qmr, qim, i;
+   unsigned int qmr, i;
u64 status;
u32 status_lo, status_hi;
int ret;

/* Find available queues */
-   qim = 0;
qmr = ioread32(ccp->io_regs + Q_MASK_REG);
for (i = 0; i < MAX_HW_QUEUES; i++) {

[PATCH v5 05/18] ima: Simplify ima_eventsig_init

[Thiago Jung Bauermann]

The "goto out" statement doesn't have any purpose since there's no cleanup
to be done when returning early, so remove it. This also makes the rc
variable unnecessary so remove it as well.

Also, the xattr_len and fmt variables are redundant so remove them as well.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima_template_lib.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/security/integrity/ima/ima_template_lib.c 
b/security/integrity/ima/ima_template_lib.c
index d941260e979f..e8ec783b6a8d 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -378,16 +378,11 @@ int ima_eventname_ng_init(struct ima_event_data 
*event_data,
 int ima_eventsig_init(struct ima_event_data *event_data,
  struct ima_field_data *field_data)
 {
-   enum data_formats fmt = DATA_FMT_HEX;
struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
-   int xattr_len = event_data->xattr_len;
-   int rc = 0;
 
if (!xattr_value || xattr_value->type != EVM_IMA_XATTR_DIGSIG)
-   goto out;
+   return 0;
 
-   rc = ima_write_template_field_data(xattr_value, xattr_len, fmt,
-  field_data);
-out:
-   return rc;
+   return ima_write_template_field_data(xattr_value, event_data->xattr_len,
+DATA_FMT_HEX, field_data);
 }

[PATCH v5 04/18] evm, ima: Remove more superfluous parentheses

[Thiago Jung Bauermann]

This patch removes unnecessary parentheses from all EVM and IMA files
not yet cleaned up by the previous patches.

It is separate from the previous one so that it can be easily dropped if
the churn and conflict potential is deemed not worth it.

Confirmed that the patch is correct by comparing the object files from
before and after the patch. They are identical.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/evm/evm_posix_acl.c | 8 
 security/integrity/ima/ima_fs.c| 6 +++---
 security/integrity/ima/ima_queue.c | 6 +++---
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/security/integrity/evm/evm_posix_acl.c 
b/security/integrity/evm/evm_posix_acl.c
index 46408b9e62e8..0a32798cfc96 100644
--- a/security/integrity/evm/evm_posix_acl.c
+++ b/security/integrity/evm/evm_posix_acl.c
@@ -17,11 +17,11 @@ int posix_xattr_acl(const char *xattr)
 {
int xattr_len = strlen(xattr);
 
-   if ((strlen(XATTR_NAME_POSIX_ACL_ACCESS) == xattr_len)
-&& (strncmp(XATTR_NAME_POSIX_ACL_ACCESS, xattr, xattr_len) == 0))
+   if (strlen(XATTR_NAME_POSIX_ACL_ACCESS) == xattr_len
+   && strncmp(XATTR_NAME_POSIX_ACL_ACCESS, xattr, xattr_len) == 0)
return 1;
-   if ((strlen(XATTR_NAME_POSIX_ACL_DEFAULT) == xattr_len)
-&& (strncmp(XATTR_NAME_POSIX_ACL_DEFAULT, xattr, xattr_len) == 0))
+   if (strlen(XATTR_NAME_POSIX_ACL_DEFAULT) == xattr_len
+   && strncmp(XATTR_NAME_POSIX_ACL_DEFAULT, xattr, xattr_len) == 0)
return 1;
return 0;
 }
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 4d50b982b453..552f79fc4291 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -105,7 +105,7 @@ static void *ima_measurements_next(struct seq_file *m, void 
*v, loff_t *pos)
rcu_read_unlock();
(*pos)++;
 
-   return (>later == _measurements) ? NULL : qe;
+   return >later == _measurements ? NULL : qe;
 }
 
 static void ima_measurements_stop(struct seq_file *m, void *v)
@@ -141,7 +141,7 @@ int ima_measurements_show(struct seq_file *m, void *v)
if (e == NULL)
return -1;
 
-   template_name = (e->template_desc->name[0] != '\0') ?
+   template_name = e->template_desc->name[0] != '\0' ?
e->template_desc->name : e->template_desc->fmt;
 
/*
@@ -228,7 +228,7 @@ static int ima_ascii_measurements_show(struct seq_file *m, 
void *v)
if (e == NULL)
return -1;
 
-   template_name = (e->template_desc->name[0] != '\0') ?
+   template_name = e->template_desc->name[0] != '\0' ?
e->template_desc->name : e->template_desc->fmt;
 
/* 1st: PCR used (config option) */
diff --git a/security/integrity/ima/ima_queue.c 
b/security/integrity/ima/ima_queue.c
index a02a86d51102..2ef7d33ba1fc 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -60,7 +60,7 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 
*digest_value,
rcu_read_lock();
hlist_for_each_entry_rcu(qe, _htable.queue[key], hnext) {
rc = memcmp(qe->entry->digest, digest_value, TPM_DIGEST_SIZE);
-   if ((rc == 0) && (qe->entry->pcr == pcr)) {
+   if (rc == 0 && qe->entry->pcr == pcr) {
ret = qe;
break;
}
@@ -119,7 +119,7 @@ static int ima_add_digest_entry(struct ima_template_entry 
*entry,
int size;
 
size = get_binary_runtime_size(entry);
-   binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
+   binary_runtime_size = binary_runtime_size < ULONG_MAX - size ?
 binary_runtime_size + size : ULONG_MAX;
}
return 0;
@@ -132,7 +132,7 @@ static int ima_add_digest_entry(struct ima_template_entry 
*entry,
  */
 unsigned long ima_get_binary_runtime_size(void)
 {
-   if (binary_runtime_size >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
+   if (binary_runtime_size >= ULONG_MAX - sizeof(struct ima_kexec_hdr))
return ULONG_MAX;
else
return binary_runtime_size + sizeof(struct ima_kexec_hdr);

[PATCH v5 08/18] integrity: Select CONFIG_KEYS instead of depending on it

[Thiago Jung Bauermann]

This avoids a dependency cycle in CONFIG_IMA_APPRAISE_MODSIG (introduced by
a later patch in this series): it will select CONFIG_MODULE_SIG_FORMAT
which in turn selects CONFIG_KEYS. Kconfig then complains that
CONFIG_INTEGRITY_SIGNATURE depends on CONFIG_KEYS.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index da9565891738..0d642e0317c7 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -17,8 +17,8 @@ if INTEGRITY
 
 config INTEGRITY_SIGNATURE
bool "Digital signature verification using multiple keyrings"
-   depends on KEYS
default n
+   select KEYS
select SIGNATURE
help
  This option enables digital signature verification support

[PATCH v5 06/18] ima: Improvements in ima_appraise_measurement

[Thiago Jung Bauermann]

Replace nested ifs in the EVM xattr verification logic with a switch
statement, making the code easier to understand.

Also, add comments to the if statements in the out section.

Signed-off-by: Mimi Zohar 
Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima.h  |  5 +
 security/integrity/ima/ima_appraise.c | 33 -
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487ad259..4e61d3189b72 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -155,6 +155,11 @@ unsigned long ima_get_binary_runtime_size(void);
 int ima_init_template(void);
 void ima_init_template_list(void);
 
+static inline bool is_ima_sig(const struct evm_ima_xattr_data *xattr_value)
+{
+   return xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG;
+}
+
 /*
  * used to protect h_table and sha_table
  */
diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index 58c6a60c7e83..2c069f47eeec 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -204,7 +204,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 int xattr_len, int opened)
 {
static const char op[] = "appraise_data";
-   char *cause = "unknown";
+   const char *cause = "unknown";
struct dentry *dentry = file_dentry(file);
struct inode *inode = d_backing_inode(dentry);
enum integrity_status status = INTEGRITY_UNKNOWN;
@@ -229,11 +229,16 @@ int ima_appraise_measurement(enum ima_hooks func,
}
 
status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
-   if (status != INTEGRITY_PASS && status != INTEGRITY_UNKNOWN) {
-   if (status == INTEGRITY_NOLABEL || status == INTEGRITY_NOXATTRS)
-   cause = "missing-HMAC";
-   else if (status == INTEGRITY_FAIL)
-   cause = "invalid-HMAC";
+   switch (status) {
+   case INTEGRITY_PASS:
+   case INTEGRITY_UNKNOWN:
+   break;
+   case INTEGRITY_NOXATTRS:/* No EVM protected xattrs. */
+   case INTEGRITY_NOLABEL: /* No security.evm xattr. */
+   cause = "missing-HMAC";
+   goto out;
+   case INTEGRITY_FAIL:/* Invalid HMAC/signature. */
+   cause = "invalid-HMAC";
goto out;
}
switch (xattr_value->type) {
@@ -287,17 +292,19 @@ int ima_appraise_measurement(enum ima_hooks func,
 
 out:
if (status != INTEGRITY_PASS) {
+   /* Fix mode, but don't replace file signatures. */
if ((ima_appraise & IMA_APPRAISE_FIX) &&
-   (!xattr_value ||
-xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
+   !is_ima_sig(xattr_value)) {
if (!ima_fix_xattr(dentry, iint))
status = INTEGRITY_PASS;
-   } else if (inode->i_size == 0 &&
-  (iint->flags & IMA_NEW_FILE) &&
-  xattr_value &&
-  xattr_value->type == EVM_IMA_XATTR_DIGSIG) {
+   }
+
+   /* Permit new files with file signatures, but without data. */
+   if (inode->i_size == 0 && (iint->flags & IMA_NEW_FILE) &&
+   is_ima_sig(xattr_value)) {
status = INTEGRITY_PASS;
}
+
integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
op, cause, rc, 0);
} else {
@@ -404,7 +411,7 @@ int ima_inode_setxattr(struct dentry *dentry, const char 
*xattr_name,
if (!xattr_value_len || xvalue->type >= IMA_XATTR_LAST)
return -EINVAL;
ima_reset_appraise_flags(d_backing_inode(dentry),
-   xvalue->type == EVM_IMA_XATTR_DIGSIG);
+is_ima_sig(xvalue));
result = 0;
}
return result;

[PATCH v5 02/18] ima: Remove some superfluous parentheses

[Thiago Jung Bauermann]

Superfluous parentheses just add clutter to the code, making it harder to
read and to understand.

In order to avoid churn and minimize conflicts with other patches from the
community, this patch only removes superfluous parentheses from lines that
are modified by other patches in this series.

Confirmed that the patch is correct by comparing the object files from
before and after the patch. They are identical.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima_appraise.c | 11 +--
 security/integrity/ima/ima_template_lib.c |  2 +-
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index ec7dfa02c051..bce0b36778bd 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -229,9 +229,8 @@ int ima_appraise_measurement(enum ima_hooks func,
}
 
status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
-   if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
-   if ((status == INTEGRITY_NOLABEL)
-   || (status == INTEGRITY_NOXATTRS))
+   if (status != INTEGRITY_PASS && status != INTEGRITY_UNKNOWN) {
+   if (status == INTEGRITY_NOLABEL || status == INTEGRITY_NOXATTRS)
cause = "missing-HMAC";
else if (status == INTEGRITY_FAIL)
cause = "invalid-HMAC";
@@ -293,10 +292,10 @@ int ima_appraise_measurement(enum ima_hooks func,
 xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
if (!ima_fix_xattr(dentry, iint))
status = INTEGRITY_PASS;
-   } else if ((inode->i_size == 0) &&
+   } else if (inode->i_size == 0 &&
   (iint->flags & IMA_NEW_FILE) &&
-  (xattr_value &&
-   xattr_value->type == EVM_IMA_XATTR_DIGSIG)) {
+  xattr_value &&
+  xattr_value->type == EVM_IMA_XATTR_DIGSIG) {
status = INTEGRITY_PASS;
}
integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
diff --git a/security/integrity/ima/ima_template_lib.c 
b/security/integrity/ima/ima_template_lib.c
index 28af43f63572..8bebcbb61162 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -383,7 +383,7 @@ int ima_eventsig_init(struct ima_event_data *event_data,
int xattr_len = event_data->xattr_len;
int rc = 0;
 
-   if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG))
+   if (!xattr_value || xattr_value->type != EVM_IMA_XATTR_DIGSIG)
goto out;
 
rc = ima_write_template_field_data(xattr_value, xattr_len, fmt,

[PATCH v5 14/18] integrity: Introduce integrity_keyring_from_id

[Thiago Jung Bauermann]

IMA will need to obtain the keyring used to verify file signatures so that
it can verify the module-style signature appended to files.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/digsig.c| 28 +++-
 security/integrity/integrity.h |  1 +
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 6f9e4ce568cd..935f1b3258e7 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -48,11 +48,10 @@ static bool init_keyring __initdata;
 #define restrict_link_to_ima restrict_link_by_builtin_trusted
 #endif
 
-int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
-   const char *digest, int digestlen)
+struct key *integrity_keyring_from_id(const unsigned int id)
 {
-   if (id >= INTEGRITY_KEYRING_MAX || siglen < 2)
-   return -EINVAL;
+   if (id >= INTEGRITY_KEYRING_MAX)
+   return ERR_PTR(-EINVAL);
 
if (!keyring[id]) {
keyring[id] =
@@ -61,18 +60,29 @@ int integrity_digsig_verify(const unsigned int id, const 
char *sig, int siglen,
int err = PTR_ERR(keyring[id]);
pr_err("no %s keyring: %d\n", keyring_name[id], err);
keyring[id] = NULL;
-   return err;
+   return ERR_PTR(err);
}
}
 
+   return keyring[id];
+}
+
+int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
+   const char *digest, int digestlen)
+{
+   struct key *keyring = integrity_keyring_from_id(id);
+
+   if (IS_ERR(keyring) || siglen < 2)
+   return PTR_ERR(keyring);
+
switch (sig[1]) {
case 1:
/* v1 API expect signature without xattr type */
-   return digsig_verify(keyring[id], sig + 1, siglen - 1,
-digest, digestlen);
+   return digsig_verify(keyring, sig + 1, siglen - 1, digest,
+digestlen);
case 2:
-   return asymmetric_verify(keyring[id], sig, siglen,
-digest, digestlen);
+   return asymmetric_verify(keyring, sig, siglen, digest,
+digestlen);
}
 
return -EOPNOTSUPP;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index dd7ac4bfc89a..657ce4558984 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -132,6 +132,7 @@ int integrity_kernel_read(struct file *file, loff_t offset,
 
 #ifdef CONFIG_INTEGRITY_SIGNATURE
 
+struct key *integrity_keyring_from_id(const unsigned int id);
 int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
const char *digest, int digestlen);
 

[PATCH v5 13/18] PKCS#7: Introduce pkcs7_get_message_sig and verify_pkcs7_message_sig

[Thiago Jung Bauermann]

IMA will need to access the digest used in the signature so that it can
verify files containing module-style appended signatures. For this purpose,
add function pkcs7_get_message_sig.

It will also need to verify an already parsed PKCS#7 message. For this
purpose, add function verify_pkcs7_message_signature which takes a struct
pkcs7_message for verification instead of the raw bytes that
verify_pkcs7_signature takes.

Signed-off-by: Thiago Jung Bauermann 
---
 certs/system_keyring.c| 60 +--
 crypto/asymmetric_keys/pkcs7_parser.c | 12 +++
 include/crypto/pkcs7.h|  2 ++
 include/linux/verification.h  | 10 ++
 4 files changed, 68 insertions(+), 16 deletions(-)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 6251d1b27f0c..6a8684959780 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -190,33 +190,26 @@ late_initcall(load_system_certificate_list);
 #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
 
 /**
- * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
+ * verify_pkcs7_message_sig - Verify a PKCS#7-based signature on system data.
  * @data: The data to be verified (NULL if expecting internal data).
  * @len: Size of @data.
- * @raw_pkcs7: The PKCS#7 message that is the signature.
- * @pkcs7_len: The size of @raw_pkcs7.
+ * @pkcs7: The PKCS#7 message that is the signature.
  * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
  * (void *)1UL for all trusted keys).
  * @usage: The use to which the key is being put.
  * @view_content: Callback to gain access to content.
  * @ctx: Context for callback.
  */
-int verify_pkcs7_signature(const void *data, size_t len,
-  const void *raw_pkcs7, size_t pkcs7_len,
-  struct key *trusted_keys,
-  enum key_being_used_for usage,
-  int (*view_content)(void *ctx,
-  const void *data, size_t len,
-  size_t asn1hdrlen),
-  void *ctx)
+int verify_pkcs7_message_sig(const void *data, size_t len,
+struct pkcs7_message *pkcs7,
+struct key *trusted_keys,
+enum key_being_used_for usage,
+int (*view_content)(void *ctx, const void *data,
+size_t len, size_t asn1hdrlen),
+void *ctx)
 {
-   struct pkcs7_message *pkcs7;
int ret;
 
-   pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
-   if (IS_ERR(pkcs7))
-   return PTR_ERR(pkcs7);
-
/* The data should be detached - so we need to supply it. */
if (data && pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
pr_err("PKCS#7 signature with non-detached data\n");
@@ -258,6 +251,41 @@ int verify_pkcs7_signature(const void *data, size_t len,
}
 
 error:
+   pr_devel("<==%s() = %d\n", __func__, ret);
+   return ret;
+}
+
+/**
+ * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
+ * @data: The data to be verified (NULL if expecting internal data).
+ * @len: Size of @data.
+ * @raw_pkcs7: The PKCS#7 message that is the signature.
+ * @pkcs7_len: The size of @raw_pkcs7.
+ * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
+ * (void *)1UL for all trusted keys).
+ * @usage: The use to which the key is being put.
+ * @view_content: Callback to gain access to content.
+ * @ctx: Context for callback.
+ */
+int verify_pkcs7_signature(const void *data, size_t len,
+  const void *raw_pkcs7, size_t pkcs7_len,
+  struct key *trusted_keys,
+  enum key_being_used_for usage,
+  int (*view_content)(void *ctx,
+  const void *data, size_t len,
+  size_t asn1hdrlen),
+  void *ctx)
+{
+   struct pkcs7_message *pkcs7;
+   int ret;
+
+   pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
+   if (IS_ERR(pkcs7))
+   return PTR_ERR(pkcs7);
+
+   ret = verify_pkcs7_message_sig(data, len, pkcs7, trusted_keys, usage,
+  view_content, ctx);
+
pkcs7_free_message(pkcs7);
pr_devel("<==%s() = %d\n", __func__, ret);
return ret;
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c 
b/crypto/asymmetric_keys/pkcs7_parser.c
index af4cd8649117..e41beda297a8 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -673,3 +673,15 @@ int pkcs7_note_signed_info(void *context, size_t 

[PATCH v5 11/18] ima: Export func_tokens

[Thiago Jung Bauermann]

ima_read_modsig will need it so that it can show an error message.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima.h|  2 ++
 security/integrity/ima/ima_policy.c | 12 ++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 4e61d3189b72..c9ba4362760b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -195,6 +195,8 @@ enum ima_hooks {
__ima_hooks(__ima_hook_enumify)
 };
 
+extern const char *const func_tokens[];
+
 /* LIM API function definitions */
 int ima_get_action(struct inode *inode, int mask,
   enum ima_hooks func, int *pcr);
diff --git a/security/integrity/ima/ima_policy.c 
b/security/integrity/ima/ima_policy.c
index efd8e1c60c10..7355d4ab7bf4 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -959,6 +959,12 @@ void ima_delete_rules(void)
}
 }
 
+#define __ima_hook_stringify(str)  (#str),
+
+const char *const func_tokens[] = {
+   __ima_hooks(__ima_hook_stringify)
+};
+
 #ifdef CONFIG_IMA_READ_POLICY
 enum {
mask_exec = 0, mask_write, mask_read, mask_append
@@ -971,12 +977,6 @@ static const char *const mask_tokens[] = {
"MAY_APPEND"
 };
 
-#define __ima_hook_stringify(str)  (#str),
-
-static const char *const func_tokens[] = {
-   __ima_hooks(__ima_hook_stringify)
-};
-
 void *ima_policy_start(struct seq_file *m, loff_t *pos)
 {
loff_t l = *pos;

[PATCH v5 10/18] ima: Store measurement after appraisal

[Thiago Jung Bauermann]

When module-style signatures appended at the end of files are supported for
IMA appraisal, the code will fallback to the xattr signature if the
appended one fails to verify.

The problem is that we don't know whether we need to fallback to the xattr
signature until the appraise step, and by then the measure step was already
completed and would need to be done again in case the template includes the
signature.

To avoid this problem, do the appraisal first so that the correct signature
is stored by the template in the measure step.

Suggested-by: Mimi Zohar 
Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
index 747a4fd9e2de..8e96450e27f5 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -242,12 +242,12 @@ static int process_measurement(struct file *file, char 
*buf, loff_t size,
if (!pathbuf)   /* ima_rdwr_violation possibly pre-fetched */
pathname = ima_d_path(>f_path, , filename);
 
-   if (action & IMA_MEASURE)
-   ima_store_measurement(iint, file, pathname,
- xattr_value, xattr_len, pcr);
if (rc == 0 && (action & IMA_APPRAISE_SUBMASK))
rc = ima_appraise_measurement(func, iint, file, pathname,
  xattr_value, xattr_len, opened);
+   if (action & IMA_MEASURE)
+   ima_store_measurement(iint, file, pathname,
+ xattr_value, xattr_len, pcr);
if (action & IMA_AUDIT)
ima_audit_measurement(iint, pathname);
 

[PATCH v5 01/18] ima: Remove redundant conditional operator

[Thiago Jung Bauermann]

A non-zero value is converted to 1 when assigned to a bool variable, so the
conditional operator in is_ima_appraise_enabled is redundant.

The value of a comparison operator is either 1 or 0 so the conditional
operator in ima_inode_setxattr is redundant as well.

Confirmed that the patch is correct by comparing the object file from
before and after the patch. They are identical.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima_appraise.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index 809ba70fbbbf..ec7dfa02c051 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -40,7 +40,7 @@ __setup("ima_appraise=", default_appraise_setup);
  */
 bool is_ima_appraise_enabled(void)
 {
-   return (ima_appraise & IMA_APPRAISE_ENFORCE) ? 1 : 0;
+   return ima_appraise & IMA_APPRAISE_ENFORCE;
 }
 
 /*
@@ -405,7 +405,7 @@ int ima_inode_setxattr(struct dentry *dentry, const char 
*xattr_name,
if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
return -EINVAL;
ima_reset_appraise_flags(d_backing_inode(dentry),
-(xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0);
+   xvalue->type == EVM_IMA_XATTR_DIGSIG);
result = 0;
}
return result;

[PATCH v2] staging: ccree: Fix bool comparison

[sunil . m]

From: Suniel Mahesh 

Comparision operator "equal to" not required on a variable
"foo" of type "bool". Bool has only two values, can be used
directly or with logical not.

This fixes the following coccinelle warning:
WARNING: Comparison of bool to 0/1

Signed-off-by: Suniel Mahesh 
---
Changes for v2:
- Changed the commit log to give a more accurate description
  of the changeset as suggested by Toby C.Harding
---
Note:
- Patch was built(ARCH=arm) on latest linux-next.
- No build issues reported, however it was not
  tested on real hardware.
- Please discard this changeset, if this is not
  helping the code look better.
---
 drivers/staging/ccree/ssi_request_mgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ccree/ssi_request_mgr.c 
b/drivers/staging/ccree/ssi_request_mgr.c
index 2e0df57..942afe2 100644
--- a/drivers/staging/ccree/ssi_request_mgr.c
+++ b/drivers/staging/ccree/ssi_request_mgr.c
@@ -272,7 +272,7 @@ int send_request(
unsigned int max_required_seq_len = (total_seq_len +
((ssi_req->ivgen_dma_addr_len == 0) ? 0 
:
SSI_IVPOOL_SEQ_LEN) +
-   ((is_dout == 0) ? 1 : 0));
+   (!is_dout ? 1 : 0));
 
 #if defined(CONFIG_PM_RUNTIME) || defined(CONFIG_PM_SLEEP)
rc = ssi_power_mgr_runtime_get(dev);
-- 
1.9.1

[PATCH v5 15/18] ima: Add modsig appraise_type option for module-style appended signatures

[Thiago Jung Bauermann]

This patch introduces the modsig keyword to the IMA policy syntax to
specify that a given hook should expect the file to have the IMA signature
appended to it. Here is how it can be used in a rule:

appraise func=KEXEC_KERNEL_CHECK appraise_type=modsig|imasig

With this rule, IMA will accept either an appended signature or a signature
stored in the extended attribute. In that case, it will first check whether
there is an appended signature, and if not it will read it from the
extended attribute.

For now, the rule above will behave exactly the same as if
appraise_type=imasig was specified because the actual modsig implementation
will be introduced in a separate patch.

Suggested-by: Mimi Zohar 
Signed-off-by: Thiago Jung Bauermann 
---
 Documentation/ABI/testing/ima_policy |  6 +-
 security/integrity/ima/Kconfig   | 10 +
 security/integrity/ima/Makefile  |  1 +
 security/integrity/ima/ima.h | 11 ++
 security/integrity/ima/ima_modsig.c  | 39 
 security/integrity/ima/ima_policy.c  | 12 +--
 security/integrity/integrity.h   |  3 ++-
 7 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy 
b/Documentation/ABI/testing/ima_policy
index e76432b9954d..ced1e1835a30 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -36,7 +36,7 @@ Description:
euid:= decimal value
fowner:= decimal value
lsm:are LSM specific
-   option: appraise_type:= [imasig]
+   option: appraise_type:= [imasig] [modsig|imasig]
pcr:= decimal value
 
default policy:
@@ -102,3 +102,7 @@ Description:
 
measure func=KEXEC_KERNEL_CHECK pcr=4
measure func=KEXEC_INITRAMFS_CHECK pcr=5
+
+   Example of appraise rule allowing modsig appended signatures:
+
+   appraise func=KEXEC_KERNEL_CHECK 
appraise_type=modsig|imasig
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 35ef69312811..40c6618d00e6 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -163,6 +163,16 @@ config IMA_APPRAISE_BOOTPARAM
  This option enables the different "ima_appraise=" modes
  (eg. fix, log) from the boot command line.
 
+config IMA_APPRAISE_MODSIG
+   bool "Support module-style signatures for appraisal"
+   depends on IMA_APPRAISE
+   default n
+   help
+  Adds support for signatures appended to files. The format of the
+  appended signature is the same used for signed kernel modules.
+  The modsig keyword can be used in the IMA policy to allow a hook
+  to accept such signatures.
+
 config IMA_TRUSTED_KEYRING
bool "Require all keys on the .ima keyring be signed (deprecated)"
depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 29f198bde02b..c72026acecc3 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -8,5 +8,6 @@ obj-$(CONFIG_IMA) += ima.o
 ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
 ima_policy.o ima_template.o ima_template_lib.o
 ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
+ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o
 ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
 obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c9ba4362760b..156ba218e0b6 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -255,6 +255,10 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data 
*xattr_value,
 int ima_read_xattr(struct dentry *dentry,
   struct evm_ima_xattr_data **xattr_value);
 
+#ifdef CONFIG_IMA_APPRAISE_MODSIG
+bool ima_hook_supports_modsig(enum ima_hooks func);
+#endif
+
 #else
 static inline int ima_appraise_measurement(enum ima_hooks func,
   struct integrity_iint_cache *iint,
@@ -298,6 +302,13 @@ static inline int ima_read_xattr(struct dentry *dentry,
 
 #endif /* CONFIG_IMA_APPRAISE */
 
+#ifndef CONFIG_IMA_APPRAISE_MODSIG
+static inline bool ima_hook_supports_modsig(enum ima_hooks func)
+{
+   return false;
+}
+#endif
+
 /* LSM based policy rules require audit */
 #ifdef CONFIG_IMA_LSM_RULES
 
diff --git a/security/integrity/ima/ima_modsig.c 
b/security/integrity/ima/ima_modsig.c
new file mode 100644
index ..452ce6048a7e
--- /dev/null
+++ b/security/integrity/ima/ima_modsig.c
@@ -0,0 +1,39 @@
+/*
+ * IMA support for appraising module-style appended signatures.
+ *
+ * Copyright (C) 2017  IBM Corporation
+ *
+ * Author:
+ * Thiago Jung Bauermann 

[PATCH v5 03/18] evm, ima: Remove superfluous parentheses

[Thiago Jung Bauermann]

This patch removes unnecessary parentheses from all EVM and IMA files
touched by this patch series.

The difference from the previous patch is that it cleans up the files as a
whole, not just the lines that were already going to be modified by other
patches. It is separate from the previous one so that it can be easily
dropped if the churn and conflict potential is deemed not worth it.

Confirmed that the patch is correct by comparing the object files from
before and after the patch. They are identical.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/evm/evm_crypto.c   |  2 +-
 security/integrity/evm/evm_main.c | 13 +-
 security/integrity/ima/ima_api.c  |  2 +-
 security/integrity/ima/ima_appraise.c |  2 +-
 security/integrity/ima/ima_main.c | 11 +
 security/integrity/ima/ima_policy.c   | 41 ---
 security/integrity/ima/ima_template.c | 25 +--
 security/integrity/ima/ima_template_lib.c |  6 ++---
 8 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/security/integrity/evm/evm_crypto.c 
b/security/integrity/evm/evm_crypto.c
index bcd64baf8788..9c2d88c80b9d 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -199,7 +199,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 
error = -ENODATA;
for (xattrname = evm_config_xattrnames; *xattrname != NULL; 
xattrname++) {
-   if ((req_xattr_name && req_xattr_value)
+   if (req_xattr_name && req_xattr_value
&& !strcmp(*xattrname, req_xattr_name)) {
error = 0;
crypto_shash_update(desc, (const u8 *)req_xattr_value,
diff --git a/security/integrity/evm/evm_main.c 
b/security/integrity/evm/evm_main.c
index 9826c02e2db8..37f062d38d5f 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -188,7 +188,7 @@ static enum integrity_status evm_verify_hmac(struct dentry 
*dentry,
}
 
if (rc)
-   evm_status = (rc == -ENODATA) ?
+   evm_status = rc == -ENODATA ?
INTEGRITY_NOXATTRS : INTEGRITY_FAIL;
 out:
if (iint)
@@ -205,8 +205,8 @@ static int evm_protected_xattr(const char *req_xattr_name)
 
namelen = strlen(req_xattr_name);
for (xattrname = evm_config_xattrnames; *xattrname != NULL; 
xattrname++) {
-   if ((strlen(*xattrname) == namelen)
-   && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) {
+   if (strlen(*xattrname) == namelen
+   && strncmp(req_xattr_name, *xattrname, namelen) == 0) {
found = 1;
break;
}
@@ -294,8 +294,8 @@ static int evm_protect_xattr(struct dentry *dentry, const 
char *xattr_name,
if (!posix_xattr_acl(xattr_name))
return 0;
evm_status = evm_verify_current_integrity(dentry);
-   if ((evm_status == INTEGRITY_PASS) ||
-   (evm_status == INTEGRITY_NOXATTRS))
+   if (evm_status == INTEGRITY_PASS ||
+   evm_status == INTEGRITY_NOXATTRS)
return 0;
goto out;
}
@@ -434,8 +434,7 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr 
*attr)
if (!(ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)))
return 0;
evm_status = evm_verify_current_integrity(dentry);
-   if ((evm_status == INTEGRITY_PASS) ||
-   (evm_status == INTEGRITY_NOXATTRS))
+   if (evm_status == INTEGRITY_PASS || evm_status == INTEGRITY_NOXATTRS)
return 0;
integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
dentry->d_name.name, "appraise_metadata",
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c7e8db0ea4c0..c6d346e9f708 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -54,7 +54,7 @@ int ima_alloc_init_template(struct ima_event_data *event_data,
u32 len;
 
result = field->field_init(event_data,
-  &((*entry)->template_data[i]));
+  &(*entry)->template_data[i]);
if (result != 0)
goto out;
 
diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index bce0b36778bd..58c6a60c7e83 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -401,7 +401,7 @@ int ima_inode_setxattr(struct dentry *dentry, const char 
*xattr_name,
result = ima_protect_xattr(dentry, xattr_name, xattr_value,
   xattr_value_len);
if (result == 1) {

[PATCH v5 17/18] ima: Implement support for module-style appended signatures

[Thiago Jung Bauermann]

This patch actually implements the appraise_type=modsig option, allowing
IMA to read and verify modsig signatures

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima.h  |  17 +++--
 security/integrity/ima/ima_appraise.c | 119 --
 security/integrity/ima/ima_main.c |   7 +-
 3 files changed, 128 insertions(+), 15 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index eb58af06566f..b082138461b3 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -157,7 +157,8 @@ void ima_init_template_list(void);
 
 static inline bool is_ima_sig(const struct evm_ima_xattr_data *xattr_value)
 {
-   return xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG;
+   return xattr_value && (xattr_value->type == EVM_IMA_XATTR_DIGSIG ||
+  xattr_value->type == IMA_MODSIG);
 }
 
 /*
@@ -243,9 +244,10 @@ int ima_policy_show(struct seq_file *m, void *v);
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise_measurement(enum ima_hooks func,
 struct integrity_iint_cache *iint,
-struct file *file, const unsigned char *filename,
-struct evm_ima_xattr_data *xattr_value,
-int xattr_len, int opened);
+struct file *file, const void *buf, loff_t size,
+const unsigned char *filename,
+struct evm_ima_xattr_data **xattr_value,
+int *xattr_len, int opened);
 int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
 void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
 enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
@@ -270,10 +272,11 @@ void ima_free_xattr_data(struct evm_ima_xattr_data *hdr);
 #else
 static inline int ima_appraise_measurement(enum ima_hooks func,
   struct integrity_iint_cache *iint,
-  struct file *file,
+  struct file *file, const void *buf,
+  loff_t size,
   const unsigned char *filename,
-  struct evm_ima_xattr_data 
*xattr_value,
-  int xattr_len, int opened)
+  struct evm_ima_xattr_data 
**xattr_value,
+  int *xattr_len, int opened)
 {
return INTEGRITY_UNKNOWN;
 }
diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index 58e147049e98..108690741c1a 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -190,6 +190,45 @@ int ima_read_xattr(struct dentry *dentry,
return ret;
 }
 
+static int appraise_modsig(struct integrity_iint_cache *iint,
+  struct evm_ima_xattr_data *xattr_value,
+  int xattr_len)
+{
+   enum hash_algo algo;
+   const void *digest;
+   void *buf;
+   int rc, len;
+   u8 dig_len;
+
+   rc = ima_modsig_verify(INTEGRITY_KEYRING_IMA, xattr_value);
+   if (rc)
+   return rc;
+
+   /*
+* The signature is good. Now let's put the sig hash
+* into the iint cache so that it gets stored in the
+* measurement list.
+*/
+
+   rc = ima_get_modsig_hash(xattr_value, , , _len);
+   if (rc)
+   return rc;
+
+   len = sizeof(iint->ima_hash) + dig_len;
+   buf = krealloc(iint->ima_hash, len, GFP_NOFS);
+   if (!buf)
+   return -ENOMEM;
+
+   iint->ima_hash = buf;
+   iint->flags |= IMA_DIGSIG;
+   iint->ima_hash->algo = algo;
+   iint->ima_hash->length = dig_len;
+
+   memcpy(iint->ima_hash->digest, digest, dig_len);
+
+   return 0;
+}
+
 /*
  * ima_appraise_measurement - appraise file measurement
  *
@@ -200,18 +239,28 @@ int ima_read_xattr(struct dentry *dentry,
  */
 int ima_appraise_measurement(enum ima_hooks func,
 struct integrity_iint_cache *iint,
-struct file *file, const unsigned char *filename,
-struct evm_ima_xattr_data *xattr_value,
-int xattr_len, int opened)
+struct file *file, const void *buf, loff_t size,
+const unsigned char *filename,
+struct evm_ima_xattr_data **xattr_value_,
+int *xattr_len_, int opened)
 {
static const char op[] = "appraise_data";
const char *cause = "unknown";
struct dentry *dentry = file_dentry(file);
struct inode *inode 

[PATCH v5 00/18] Appended signatures support for IMA appraisal

[Thiago Jung Bauermann]

Hello,

The main highlight in this version is that it fixes a bug where the modsig
wasn't being included in the measurement list if the appraised file was
already measured by another rule. The fix is in the last patch.

Another change is that the last patch in the v4 series ("ima: Support
module-style appended signatures for appraisal") has been broken up into
smaller patches. I may have overdone it...

Finally, I have added some patches removing superfluous parentheses from
expressions. IMO these patches make it easier (and more pleasant) to read
the code, and thus easier to understand it. Since I'm not sure how welcome
the changes are, I split them in 3 "levels" in increasing potential for
conflict with patches from other people (they can be squashed together when
applied):

1. patch 2 contains the bare minimum, changing only lines that are also
   touched by other patches in the series;

2. patch 3 cleans up all the files that are touched by this patch series;

3. patch 4 cleans up all other EVM and IMA files that weren't already fixed
   by the previous patches.

If unwanted, patches 3 and 4 can be simply skipped without affecting the
rest of the patches. I have already rebased them from v4.13-rc2 to
v4.14-rc3 and now to linux-integrity/next with very few easy to resolve
conflicts, so I think they are worth keeping.

These patches apply on top of today's linux-integrity/next.

Original cover letter:

On the OpenPOWER platform, secure boot and trusted boot are being
implemented using IMA for taking measurements and verifying signatures.
Since the kernel image on Power servers is an ELF binary, kernels are
signed using the scripts/sign-file tool and thus use the same signature
format as signed kernel modules.

This patch series adds support in IMA for verifying those signatures.
It adds flexibility to OpenPOWER secure boot, because it allows it to boot
kernels with the signature appended to them as well as kernels where the
signature is stored in the IMA extended attribute.

Since modsig is only supported on some specific hooks which don't get
called often (cf. ima_hook_supports_modsig), it's possible to always check
for the presence of an appended modsig before looking for the xattr sig. In
that case, the policy doesn't need to be changed to support the modsig
keyword. Is that preferable than requiring the policy to explicitly allow a
modsig like this code does?

I tested these patches with EVM and I believe they don't break it and
things work as expected, but I'm not really familiar with EVM and its use
cases so this should be taken with a grain of salt.

I also verified that the code correctly recalculates the file hash if the
modsig verification fails and the file also has an xattr signature which
uses a different hash algorithm.

Changes since v4:
- Patch "ima: Remove redundant conditional operator"
  - New patch.

- Patch "ima: Remove some superfluous parentheses"
  - New patch.

- Patch "evm, ima: Remove superfluous parentheses"
  - New patch.

- Patch "evm, ima: Remove more superfluous parentheses"
  - New patch.

- Patch "ima: Simplify ima_eventsig_init"
  - New patch.

- Patch "ima: Improvements in ima_appraise_measurement"
  - New patch.

- Patch "ima: Don't pass xattr value to EVM xattr verification."
  - New patch.

- Patch "ima: Export func_tokens"
  - Split from patch "ima: Support module-style appended signatures for
appraisal".

- Patch "ima: Add modsig appraise_type option for module-style appended
 signatures"
  - Split from patch "ima: Support module-style appended signatures for
appraisal".
  - Mention modsig option in Documentation/ABI/testing/ima_policy
(suggested by Mimi Zohar).

- Patch "ima: Add functions to read and verify a modsig signature"
  - Split from patch "ima: Support module-style appended signatures for
appraisal".

- Patch "ima: Implement support for module-style appended signatures"
  - Split from patch "ima: Support module-style appended signatures for
appraisal".
  - In ima_appraise_measurement, change the logic of dealing with xattr
errors in case the modsig verification fails. With this,
process_xattr_error isn't needed anymore.

- Patch "ima: Write modsig to the measurement list"
  - Split from patch "ima: Support module-style appended signatures for
appraisal".
  - Added ima_current_template_has_sig function.
  - Removed hdr parameter from ima_modsig_serialize_data.
  - In ima_store_measurement, continue processing even if the given PCR
is already measured if it's for a modsig.
  - In process_measurement, add exception to store measurement even if
IMA_MEASURE is not set when appraising a modsig (suggested by
Mimi Zohar).
  - Call is_ima_sig in ima_eventsig_init.

Changes since v3:
- Patch "integrity: Introduce struct evm_hmac_xattr"
  - Renamed new struct to evm_xattr.
  - Define struct evm_xattr using struct evm_ima_xattr_data, and moved it
from evm.h to integrity.h (suggested by Mimi Zohar).
- Patch 

[PATCH v5 18/18] ima: Write modsig to the measurement list

[Thiago Jung Bauermann]

Add modsig support for templates which require the contents of the file
signature to be included in the measurement list.

Suggested-by: Mimi Zohar 
Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima.h  |  8 
 security/integrity/ima/ima_api.c  |  8 +++-
 security/integrity/ima/ima_main.c | 30 +-
 security/integrity/ima/ima_modsig.c   | 20 +++-
 security/integrity/ima/ima_template.c | 12 
 security/integrity/ima/ima_template_lib.c | 17 +++--
 6 files changed, 90 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index b082138461b3..68f471666151 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -148,6 +148,7 @@ int ima_init_crypto(void);
 void ima_putc(struct seq_file *m, void *data, int datalen);
 void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
 struct ima_template_desc *ima_template_desc_current(void);
+bool ima_current_template_has_sig(void);
 int ima_restore_measurement_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
 int ima_measurements_show(struct seq_file *m, void *v);
@@ -264,6 +265,7 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, 
loff_t buf_len,
int *xattr_len);
 int ima_get_modsig_hash(struct evm_ima_xattr_data *hdr, enum hash_algo *algo,
void const **hash, u8 *len);
+int ima_modsig_serialize_data(struct evm_ima_xattr_data **data, int *data_len);
 int ima_modsig_verify(const unsigned int keyring_id,
  struct evm_ima_xattr_data *hdr);
 void ima_free_xattr_data(struct evm_ima_xattr_data *hdr);
@@ -334,6 +336,12 @@ static inline int ima_get_modsig_hash(struct 
evm_ima_xattr_data *hdr,
return -ENOTSUPP;
 }
 
+static inline int ima_modsig_serialize_data(struct evm_ima_xattr_data **data,
+   int *data_len)
+{
+   return -ENOTSUPP;
+}
+
 static inline int ima_modsig_verify(const unsigned int keyring_id,
struct evm_ima_xattr_data *hdr)
 {
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c6d346e9f708..59a5b044b48b 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -284,7 +284,13 @@ void ima_store_measurement(struct integrity_iint_cache 
*iint,
xattr_len, NULL};
int violation = 0;
 
-   if (iint->measured_pcrs & (0x1 << pcr))
+   /*
+* We still need to store the measurement in the case of MODSIG because
+* we only have its contents to put in the list at the time of
+* appraisal. See comment in process_measurement for more details.
+*/
+   if ((iint->measured_pcrs & (0x1 << pcr)) &&
+   (!xattr_value || xattr_value->type != IMA_MODSIG))
return;
 
result = ima_alloc_init_template(_data, );
diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
index 6a2d960fbd92..0d3390de7432 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -246,7 +246,35 @@ static int process_measurement(struct file *file, char 
*buf, loff_t size,
rc = ima_appraise_measurement(func, iint, file, buf, size,
  pathname, _value,
  _len, opened);
-   if (action & IMA_MEASURE)
+
+   /*
+* MODSIG has one corner case we need to deal with here:
+*
+* Suppose the policy has one measure rule for one hook and an appraise
+* rule for a different hook. Suppose also that the template requires
+* the signature to be stored in the measurement list.
+*
+* If a file containing a MODSIG is measured by the first hook before
+* being appraised by the second one, the corresponding entry in the
+* measurement list will not contain the MODSIG because we only fetch it
+* for IMA_APPRAISAL. We don't fetch it earlier because if the file has
+* both a DIGSIG and a MODSIG it's not possible to know which one will
+* be valid without actually doing the appraisal.
+*
+* Therefore, after appraisal of a MODSIG signature we need to store the
+* measurement again if the current template requires storing the
+* signature.
+*
+* With the opposite ordering (the appraise rule triggering before the
+* measurement rule) there is the same problem but it's not possible to
+* do anything about it because at the time we are appraising the
+* signature it's impossible to know whether a measurement will ever
+* need to be stored for 

[PATCH v5 07/18] integrity: Introduce struct evm_xattr

[Thiago Jung Bauermann]

Even though struct evm_ima_xattr_data includes a fixed-size array to hold a
SHA1 digest, most of the code ignores the array and uses the struct to mean
"type indicator followed by data of unspecified size" and tracks the real
size of what the struct represents in a separate length variable.

The only exception to that is the EVM code, which correctly uses the
definition of struct evm_ima_xattr_data.

This patch makes this explicit in the code by removing the length
specification from the array in struct evm_ima_xattr_data. It also changes
the name of the element from digest to data, since in most places the array
doesn't hold a digest.

A separate struct evm_xattr is introduced, with the original definition of
evm_ima_xattr_data to be used in the places that actually expect that
definition.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/evm/evm_crypto.c   |  4 ++--
 security/integrity/evm/evm_main.c | 10 +-
 security/integrity/ima/ima_appraise.c |  7 ---
 security/integrity/integrity.h|  5 +
 4 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/security/integrity/evm/evm_crypto.c 
b/security/integrity/evm/evm_crypto.c
index 9c2d88c80b9d..5fddc28e8a0e 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -252,13 +252,13 @@ int evm_update_evmxattr(struct dentry *dentry, const char 
*xattr_name,
const char *xattr_value, size_t xattr_value_len)
 {
struct inode *inode = d_backing_inode(dentry);
-   struct evm_ima_xattr_data xattr_data;
+   struct evm_xattr xattr_data;
int rc = 0;
 
rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
   xattr_value_len, xattr_data.digest);
if (rc == 0) {
-   xattr_data.type = EVM_XATTR_HMAC;
+   xattr_data.data.type = EVM_XATTR_HMAC;
rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM,
   _data,
   sizeof(xattr_data), 0);
diff --git a/security/integrity/evm/evm_main.c 
b/security/integrity/evm/evm_main.c
index 37f062d38d5f..7f31f65b8492 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -119,7 +119,7 @@ static enum integrity_status evm_verify_hmac(struct dentry 
*dentry,
 struct integrity_iint_cache *iint)
 {
struct evm_ima_xattr_data *xattr_data = NULL;
-   struct evm_ima_xattr_data calc;
+   struct evm_xattr calc;
enum integrity_status evm_status = INTEGRITY_PASS;
int rc, xattr_len;
 
@@ -150,7 +150,7 @@ static enum integrity_status evm_verify_hmac(struct dentry 
*dentry,
/* check value type */
switch (xattr_data->type) {
case EVM_XATTR_HMAC:
-   if (xattr_len != sizeof(struct evm_ima_xattr_data)) {
+   if (xattr_len != sizeof(struct evm_xattr)) {
evm_status = INTEGRITY_FAIL;
goto out;
}
@@ -158,7 +158,7 @@ static enum integrity_status evm_verify_hmac(struct dentry 
*dentry,
   xattr_value_len, calc.digest);
if (rc)
break;
-   rc = crypto_memneq(xattr_data->digest, calc.digest,
+   rc = crypto_memneq(xattr_data->data, calc.digest,
sizeof(calc.digest));
if (rc)
rc = -EINVAL;
@@ -469,7 +469,7 @@ int evm_inode_init_security(struct inode *inode,
 const struct xattr *lsm_xattr,
 struct xattr *evm_xattr)
 {
-   struct evm_ima_xattr_data *xattr_data;
+   struct evm_xattr *xattr_data;
int rc;
 
if (!evm_initialized || !evm_protected_xattr(lsm_xattr->name))
@@ -479,7 +479,7 @@ int evm_inode_init_security(struct inode *inode,
if (!xattr_data)
return -ENOMEM;
 
-   xattr_data->type = EVM_XATTR_HMAC;
+   xattr_data->data.type = EVM_XATTR_HMAC;
rc = evm_init_hmac(inode, lsm_xattr, xattr_data->digest);
if (rc < 0)
goto out;
diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index 2c069f47eeec..091977c8ec40 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -156,7 +156,8 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data 
*xattr_value,
return sig->hash_algo;
break;
case IMA_XATTR_DIGEST_NG:
-   ret = xattr_value->digest[0];
+   /* first byte contains algorithm id */
+   ret = xattr_value->data[0];
if (ret < HASH_ALGO__LAST)
return ret;
break;
@@ -164,7 +165,7 @@ enum hash_algo ima_get_hash_algo(struct 

[PATCH v5 12/18] MODSIGN: Export module signature definitions

[Thiago Jung Bauermann]

IMA will use the module_signature format for append signatures, so export
the relevant definitions and factor out the code which verifies that the
appended signature trailer is valid.

Also, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it
and be able to use validate_module_signature without having to depend on
CONFIG_MODULE_SIG.

Signed-off-by: Thiago Jung Bauermann 
---
 include/linux/module.h   |  3 --
 include/linux/module_signature.h | 47 +
 init/Kconfig |  6 +++-
 kernel/Makefile  |  2 +-
 kernel/module.c  |  1 +
 kernel/module_signing.c  | 74 +---
 6 files changed, 85 insertions(+), 48 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index fe5aa3736707..108874af9838 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -23,9 +23,6 @@
 #include 
 #include 
 
-/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
-#define MODULE_SIG_STRING "~Module signature appended~\n"
-
 /* Not Yet Implemented */
 #define MODULE_SUPPORTED_DEVICE(name)
 
diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
new file mode 100644
index ..e80728e5b86c
--- /dev/null
+++ b/include/linux/module_signature.h
@@ -0,0 +1,47 @@
+/* Module signature handling.
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowe...@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#ifndef _LINUX_MODULE_SIGNATURE_H
+#define _LINUX_MODULE_SIGNATURE_H
+
+/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
+#define MODULE_SIG_STRING "~Module signature appended~\n"
+
+enum pkey_id_type {
+   PKEY_ID_PGP,/* OpenPGP generated key ID */
+   PKEY_ID_X509,   /* X.509 arbitrary subjectKeyIdentifier */
+   PKEY_ID_PKCS7,  /* Signature in PKCS#7 message */
+};
+
+/*
+ * Module signature information block.
+ *
+ * The constituents of the signature section are, in order:
+ *
+ * - Signer's name
+ * - Key identifier
+ * - Signature data
+ * - Information block
+ */
+struct module_signature {
+   u8  algo;   /* Public-key crypto algorithm [0] */
+   u8  hash;   /* Digest algorithm [0] */
+   u8  id_type;/* Key identifier type [PKEY_ID_PKCS7] */
+   u8  signer_len; /* Length of signer's name [0] */
+   u8  key_id_len; /* Length of key identifier [0] */
+   u8  __pad[3];
+   __be32  sig_len;/* Length of signature data */
+};
+
+int validate_module_sig(const struct module_signature *ms, size_t file_len);
+int mod_verify_sig(const void *mod, unsigned long *_modlen);
+
+#endif /* _LINUX_MODULE_SIGNATURE_H */
diff --git a/init/Kconfig b/init/Kconfig
index 78cb2461012e..230e9f3aedaf 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1748,7 +1748,7 @@ config MODULE_SRCVERSION_ALL
 config MODULE_SIG
bool "Module signature verification"
depends on MODULES
-   select SYSTEM_DATA_VERIFICATION
+   select MODULE_SIG_FORMAT
help
  Check modules for valid signatures upon load: the signature
  is simply appended to the module. For more information see
@@ -1763,6 +1763,10 @@ config MODULE_SIG
  debuginfo strip done by some packagers (such as rpmbuild) and
  inclusion into an initramfs that wants the module size reduced.
 
+config MODULE_SIG_FORMAT
+   def_bool n
+   select SYSTEM_DATA_VERIFICATION
+
 config MODULE_SIG_FORCE
bool "Require modules to be validly signed"
depends on MODULE_SIG
diff --git a/kernel/Makefile b/kernel/Makefile
index ed470aac53da..20fd6d76232e 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -57,7 +57,7 @@ obj-y += up.o
 endif
 obj-$(CONFIG_UID16) += uid16.o
 obj-$(CONFIG_MODULES) += module.o
-obj-$(CONFIG_MODULE_SIG) += module_signing.o
+obj-$(CONFIG_MODULE_SIG_FORMAT) += module_signing.o
 obj-$(CONFIG_KALLSYMS) += kallsyms.o
 obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
 obj-$(CONFIG_CRASH_CORE) += crash_core.o
diff --git a/kernel/module.c b/kernel/module.c
index de66ec825992..a259e9df570d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 937c844bee4a..204c60d4cc9f 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -11,36 +11,38 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include "module-internal.h"
 
-enum pkey_id_type {
-   PKEY_ID_PGP,/* OpenPGP 

[PATCH v5 16/18] ima: Add functions to read and verify a modsig signature

[Thiago Jung Bauermann]

This is the code needed by IMA-appraise to work with modsig signatures.
It will be used by the next patch.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/Kconfig  |   3 +
 security/integrity/ima/ima.h|  34 +++
 security/integrity/ima/ima_modsig.c | 119 
 security/integrity/integrity.h  |   1 +
 4 files changed, 157 insertions(+)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 40c6618d00e6..55f734a6124b 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -166,6 +166,9 @@ config IMA_APPRAISE_BOOTPARAM
 config IMA_APPRAISE_MODSIG
bool "Support module-style signatures for appraisal"
depends on IMA_APPRAISE
+   depends on INTEGRITY_ASYMMETRIC_KEYS
+   select PKCS7_MESSAGE_PARSER
+   select MODULE_SIG_FORMAT
default n
help
   Adds support for signatures appended to files. The format of the
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 156ba218e0b6..eb58af06566f 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -257,6 +257,14 @@ int ima_read_xattr(struct dentry *dentry,
 
 #ifdef CONFIG_IMA_APPRAISE_MODSIG
 bool ima_hook_supports_modsig(enum ima_hooks func);
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+   struct evm_ima_xattr_data **xattr_value,
+   int *xattr_len);
+int ima_get_modsig_hash(struct evm_ima_xattr_data *hdr, enum hash_algo *algo,
+   void const **hash, u8 *len);
+int ima_modsig_verify(const unsigned int keyring_id,
+ struct evm_ima_xattr_data *hdr);
+void ima_free_xattr_data(struct evm_ima_xattr_data *hdr);
 #endif
 
 #else
@@ -307,6 +315,32 @@ static inline bool ima_hook_supports_modsig(enum ima_hooks 
func)
 {
return false;
 }
+
+static inline int ima_read_modsig(enum ima_hooks func, const void *buf,
+ loff_t buf_len,
+ struct evm_ima_xattr_data **xattr_value,
+ int *xattr_len)
+{
+   return -ENOTSUPP;
+}
+
+static inline int ima_get_modsig_hash(struct evm_ima_xattr_data *hdr,
+ enum hash_algo *algo, void const **hash,
+ u8 *len)
+{
+   return -ENOTSUPP;
+}
+
+static inline int ima_modsig_verify(const unsigned int keyring_id,
+   struct evm_ima_xattr_data *hdr)
+{
+   return -ENOTSUPP;
+}
+
+static inline void ima_free_xattr_data(struct evm_ima_xattr_data *hdr)
+{
+   kfree(hdr);
+}
 #endif
 
 /* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_modsig.c 
b/security/integrity/ima/ima_modsig.c
index 452ce6048a7e..2786aa97060e 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -16,8 +16,19 @@
  * GNU General Public License for more details.
  */
 
+#include 
+#include 
+#include 
+
 #include "ima.h"
 
+struct modsig_hdr {
+   uint8_t type;   /* Should be IMA_MODSIG. */
+   const void *data;   /* Pointer to data covered by pkcs7_msg. */
+   size_t data_len;
+   struct pkcs7_message *pkcs7_msg;
+};
+
 /**
  * ima_hook_supports_modsig - can the policy allow modsig for this hook?
  *
@@ -37,3 +48,111 @@ bool ima_hook_supports_modsig(enum ima_hooks func)
return false;
}
 }
+
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+   struct evm_ima_xattr_data **xattr_value,
+   int *xattr_len)
+{
+   const size_t marker_len = sizeof(MODULE_SIG_STRING) - 1;
+   const struct module_signature *sig;
+   struct modsig_hdr *hdr;
+   size_t sig_len;
+   const void *p;
+   int rc;
+
+   /*
+* Not supposed to happen. Hooks that support modsig are
+* whitelisted when parsing the policy using
+* ima_hooks_supports_modsig.
+*/
+   if (!buf || !buf_len) {
+   WARN_ONCE(true, "%s doesn't support modsig\n",
+ func_tokens[func]);
+   return -ENOENT;
+   } else if (buf_len <= marker_len + sizeof(*sig))
+   return -ENOENT;
+
+   p = buf + buf_len - marker_len;
+   if (memcmp(p, MODULE_SIG_STRING, marker_len))
+   return -ENOENT;
+
+   buf_len -= marker_len;
+   sig = (const struct module_signature *) (p - sizeof(*sig));
+
+   rc = validate_module_sig(sig, buf_len);
+   if (rc)
+   return rc;
+
+   sig_len = be32_to_cpu(sig->sig_len);
+   buf_len -= sig_len + sizeof(*sig);
+
+   hdr = kmalloc(sizeof(*hdr), GFP_KERNEL);
+   if (!hdr)
+   return -ENOMEM;
+
+   hdr->pkcs7_msg = pkcs7_parse_message(buf + buf_len, sig_len);
+   if 

[PATCH v5 09/18] ima: Don't pass xattr value to EVM xattr verification.

[Thiago Jung Bauermann]

The patch implementing modsig support will retry verifying the xattr
signature if the modsig verification fails, and if we have already passed
the modsig as the xattr_value we'll have problems if we pass the xattr sig
in the second call to evm_verifyxattr.

Since this is an optimization and not actually required, just don't do it.

Suggested-by: Mimi Zohar 
Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima_appraise.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index 091977c8ec40..58e147049e98 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -229,7 +229,7 @@ int ima_appraise_measurement(enum ima_hooks func,
goto out;
}
 
-   status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
+   status = evm_verifyxattr(dentry, XATTR_NAME_IMA, NULL, 0, iint);
switch (status) {
case INTEGRITY_PASS:
case INTEGRITY_UNKNOWN:

[PATCH v2] staging: ccree: fix boolreturn.cocci warning

[sunil . m]

From: Suniel Mahesh 

Return "false" instead of 0.

This fixes the following coccinelle warning:
WARNING: return of 0/1 in function 'ssi_is_hw_key' with return type bool.

Signed-off-by: Suniel Mahesh 
---
Changes for v2:
- Changed the commit log to give a more accurate description
  of the changeset as suggested by Toby C.Harding.
---
Note:
- Patch was built(ARCH=arm) on latest linux-next.
- No build issues reported, however it was not
  tested on real hardware.
- Please discard this changeset, if this is not
  helping the code look better.
---
 drivers/staging/ccree/ssi_cipher.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ccree/ssi_cipher.h 
b/drivers/staging/ccree/ssi_cipher.h
index c9a83df..f499962 100644
--- a/drivers/staging/ccree/ssi_cipher.h
+++ b/drivers/staging/ccree/ssi_cipher.h
@@ -75,7 +75,7 @@ struct arm_hw_key_info {
 
 static inline bool ssi_is_hw_key(struct crypto_tfm *tfm)
 {
-   return 0;
+   return false;
 }
 
 #endif /* CRYPTO_TFM_REQ_HW_KEY */
-- 
1.9.1

Re: [PATCH v2] staging: ccree: Fix bool comparison

[Tobin C. Harding]

On Wed, Oct 18, 2017 at 07:40:14AM +0530, suni...@techveda.org wrote:
> From: Suniel Mahesh 
> 
> Comparision operator "equal to" not required on a variable
> "foo" of type "bool". Bool has only two values, can be used
> directly or with logical not.
> 
> This fixes the following coccinelle warning:
> WARNING: Comparison of bool to 0/1
> 
> Signed-off-by: Suniel Mahesh 

Nice.

Re: [PATCH v2] staging: ccree: fix boolreturn.cocci warning

[Tobin C. Harding]

On Wed, Oct 18, 2017 at 07:42:53AM +0530, suni...@techveda.org wrote:
> From: Suniel Mahesh 
>
> Return "false" instead of 0.
> 
> This fixes the following coccinelle warning:
> WARNING: return of 0/1 in function 'ssi_is_hw_key' with return type bool.

So close! The order of problem description and fix is inverted. What about

```
coccinelle emits: WARNING: return of 0/1 in function 'ssi_is_hw_key' with
return type bool.

Return "false" instead of 0.
```


Good luck,
Tobin.

Re: [PATCH v9 00/20] simplify crypto wait for async op

[Gilad Ben-Yossef]

On Tue, Oct 17, 2017 at 5:06 PM, Russell King - ARM Linux
 wrote:
> On Sun, Oct 15, 2017 at 10:19:45AM +0100, Gilad Ben-Yossef wrote:
>> Many users of kernel async. crypto services have a pattern of
>> starting an async. crypto op and than using a completion
>> to wait for it to end.
>>
>> This patch set simplifies this common use case in two ways:
>>
>> First, by separating the return codes of the case where a
>> request is queued to a backlog due to the provider being
>> busy (-EBUSY) from the case the request has failed due
>> to the provider being busy and backlogging is not enabled
>> (-EAGAIN).
>>
>> Next, this change is than built on to create a generic API
>> to wait for a async. crypto operation to complete.
>>
>> The end result is a smaller code base and an API that is
>> easier to use and more difficult to get wrong.
>>
>> The patch set was boot tested on x86_64 and arm64 which
>> at the very least tests the crypto users via testmgr and
>> tcrypt but I do note that I do not have access to some
>> of the HW whose drivers are modified nor do I claim I was
>> able to test all of the corner cases.
>>
>> The patch set is based upon linux-next release tagged
>> next-20171013.
>
> Has there been any performance impact analysis of these changes?  I
> ended up with patches for one of the crypto drivers which converted
> its interrupt handling to threaded interrupts being reverted because
> it caused a performance degredation.
>
> Moving code to latest APIs to simplify it is not always beneficial.

I agree with the sentiment but I believe this one is justified.

This patch set basically does 3 things:

1.  Replace one immediate value (-EBUSY) by another (-EAGAIN). Mostly it's just
s/EBUSY/EAGAIN/g. In very few places this resulted very trivial code
changes. I don't
foresee this having any effect on performance.

2. Removal of some conditions and/or conditional jumps that were used to discern
between two different cases which are now now easily tested for by the
different return
value. If at all, this will be an increase in performance, although I
don't expect it to be
noticeable.

3. Replacing a whole bunch of open coded code and data structures
which were pretty much
cut and pasted from the Documentation and therefore identical, with a
single copy thereof.

Every place that I found that deviated slightly from the identical
pattern, it turned out to be
a bug of some sorts and patches for those were sent and accepted already.

So, we might be losing a few inline optimization opportunities but
we're gaining better
cache utilization. Again, I don't expect any of this to have a
noticeable effect to either
direction.

I did run the changed code as best I could and did not notice any
performance changes and
none of the testers and maintainers that ACKed mentioned any.

Having said that, it's a big change that touches many places,
sub-systems and drivers. I do
not claim to have thoroughly tested for performance all the changes in
person. In some cases,
I don't even have access to the specialized hardware. I did get a
reasonable amount of review
and testers I believe - would always love to see more :-)

Many thanks,
Gilad




-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru