Re: export pcie_flr and remove copies of it in drivers V2

[Leon Romanovsky]

On Tue, Apr 18, 2017 at 01:36:12PM -0500, Bjorn Helgaas wrote:
> On Fri, Apr 14, 2017 at 09:11:24PM +0200, Christoph Hellwig wrote:
> > Hi all,
> >
> > this exports the PCI layer pcie_flr helper, and removes various opencoded
> > copies of it.
> >
> > Changes since V1:
> >  - rebase on top of the pci/virtualization branch
> >  - fixed the probe case in __pci_dev_reset
> >  - added ACKs from Bjorn
>
> Applied the first three patches:
>
>   bc13871ef35a PCI: Export pcie_flr()
>   e641c375d414 PCI: Call pcie_flr() from reset_intel_82599_sfp_virtfn()
>   40e0901ea4bf PCI: Call pcie_flr() from reset_chelsio_generic_dev()
>

Bjorn,

How do you suggest to proceed with other patches? They should be applied
to your tree either, because they depend on "bc13871ef35a PCI: Export
pcie_flr()".

Thanks


> to pci/virtualization for v4.12, thanks!


signature.asc
Description: PGP signature

padata & workqueue list corruption

[Samuel Holland]

Hello Steffen & Workqueue People,

As Jason wrote about here a few weeks ago, we've been having issues
with padata. After spending considerable time working to rule out
the possibility that our code was doing something wrong, I've begun
to debug padata and the workqueue subsystems. I've gotten some
potentially useful backtraces and was hoping somebody here might
read them and have an "ah ha!" moment.

We've been using the padata library for some high-throughput
encryption/decryption workloads, and on a relatively weak CPU (Celeron
J1800), we have run into list corruption that results in all workqueues
getting stalled, and occasional panics. I can reproduce this fairly
easily, albeit after several hours of load.

Representative backtraces follow (the warnings come in sets). I have
kernel .configs and extended netconsole output from several occurrences
available upon request.

WARNING: CPU: 1 PID: 0 at lib/list_debug.c:33 __list_add+0x89/0xb0
list_add corruption. prev->next should be next (99f135016a90), but
was d34affc03b10. (prev=d34affc03b10).
CPU: 1 PID: 0 Comm: swapper/1 Tainted: G   O4.9.20+ #1
Call Trace:
 
 dump_stack+0x67/0x92
 __warn+0xc6/0xe0
 warn_slowpath_fmt+0x5a/0x80
 __list_add+0x89/0xb0
 insert_work+0x3c/0xc0
 __queue_work+0x18a/0x600
 queue_work_on+0x33/0x70
 padata_do_parallel+0x14f/0x240
 ? padata_index_to_cpu+0x50/0x50
 ? packet_receive+0x140/0x140 [wireguard]
 packet_consume_data+0x1b9/0x2b0 [wireguard]
 ? packet_create_data+0x6b0/0x6b0 [wireguard]
 ? get_partial_node.isra.72+0x47/0x250
 ? skb_prepare_header+0xd5/0x3f0 [wireguard]
 ? packet_receive+0x140/0x140 [wireguard]
 packet_receive+0x79/0x140 [wireguard]
 ? __udp4_lib_lookup+0x147/0x2d0
 receive+0x1a/0x30 [wireguard]
 udp_queue_rcv_skb+0x34a/0x5b0
 __udp4_lib_rcv+0x468/0xb40
 ? ip_local_deliver_finish+0x21/0x370
 udp_rcv+0x15/0x20
 ip_local_deliver_finish+0xb7/0x370
 ? ip_local_deliver_finish+0x21/0x370
 ip_local_deliver+0x1e6/0x230
 ? ip_local_deliver+0x62/0x230
 ? ip_rcv_finish+0x670/0x670
 ip_rcv_finish+0x1ae/0x670
 ip_rcv+0x366/0x4d0
 ? ip_rcv+0x26a/0x4d0
 ? inet_del_offload+0x40/0x40
 __netif_receive_skb_core+0xae1/0xc80
 ? inet_del_offload+0x40/0x40
 ? netif_receive_skb_internal+0x29/0x200
 __netif_receive_skb+0x18/0x60
 netif_receive_skb_internal+0x7b/0x200
 ? netif_receive_skb_internal+0x29/0x200
 netif_receive_skb+0xcd/0x130
 br_pass_frame_up+0x2b1/0x2c0
 ? br_pass_frame_up+0xad/0x2c0
 ? br_allowed_ingress+0x38a/0x5d0
 ? br_allowed_ingress+0x1f5/0x5d0
 br_handle_frame_finish+0x28f/0x5a0
 ? br_handle_frame+0x1c1/0x5e0
 br_handle_frame+0x2c5/0x5e0
 ? br_handle_frame+0x1c1/0x5e0
 ? vlan_do_receive+0x37/0x380
 ? br_handle_frame_finish+0x5a0/0x5a0
 __netif_receive_skb_core+0x1e6/0xc80
 ? netif_receive_skb_internal+0x29/0x200
 __netif_receive_skb+0x18/0x60
 netif_receive_skb_internal+0x7b/0x200
 ? netif_receive_skb_internal+0x29/0x200
 napi_gro_receive+0x148/0x200
 igb_poll+0x67b/0xdb0
 ? net_rx_action+0xe5/0x450
 net_rx_action+0x224/0x450
 __do_softirq+0x1a9/0x4a0
 irq_exit+0xbe/0xd0
 do_IRQ+0x65/0x110
 common_interrupt+0x89/0x89
 

This add looks to be racing with a deletion of the last item in the
list, because in the actual list prev->next = prev.

WARNING: CPU: 1 PID: 0 at lib/list_debug.c:36 __list_add+0xac/0xb0
list_add double add: new=d34affc03b10, prev=d34affc03b10,
next=99f135016a90.
CPU: 1 PID: 0 Comm: swapper/1 Tainted: GW  O4.9.20+ #1
Call Trace:
 
 dump_stack+0x67/0x92
 __warn+0xc6/0xe0
 warn_slowpath_fmt+0x5a/0x80
 __list_add+0xac/0xb0
 insert_work+0x3c/0xc0
 __queue_work+0x18a/0x600
 queue_work_on+0x33/0x70
 padata_do_parallel+0x14f/0x240
 ? padata_index_to_cpu+0x50/0x50
 ? packet_receive+0x140/0x140 [wireguard]
 packet_consume_data+0x1b9/0x2b0 [wireguard]
 ? packet_create_data+0x6b0/0x6b0 [wireguard]
 ? get_partial_node.isra.72+0x47/0x250
 ? skb_prepare_header+0xd5/0x3f0 [wireguard]
 ? packet_receive+0x140/0x140 [wireguard]
 packet_receive+0x79/0x140 [wireguard]
 ? __udp4_lib_lookup+0x147/0x2d0
 receive+0x1a/0x30 [wireguard]
 udp_queue_rcv_skb+0x34a/0x5b0
 __udp4_lib_rcv+0x468/0xb40
 ? ip_local_deliver_finish+0x21/0x370
 udp_rcv+0x15/0x20
 ip_local_deliver_finish+0xb7/0x370
 ? ip_local_deliver_finish+0x21/0x370
 ip_local_deliver+0x1e6/0x230
 ? ip_local_deliver+0x62/0x230
 ? ip_rcv_finish+0x670/0x670
 ip_rcv_finish+0x1ae/0x670
 ip_rcv+0x366/0x4d0
 ? ip_rcv+0x26a/0x4d0
 ? inet_del_offload+0x40/0x40
 __netif_receive_skb_core+0xae1/0xc80
 ? inet_del_offload+0x40/0x40
 ? netif_receive_skb_internal+0x29/0x200
 __netif_receive_skb+0x18/0x60
 netif_receive_skb_internal+0x7b/0x200
 ? netif_receive_skb_internal+0x29/0x200
 netif_receive_skb+0xcd/0x130
 br_pass_frame_up+0x2b1/0x2c0
 ? br_pass_frame_up+0xad/0x2c0
 ? br_allowed_ingress+0x38a/0x5d0
 ? br_allowed_ingress+0x1f5/0x5d0
 br_handle_frame_finish+0x28f/0x5a0
 ? br_handle_frame+0x1c1/0x5e0
 br_handle_frame+0x2c5/0x5e0
 ? br_handle_frame+0x1c1/0x5e0
 ? vlan_do_receive+0x37/0x380
 ? br_handle_frame_finish+0x5a0/0x5a0
 

[PATCH 6/8] crypto: DH - add PKCS#3 parameter handling

[Stephan Müller]

DH parameters are commonly handled in PKCS#3 ASN.1 DER encoded files.
The addition adds support for the parsing of such DER encoded parameter
sets. After parsing, the data is added to the DH context data structure.

This support allows using of parameter sets generated with the openssl
dhparam command.

Note, the privateValueLength parameter defined in PKCS#3 is not parsed
as it is not required for the DH operation.

The PKCS#3 parser is able to parse data created by other tools, such as:

openssl dhparam -outform DER -out dhparam.der 2048

Signed-off-by: Stephan Mueller 
---
 crypto/Makefile |  7 ++-
 crypto/dh.c | 15 ++
 crypto/dh_helper.c  | 53 ++---
 crypto/dhparameter.asn1 |  4 
 include/crypto/dh.h | 21 +---
 5 files changed, 93 insertions(+), 7 deletions(-)
 create mode 100644 crypto/dhparameter.asn1

diff --git a/crypto/Makefile b/crypto/Makefile
index 8a44057..a25cfdd 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -30,7 +30,12 @@ obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o
 obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o
 obj-$(CONFIG_CRYPTO_KPP2) += kpp.o
 
-dh_generic-y := dh.o
+$(obj)/dhparameter-asn1.o: $(obj)/dhparameter-asn1.c $(obj)/dhparameter-asn1.h
+$(obj)/dh_helper.o: $(obj)/dhparameter-asn1.h
+clean-files += dhparameter-asn1.c dhparameter-asn1.h
+
+dh_generic-y := dhparameter-asn1.o
+dh_generic-y += dh.o
 dh_generic-y += dh_helper.o
 obj-$(CONFIG_CRYPTO_DH) += dh_generic.o
 ecdh_generic-y := ecc.o
diff --git a/crypto/dh.c b/crypto/dh.c
index f7be48e..a82dceb 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -83,6 +83,20 @@ static int dh_set_params(struct dh_ctx *ctx, struct dh 
*params)
return 0;
 }
 
+static int dh_set_params_pkcs3(struct crypto_kpp *tfm, const void *param,
+  unsigned int param_len)
+{
+   struct dh_ctx *ctx = dh_get_ctx(tfm);
+   struct dh parsed_params;
+   int ret;
+
+   ret = dh_parse_params_pkcs3(_params, param, param_len);
+   if (ret)
+   return ret;
+
+   return dh_set_params(ctx, _params);
+}
+
 static int dh_set_secret(struct crypto_kpp *tfm, const void *buf,
 unsigned int len)
 {
@@ -163,6 +177,7 @@ static void dh_exit_tfm(struct crypto_kpp *tfm)
 }
 
 static struct kpp_alg dh = {
+   .set_params = dh_set_params_pkcs3,
.set_secret = dh_set_secret,
.generate_public_key = dh_compute_value,
.compute_shared_secret = dh_compute_value,
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index 02db76b..27bffc9 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include "dhparameter-asn1.h"
 
 #define DH_KPP_SECRET_MIN_SIZE (sizeof(struct kpp_secret) + 3 * sizeof(int))
 
@@ -53,13 +54,22 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const 
struct dh *params)
if (len != crypto_dh_key_len(params))
return -EINVAL;
 
+   /* Prevention of out-of-bounds access in decode code path */
+   if ((!params->key && params->key_size) ||
+   (!params->p && params->p_size) ||
+   (!params->g && params->g_size))
+   return -EINVAL;
+
ptr = dh_pack_data(ptr, , sizeof(secret));
ptr = dh_pack_data(ptr, >key_size, sizeof(params->key_size));
ptr = dh_pack_data(ptr, >p_size, sizeof(params->p_size));
ptr = dh_pack_data(ptr, >g_size, sizeof(params->g_size));
-   ptr = dh_pack_data(ptr, params->key, params->key_size);
-   ptr = dh_pack_data(ptr, params->p, params->p_size);
-   dh_pack_data(ptr, params->g, params->g_size);
+   if (params->key)
+   ptr = dh_pack_data(ptr, params->key, params->key_size);
+   if (params->p)
+   ptr = dh_pack_data(ptr, params->p, params->p_size);
+   if (params->g)
+   dh_pack_data(ptr, params->g, params->g_size);
 
return 0;
 }
@@ -93,3 +103,40 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, 
struct dh *params)
return 0;
 }
 EXPORT_SYMBOL_GPL(crypto_dh_decode_key);
+
+int dh_get_p(void *context, size_t hdrlen, unsigned char tag, const void 
*value,
+size_t vlen)
+{
+   struct dh *dh = context;
+
+   /* invalid key provided */
+   if (!value || !vlen)
+   return -EINVAL;
+
+   dh->p = value;
+   dh->p_size = vlen;
+
+   return 0;
+}
+
+int dh_get_g(void *context, size_t hdrlen, unsigned char tag,
+const void *value, size_t vlen)
+{
+   struct dh *dh = context;
+
+   /* invalid base provided */
+   if (!value || !dh->p_size || !vlen || vlen > dh->p_size)
+   return -EINVAL;
+
+   dh->g = value;
+   dh->g_size = vlen;
+
+   return 0;
+}
+
+int dh_parse_params_pkcs3(struct dh *dh, const void *param,
+unsigned int param_len)
+{
+   return 

[PATCH 1/8] crypto: AF_ALG -- add DH keygen / ssgen API

[Stephan Müller]

Add the flags for handling DH key generation and DH shared
secret generation.

Signed-off-by: Stephan Mueller 
---
 include/uapi/linux/if_alg.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index 02e6162..c48eeb6 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -41,5 +41,7 @@ struct af_alg_iv {
 #define ALG_OP_ENCRYPT 1
 #define ALG_OP_SIGN2
 #define ALG_OP_VERIFY  3
+#define ALG_OP_KEYGEN  4
+#define ALG_OP_SSGEN   5
 
 #endif /* _LINUX_IF_ALG_H */
-- 
2.9.3

[PATCH 8/8] crypto: AF_ALG - add KPP support

[Stephan Müller]

The patch externalizes the KPP kernel crypto API to user space. This
allows user space to make use of Diffie-Hellman and EC Diffie-Hellman
operations.

The following operations are supported:

 * DH parameters formatted in PKCS#3 with ALG_SET_DH_PARAMETERS
   setsockopt. The call returns the buffer length user space must
   allocate for the shared secret / public key operation.

 * ECDH curve selection via ALG_SET_ECDH_CURVE setsockopt. The call
   returns the buffer length user space must allocate for the shared
   secret / public key operation.

 * The private key can be set with the ALG_SET_KEY setsockopt. It is
   permissible to provide a NULL key. In this case, the kernel uses the
   stdrng to generate a private key of appropriate size and sets the key
   with the TFM. The idea is that the caller can obtain the public key
   from the private key, exchange it with the peer, inject the peer's
   public key into the kernel and derive the shared secret. This way,
   the private key does not leave the kernel realm. The call returns the
   buffer length user space must allocate for the shared secret / public
   key operation.

 * The public key is obtained from the private key via the recvmsg
   operation. For this operation, no data sent to the kernel via sendmsg
   or sendpage is required.

 * The shared secret is obtained by providing the peer's public key via
   sendmsg / sendpage and reading the shared secret via recvmsg.

Signed-off-by: Stephan Mueller 
---
 crypto/Kconfig |   10 +
 crypto/Makefile|1 +
 crypto/algif_kpp.c | 1230 
 3 files changed, 1241 insertions(+)
 create mode 100644 crypto/algif_kpp.c

diff --git a/crypto/Kconfig b/crypto/Kconfig
index aac4bc9..cf59f76 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1757,6 +1757,16 @@ config CRYPTO_USER_API_AEAD
  This option enables the user-spaces interface for AEAD
  cipher algorithms.
 
+config CRYPTO_USER_API_KPP
+   tristate "User-space interface for key protocol primitives algorithms"
+   depends on NET
+   select CRYPTO_KPP2
+   select CRYPTO_USER_API
+   help
+ This option enables the user-spaces interface for key protocol
+ primitives algorithms. This covers Diffie-Hellman and EC
+ Diffie-Hellman
+
 config CRYPTO_HASH_INFO
bool
 
diff --git a/crypto/Makefile b/crypto/Makefile
index a25cfdd..45d861b 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
 obj-$(CONFIG_CRYPTO_USER_API_SKCIPHER) += algif_skcipher.o
 obj-$(CONFIG_CRYPTO_USER_API_RNG) += algif_rng.o
 obj-$(CONFIG_CRYPTO_USER_API_AEAD) += algif_aead.o
+obj-$(CONFIG_CRYPTO_USER_API_KPP) += algif_kpp.o
 
 #
 # generic algorithms and the async_tx api
diff --git a/crypto/algif_kpp.c b/crypto/algif_kpp.c
new file mode 100644
index 000..664b517
--- /dev/null
+++ b/crypto/algif_kpp.c
@@ -0,0 +1,1230 @@
+/*
+ * algif_kpp: User-space interface for key protocol primitives algorithms
+ *
+ * Copyright (C) 2017, Stephan Mueller 
+ *
+ * This file provides the user-space API for key protocol primitives.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * The following concept of the memory management is used:
+ *
+ * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
+ * filled by user space with the data submitted via sendpage/sendmsg. Filling
+ * up the TX SGL does not cause a crypto operation -- the data will only be
+ * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
+ * provide a buffer which is tracked with the RX SGL.
+ *
+ * During the processing of the recvmsg operation, the cipher request is
+ * allocated and prepared. As part of the recvmsg operation, the processed
+ * TX buffers are extracted from the TX SGL into a separate SGL.
+ *
+ * After the completion of the crypto operation, the RX SGL and the cipher
+ * request is released. The extracted TX SGL parts are released together with
+ * the RX SGL release.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct kpp_tsgl {
+   struct list_head list;
+   unsigned int cur;   /* Last processed SG entry */
+   struct scatterlist sg[0];   /* Array of SGs forming the SGL */
+};
+
+struct kpp_rsgl {
+   struct af_alg_sgl sgl;
+   struct list_head list;
+   size_t sg_num_bytes;/* Bytes of data in that SGL */
+};
+
+struct kpp_async_req {
+   struct kiocb *iocb;
+   struct sock *sk;
+
+   struct kpp_rsgl first_rsgl; /* First RX SG */
+   struct 

[PATCH 5/8] crypto: DH - allow params and key to be set independently

[Stephan Müller]

Parameters are handled independently from the secret key. Therefore,
this patch allows setting of the parameter independently from the secret
key. Before invoking the actual crypto operation, the code must now
check that the secret key and the parameters are all present.

Signed-off-by: Stephan Mueller 
---
 crypto/dh.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/crypto/dh.c b/crypto/dh.c
index 87e3542..f7be48e 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -60,6 +60,10 @@ static int dh_check_params_length(unsigned int p_len)
 
 static int dh_set_params(struct dh_ctx *ctx, struct dh *params)
 {
+   /* If DH parameters are not given, do not check them. */
+   if (!params->p_size && !params->g_size)
+   return 0;
+
if (unlikely(!params->p || !params->g))
return -EINVAL;
 
@@ -111,7 +115,7 @@ static int dh_compute_value(struct kpp_request *req)
if (!val)
return -ENOMEM;
 
-   if (unlikely(!ctx->xa)) {
+   if (unlikely(!ctx->xa || !ctx->p || !ctx->g)) {
ret = -EINVAL;
goto err_free_val;
}
-- 
2.9.3

[RFC PATCH 0/8] crypto: AF_ALG support for KPP

[Stephan Müller]

Hi,

This is an RFC to discuss whether and how to support access to the
KPP kernel crypto API (Diffie-Hellman and EC Diffie-Hellman) by
user space.

The patch set is only meant for discussion and not for production use.
Testing is only performed for the DH part via [1]. The DH testing shows
that the DH operation shows the same results as OpenSSL.

The patch set includes:

- addition of PKCS#3 support for DH to allow domain parameters to be set

- extension of the setsockopt and cmsg API

- addition of AF_ALG KPP interface where the implementation follows the
  exact same coding style as currently discussed for the memory management
  fix patch set for algif_skcipher and algif_aead -- this shall allow
  the immediate incorporation of patches that replace duplicated code with
  common service functions.

The patch 8 describes the different operations that are supported by AF_ALG
KPP. This support includes generation and retaining of the private key
inside the kernel. This private key would never be sent to user space.

I am aware and in fact supported development of the DH support in the keys
subsystem. The question will be raised whether the AF_ALG KPP interface is
superfluous in light of the keys DH support. My answer is that AF_ALG KPP is
orthogonal to the keys DH support. The keys DH support use case is that
the keys are managed inside the kernel and thus has the focus on the
key management. Contrary, AF_ALG KPP does not really focus on key management
but simply externalizes the DH/ECDH support found in the kernel including
hardware acceleration. User space is in full control of the key life cycle.
This way, AF_ALG could be used to complement user-space network protocol
implementations like TLS or IKE to offload the resource intense DH
calculations to accelerators.

A full description of the DH testing and how to compare it to OpenSSL
is present in [1] with the test/kcapi-main.c tool, function "kpp".

[1] https://github.com/smuellerDD/libkcapi/tree/kpp

Stephan Mueller (8):
  crypto: AF_ALG -- add DH keygen / ssgen API
  crypto: AF_ALG -- add DH param / ECDH curve setsockopt call
  crypto: AF_ALG - eliminate code duplication
  crypto: KPP - add API crypto_kpp_set_params
  crypto: DH - allow params and key to be set independently
  crypto: DH - add PKCS#3 parameter handling
  crypto: ecdh - constify key
  crypto: AF_ALG - add KPP support

 Documentation/crypto/api-kpp.rst |2 +-
 crypto/Kconfig   |   10 +
 crypto/Makefile  |8 +-
 crypto/af_alg.c  |   17 +-
 crypto/algif_kpp.c   | 1230 ++
 crypto/dh.c  |   21 +-
 crypto/dh_helper.c   |   53 +-
 crypto/dhparameter.asn1  |4 +
 include/crypto/dh.h  |   21 +-
 include/crypto/ecdh.h|2 +-
 include/crypto/if_alg.h  |2 +
 include/crypto/kpp.h |   28 +
 include/uapi/linux/if_alg.h  |4 +
 13 files changed, 1384 insertions(+), 18 deletions(-)
 create mode 100644 crypto/algif_kpp.c
 create mode 100644 crypto/dhparameter.asn1

-- 
2.9.3

[PATCH 7/8] crypto: ecdh - constify key

[Stephan Müller]

Signed-off-by: Stephan Mueller 
---
 include/crypto/ecdh.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/crypto/ecdh.h b/include/crypto/ecdh.h
index 03a64f6..b5bb149 100644
--- a/include/crypto/ecdh.h
+++ b/include/crypto/ecdh.h
@@ -40,7 +40,7 @@
  */
 struct ecdh {
unsigned short curve_id;
-   char *key;
+   const char *key;
unsigned short key_size;
 };
 
-- 
2.9.3

[PATCH 4/8] crypto: KPP - add API crypto_kpp_set_params

[Stephan Müller]

KPP mechanisms like DH require a parameter set to be provided by the
caller. That parameter set may be provided by the crypto_kpp_set_secret
function. Yet, the parameters hare handled independently from the secret
key which implies that they should be able to be set independently from
the key.

The new API allows KPP mechanisms to register a callback allowing to set
such parameters.

Signed-off-by: Stephan Mueller 
---
 Documentation/crypto/api-kpp.rst |  2 +-
 include/crypto/kpp.h | 28 
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/Documentation/crypto/api-kpp.rst b/Documentation/crypto/api-kpp.rst
index 7d86ab9..7b2c0d4 100644
--- a/Documentation/crypto/api-kpp.rst
+++ b/Documentation/crypto/api-kpp.rst
@@ -11,7 +11,7 @@ Key-agreement Protocol Primitives (KPP) Cipher API
:doc: Generic Key-agreement Protocol Primitives API
 
 .. kernel-doc:: include/crypto/kpp.h
-   :functions: crypto_alloc_kpp crypto_free_kpp crypto_kpp_set_secret 
crypto_kpp_generate_public_key crypto_kpp_compute_shared_secret 
crypto_kpp_maxsize
+   :functions: crypto_alloc_kpp crypto_free_kpp crypto_kpp_set_params 
crypto_kpp_set_secret crypto_kpp_generate_public_key 
crypto_kpp_compute_shared_secret crypto_kpp_maxsize
 
 Key-agreement Protocol Primitives (KPP) Cipher Request Handle
 -
diff --git a/include/crypto/kpp.h b/include/crypto/kpp.h
index ce8e1f7..5931364 100644
--- a/include/crypto/kpp.h
+++ b/include/crypto/kpp.h
@@ -51,6 +51,9 @@ struct crypto_kpp {
 /**
  * struct kpp_alg - generic key-agreement protocol primitives
  *
+ * @set_params:Function allows the caller to set the parameters
+ * separately from the key. The format of the parameters
+ * is protocol specific.
  * @set_secret:Function invokes the protocol specific function 
to
  * store the secret private key along with parameters.
  * The implementation knows how to decode thie buffer
@@ -74,6 +77,8 @@ struct crypto_kpp {
  * @base:  Common crypto API algorithm data structure
  */
 struct kpp_alg {
+   int (*set_params)(struct crypto_kpp *tfm, const void *buffer,
+ unsigned int len);
int (*set_secret)(struct crypto_kpp *tfm, const void *buffer,
  unsigned int len);
int (*generate_public_key)(struct kpp_request *req);
@@ -259,6 +264,29 @@ struct kpp_secret {
 };
 
 /**
+ * crypto_kpp_set_params() - Set parameters needed for kpp operation
+ *
+ * Function invokes the specific kpp operation for a given alg.
+ *
+ * @tfm:   tfm handle
+ * @buffer:Buffer holding the protocol specific representation of the
+ * parameters (e.g. PKCS#3 DER for DH)
+ * @len:   Length of the parameter buffer.
+ *
+ * Return: zero on success; error code in case of error
+ */
+static inline int crypto_kpp_set_params(struct crypto_kpp *tfm,
+   const void *buffer, unsigned int len)
+{
+   struct kpp_alg *alg = crypto_kpp_alg(tfm);
+
+   if (alg->set_params)
+   return alg->set_params(tfm, buffer, len);
+   else
+   return -EOPNOTSUPP;
+}
+
+/**
  * crypto_kpp_set_secret() - Invoke kpp operation
  *
  * Function invokes the specific kpp operation for a given alg.
-- 
2.9.3

[PATCH 3/8] crypto: AF_ALG - eliminate code duplication

[Stephan Müller]

The handling function for setsockopt contains duplicated code which is
cleaned up with this patch. This patch does not change the functionality.

Signed-off-by: Stephan Mueller 
---
 crypto/af_alg.c | 17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a3210bf..f2b2865 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -247,34 +247,23 @@ static int alg_setsockopt(struct socket *sock, int level, 
int optname,
if (level != SOL_ALG || !type)
goto unlock;
 
+   if (sock->state == SS_CONNECTED)
+   goto unlock;
+
switch (optname) {
case ALG_SET_KEY:
-   if (sock->state == SS_CONNECTED)
-   goto unlock;
-
err = alg_setkey(sk, optval, optlen, type->setkey);
break;
case ALG_SET_PUBKEY:
-   if (sock->state == SS_CONNECTED)
-   goto unlock;
-
err = alg_setkey(sk, optval, optlen, type->setpubkey);
break;
case ALG_SET_DH_PARAMETERS:
-   if (sock->state == SS_CONNECTED)
-   goto unlock;
-
err = alg_setkey(sk, optval, optlen, type->dhparams);
break;
case ALG_SET_ECDH_CURVE:
-   if (sock->state == SS_CONNECTED)
-   goto unlock;
-
err = alg_setkey(sk, optval, optlen, type->ecdhcurve);
break;
case ALG_SET_AEAD_AUTHSIZE:
-   if (sock->state == SS_CONNECTED)
-   goto unlock;
if (!type->setauthsize)
goto unlock;
err = type->setauthsize(ask->private, optlen);
-- 
2.9.3

[PATCH 2/8] crypto: AF_ALG -- add DH param / ECDH curve setsockopt call

[Stephan Müller]

For supporting DH ciphers, user space must be able to set the
DH parameters. The patch adds a new setsockopt call for setting
these parameters.

Similarly, the ECDH curve information can be set by user space via the
newly added setsockopt call.

Signed-off-by: Stephan Mueller 
---
 crypto/af_alg.c | 12 
 include/crypto/if_alg.h |  2 ++
 include/uapi/linux/if_alg.h |  2 ++
 3 files changed, 16 insertions(+)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 0a91404..a3210bf 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -260,6 +260,18 @@ static int alg_setsockopt(struct socket *sock, int level, 
int optname,
 
err = alg_setkey(sk, optval, optlen, type->setpubkey);
break;
+   case ALG_SET_DH_PARAMETERS:
+   if (sock->state == SS_CONNECTED)
+   goto unlock;
+
+   err = alg_setkey(sk, optval, optlen, type->dhparams);
+   break;
+   case ALG_SET_ECDH_CURVE:
+   if (sock->state == SS_CONNECTED)
+   goto unlock;
+
+   err = alg_setkey(sk, optval, optlen, type->ecdhcurve);
+   break;
case ALG_SET_AEAD_AUTHSIZE:
if (sock->state == SS_CONNECTED)
goto unlock;
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 3c3ebe3..52dcefe 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -53,6 +53,8 @@ struct af_alg_type {
void (*release)(void *private);
int (*setkey)(void *private, const u8 *key, unsigned int keylen);
int (*setpubkey)(void *private, const u8 *key, unsigned int keylen);
+   int (*dhparams)(void *private, const u8 *param, unsigned int paramlen);
+   int (*ecdhcurve)(void *private, const u8 *param, unsigned int paramlen);
int (*accept)(void *private, struct sock *sk);
int (*accept_nokey)(void *private, struct sock *sk);
int (*setauthsize)(void *private, unsigned int authsize);
diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index c48eeb6..fc9ed9f 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,8 @@ struct af_alg_iv {
 #define ALG_SET_AEAD_ASSOCLEN  4
 #define ALG_SET_AEAD_AUTHSIZE  5
 #define ALG_SET_PUBKEY 6
+#define ALG_SET_DH_PARAMETERS  7
+#define ALG_SET_ECDH_CURVE 8
 
 /* Operations */
 #define ALG_OP_DECRYPT 0
-- 
2.9.3

[PATCH 2/6] ima: Tidy up constant strings

[Thiago Jung Bauermann]

Strictly speaking, boot_aggregate_name is a constant string, not a
modifiable pointer to a constant string.

Also, constify mask_tokens and func_tokens arrays.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima_init.c   | 2 +-
 security/integrity/ima/ima_policy.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/ima_init.c 
b/security/integrity/ima/ima_init.c
index 2967d497a665..24c703366ba8 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -25,7 +25,7 @@
 #include "ima.h"
 
 /* name for boot aggregate entry */
-static const char *boot_aggregate_name = "boot_aggregate";
+static const char boot_aggregate_name[] = "boot_aggregate";
 int ima_used_chip;
 
 /* Add the boot aggregate to the IMA measurement list and extend
diff --git a/security/integrity/ima/ima_policy.c 
b/security/integrity/ima/ima_policy.c
index aed47b777a57..cfda5d7b17ec 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -889,7 +889,7 @@ enum {
mask_exec = 0, mask_write, mask_read, mask_append
 };
 
-static char *mask_tokens[] = {
+static const char *const mask_tokens[] = {
"MAY_EXEC",
"MAY_WRITE",
"MAY_READ",
@@ -903,7 +903,7 @@ enum {
func_policy
 };
 
-static char *func_tokens[] = {
+static const char *const func_tokens[] = {
"FILE_CHECK",
"MMAP_CHECK",
"BPRM_CHECK",
-- 
2.7.4

[PATCH 5/6] 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_signature.h | 45 
 init/Kconfig |  6 +++-
 kernel/Makefile  |  2 +-
 kernel/module_signing.c  | 74 +---
 4 files changed, 82 insertions(+), 45 deletions(-)

diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
new file mode 100644
index ..b04f16559b47
--- /dev/null
+++ b/include/linux/module_signature.h
@@ -0,0 +1,45 @@
+/* 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
+
+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_signature(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 a92f27da4a27..891325e5aeff 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2024,7 +2024,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
@@ -2039,6 +2039,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 b302b4731d16..4451bbc70a08 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -56,7 +56,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_KEXEC_CORE) += kexec_core.o
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 937c844bee4a..421e3e668714 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 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:
+/**
+ * validate_module_signature - validate that the given signature is sane
  *
- * - Signer's name
- * - Key identifier
- * - Signature data
- * - Information block
+ * @ms:Signature to validate.
+ * @file_len:  Size of the file to which @ms is appended.
  */
-struct module_signature {
-   u8  algo;   /* Public-key crypto algorithm [0] */
-   u8  hash;   /* Digest algorithm [0] */
-   u8  id_type;/* Key identifier type [PKEY_ID_PKCS7] */
-  

[PATCH 3/6] ima: Simplify policy_func_show.

[Thiago Jung Bauermann]

If the func_tokens array uses the same indices as enum ima_hooks,
policy_func_show can be a lot simpler, and the func_* enum becomes
unnecessary.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima_policy.c | 47 ++---
 1 file changed, 7 insertions(+), 40 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c 
b/security/integrity/ima/ima_policy.c
index cfda5d7b17ec..158eafef64e8 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -896,20 +896,14 @@ static const char *const mask_tokens[] = {
"MAY_APPEND"
 };
 
-enum {
-   func_file = 0, func_mmap, func_bprm,
-   func_module, func_firmware, func_post,
-   func_kexec_kernel, func_kexec_initramfs,
-   func_policy
-};
-
 static const char *const func_tokens[] = {
+   NULL,
"FILE_CHECK",
"MMAP_CHECK",
"BPRM_CHECK",
+   "POST_SETATTR",
"MODULE_CHECK",
"FIRMWARE_CHECK",
-   "POST_SETATTR",
"KEXEC_KERNEL_CHECK",
"KEXEC_INITRAMFS_CHECK",
"POLICY_CHECK"
@@ -949,48 +943,21 @@ void ima_policy_stop(struct seq_file *m, void *v)
 
 #define pt(token)  policy_tokens[token + Opt_err].pattern
 #define mt(token)  mask_tokens[token]
-#define ft(token)  func_tokens[token]
 
 /*
  * policy_func_show - display the ima_hooks policy rule
  */
 static void policy_func_show(struct seq_file *m, enum ima_hooks func)
 {
-   char tbuf[64] = {0,};
+   if (func > 0 && func < MAX_CHECK)
+   seq_printf(m, pt(Opt_func), func_tokens[func]);
+   else {
+   char tbuf[64] = {0,};
 
-   switch (func) {
-   case FILE_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_file));
-   break;
-   case MMAP_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_mmap));
-   break;
-   case BPRM_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_bprm));
-   break;
-   case MODULE_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_module));
-   break;
-   case FIRMWARE_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_firmware));
-   break;
-   case POST_SETATTR:
-   seq_printf(m, pt(Opt_func), ft(func_post));
-   break;
-   case KEXEC_KERNEL_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_kexec_kernel));
-   break;
-   case KEXEC_INITRAMFS_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_kexec_initramfs));
-   break;
-   case POLICY_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_policy));
-   break;
-   default:
snprintf(tbuf, sizeof(tbuf), "%d", func);
seq_printf(m, pt(Opt_func), tbuf);
-   break;
}
+
seq_puts(m, " ");
 }
 
-- 
2.7.4

[PATCH 1/6] integrity: Small code improvements

[Thiago Jung Bauermann]

The keyid and sig_size members of struct signature_v2_hdr are in BE format,
so use a type that makes this assumption explicit. Also, use beXX_to_cpu
instead of __beXX_to_cpu to read them.

Change integrity_kernel_read to take a void * buffer instead of char *
buffer, so that callers don't have to use a cast if they provide a buffer
that isn't a char *.

Also, add missing fall through comment in ima_appraise.c.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/digsig_asymmetric.c | 4 ++--
 security/integrity/iint.c  | 2 +-
 security/integrity/ima/ima_appraise.c  | 1 +
 security/integrity/integrity.h | 7 ---
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/security/integrity/digsig_asymmetric.c 
b/security/integrity/digsig_asymmetric.c
index 80052ed8d467..ab6a029062a1 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -92,13 +92,13 @@ int asymmetric_verify(struct key *keyring, const char *sig,
 
siglen -= sizeof(*hdr);
 
-   if (siglen != __be16_to_cpu(hdr->sig_size))
+   if (siglen != be16_to_cpu(hdr->sig_size))
return -EBADMSG;
 
if (hdr->hash_algo >= HASH_ALGO__LAST)
return -ENOPKG;
 
-   key = request_asymmetric_key(keyring, __be32_to_cpu(hdr->keyid));
+   key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
if (IS_ERR(key))
return PTR_ERR(key);
 
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index c710d22042f9..6fc888ca468e 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -182,7 +182,7 @@ security_initcall(integrity_iintcache_init);
  *
  */
 int integrity_kernel_read(struct file *file, loff_t offset,
- char *addr, unsigned long count)
+ void *addr, unsigned long count)
 {
mm_segment_t old_fs;
char __user *buf = (char __user *)addr;
diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index 1fd9539a969d..427a896bc806 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -227,6 +227,7 @@ int ima_appraise_measurement(enum ima_hooks func,
case IMA_XATTR_DIGEST_NG:
/* first byte contains algorithm id */
hash_start = 1;
+   /* fall through */
case IMA_XATTR_DIGEST:
if (iint->flags & IMA_DIGSIG_REQUIRED) {
cause = "IMA-signature-required";
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 24520b4ef3b0..a53e7e4ab06c 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -92,8 +92,8 @@ struct signature_v2_hdr {
uint8_t type;   /* xattr type */
uint8_t version;/* signature format version */
uint8_t hash_algo;  /* Digest algorithm [enum hash_algo] */
-   uint32_t keyid; /* IMA key identifier - not X509/PGP specific */
-   uint16_t sig_size;  /* signature size */
+   __be32 keyid;   /* IMA key identifier - not X509/PGP specific */
+   __be16 sig_size;/* signature size */
uint8_t sig[0]; /* signature payload */
 } __packed;
 
@@ -118,7 +118,8 @@ struct integrity_iint_cache {
 struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
 
 int integrity_kernel_read(struct file *file, loff_t offset,
- char *addr, unsigned long count);
+ void *addr, unsigned long count);
+
 int __init integrity_read_file(const char *path, char **data);
 
 #define INTEGRITY_KEYRING_EVM  0
-- 
2.7.4

[PATCH 4/6] ima: Log the same audit cause whenever a file has no signature

[Thiago Jung Bauermann]

If the file doesn't have an xattr, ima_appraise_measurement sets cause to
"missing-hash" while if there's an xattr but it's a digest instead of a
signature it sets cause to "IMA-signature-required".

Fix it by setting cause to "IMA-signature-required" in both cases.

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

diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index 427a896bc806..8c8030a29117 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -205,7 +205,8 @@ int ima_appraise_measurement(enum ima_hooks func,
if (rc && rc != -ENODATA)
goto out;
 
-   cause = "missing-hash";
+   cause = iint->flags & IMA_DIGSIG_REQUIRED ?
+   "IMA-signature-required" : "missing-hash";
status = INTEGRITY_NOLABEL;
if (opened & FILE_CREATED) {
iint->flags |= IMA_NEW_FILE;
-- 
2.7.4

[PATCH 6/6] ima: Support appended signatures for appraisal

[Thiago Jung Bauermann]

This patch introduces the appended_imasig 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=appended_imasig
appraise func=KEXEC_KERNEL_CHECK appraise_type=appended_imasig|imasig

In the second form, 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.

The format of the appended signature is the same used for signed kernel
modules. This means that the file can be signed with the scripts/sign-file
tool, with a command line such as this:

$ sign-file sha256 privkey_ima.pem x509_ima.der vmlinux

This code only works for files that are hashed from a memory buffer, not
for files that are read from disk at the time of hash calculation. In other
words, only hooks that use kernel_read_file can support appended
signatures.

The change in CONFIG_INTEGRITY_SIGNATURE to select CONFIG_KEYS instead of
depending on it is to avoid a dependency recursion in
CONFIG_IMA_APPRAISE_APPENDED_SIG, because CONFIG_MODULE_SIG_FORMAT selects
CONFIG_KEYS and Kconfig complains that CONFIG_INTEGRITY_SIGNATURE depends
on it.

Signed-off-by: Thiago Jung Bauermann 
---
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 crypto/asymmetric_keys/pkcs7_parser.c| 12 +
 crypto/asymmetric_keys/pkcs7_verify.c| 13 +
 include/crypto/pkcs7.h   |  3 ++
 include/linux/verification.h |  1 +
 security/integrity/Kconfig   |  2 +-
 security/integrity/ima/Kconfig   | 13 +
 security/integrity/ima/ima.h |  8 +++
 security/integrity/ima/ima_appraise.c| 84 +++-
 security/integrity/ima/ima_main.c| 30 ++--
 security/integrity/ima/ima_policy.c  | 43 ++--
 security/integrity/integrity.h   | 20 +---
 12 files changed, 204 insertions(+), 26 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c 
b/crypto/asymmetric_keys/asymmetric_type.c
index e4b0ed386bc8..0d58ecfba2ea 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -28,6 +28,7 @@ const char *const key_being_used_for[NR__KEY_BEING_USED_FOR] 
= {
[VERIFYING_KEXEC_PE_SIGNATURE]  = "kexec PE sig",
[VERIFYING_KEY_SIGNATURE]   = "key sig",
[VERIFYING_KEY_SELF_SIGNATURE]  = "key self sig",
+   [VERIFYING_KEXEC_CMS_SIGNATURE] = "kexec CMS sig",
[VERIFYING_UNSPECIFIED_SIGNATURE]   = "unspec sig",
 };
 EXPORT_SYMBOL_GPL(key_being_used_for);
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 hdrlen,
return -ENOMEM;
return 0;
 }
+
+/**
+ * pkcs7_get_message_sig - get signature in @pkcs7
+ *
+ * This function doesn't meaningfully support messages with more than one
+ * signature. It will always return the first signature.
+ */
+const struct public_key_signature *pkcs7_get_message_sig(
+   const struct pkcs7_message *pkcs7)
+{
+   return pkcs7->signed_infos ? pkcs7->signed_infos->sig : NULL;
+}
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c 
b/crypto/asymmetric_keys/pkcs7_verify.c
index 2d93d9eccb4d..eee78074580a 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -420,6 +420,19 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
return -EKEYREJECTED;
}
break;
+   case VERIFYING_KEXEC_CMS_SIGNATURE:
+   /* Shipping certificates in the CMS message is not allowed. */
+   if (pkcs7->certs) {
+   pr_warn("Signature isn't allowed to contain 
certificates.\n");
+   return -EBADMSG;
+   }
+
+   /* Shipping CRLs in the CMS message is not allowed. */
+   if (pkcs7->crl) {
+   pr_warn("Signature isn't allowed to contain CRLs.\n");
+   return -EBADMSG;
+   }
+   break;
default:
return -EINVAL;
}
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 583f199400a3..a05a0d7214e6 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -29,6 +29,9 @@ extern int pkcs7_get_content_data(const struct pkcs7_message 
*pkcs7,
  const void **_data, size_t *_datalen,
  size_t *_headerlen);
 
+extern const struct 

[PATCH 0/6] Appended signatures support for IMA appraisal

[Thiago Jung Bauermann]

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 can boot kernels
with the signature appended to them as well as kernels where the signature
is stored in the IMA extended attribute.

The first four patches are cleanups and improvements that can be taken
independently from the others (and from each other as well). The last two
are the ones actually focused on this feature.

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

Thiago Jung Bauermann (6):
  integrity: Small code improvements
  ima: Tidy up constant strings
  ima: Simplify policy_func_show.
  ima: Log the same audit cause whenever a file has no signature
  MODSIGN: Export module signature definitions.
  ima: Support appended signatures for appraisal

 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 crypto/asymmetric_keys/pkcs7_parser.c| 12 +
 crypto/asymmetric_keys/pkcs7_verify.c| 13 +
 include/crypto/pkcs7.h   |  3 ++
 include/linux/module_signature.h | 45 
 include/linux/verification.h |  1 +
 init/Kconfig |  6 ++-
 kernel/Makefile  |  2 +-
 kernel/module_signing.c  | 74 +++
 security/integrity/Kconfig   |  2 +-
 security/integrity/digsig_asymmetric.c   |  4 +-
 security/integrity/iint.c|  2 +-
 security/integrity/ima/Kconfig   | 13 +
 security/integrity/ima/ima.h |  8 +++
 security/integrity/ima/ima_appraise.c| 86 ++-
 security/integrity/ima/ima_init.c|  2 +-
 security/integrity/ima/ima_main.c| 30 +--
 security/integrity/ima/ima_policy.c  | 88 
 security/integrity/integrity.h   | 27 ++
 19 files changed, 302 insertions(+), 117 deletions(-)
 create mode 100644 include/linux/module_signature.h

-- 
2.7.4

[patch V2 04/24] padata: Avoid nested calls to get_online_cpus() in pcrypt_init_padata()

[Thomas Gleixner]

From: Sebastian Andrzej Siewior 

pcrypt_init_padata()
   get_online_cpus()
   padata_alloc_possible()
 padata_alloc()
   get_online_cpus()

The nested call to get_online_cpus() works with the current implementation,
but prevents the conversion to a percpu rwsem.

The other caller of padata_alloc_possible() is pcrypt_init_padata() which
calls from a get_online_cpus() protected region as well.

Remove the get_online_cpus() call in padata_alloc() and document the
calling convention.

Signed-off-by: Sebastian Andrzej Siewior 
Signed-off-by: Thomas Gleixner 
Cc: Steffen Klassert 
Cc: linux-crypto@vger.kernel.org

---
 kernel/padata.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -945,6 +945,8 @@ static struct kobj_type padata_attr_type
  * @wq: workqueue to use for the allocated padata instance
  * @pcpumask: cpumask that will be used for padata parallelization
  * @cbcpumask: cpumask that will be used for padata serialization
+ *
+ * Must be called from a get_online_cpus() protected region
  */
 static struct padata_instance *padata_alloc(struct workqueue_struct *wq,
const struct cpumask *pcpumask,
@@ -957,7 +959,6 @@ static struct padata_instance *padata_al
if (!pinst)
goto err;
 
-   get_online_cpus();
if (!alloc_cpumask_var(>cpumask.pcpu, GFP_KERNEL))
goto err_free_inst;
if (!alloc_cpumask_var(>cpumask.cbcpu, GFP_KERNEL)) {
@@ -997,7 +998,6 @@ static struct padata_instance *padata_al
free_cpumask_var(pinst->cpumask.cbcpu);
 err_free_inst:
kfree(pinst);
-   put_online_cpus();
 err:
return NULL;
 }
@@ -1008,6 +1008,8 @@ static struct padata_instance *padata_al
  * parallel workers.
  *
  * @wq: workqueue to use for the allocated padata instance
+ *
+ * Must be called from a get_online_cpus() protected region
  */
 struct padata_instance *padata_alloc_possible(struct workqueue_struct *wq)
 {

[patch V2 03/24] padata: Make padata_alloc() static

[Thomas Gleixner]

No users outside of padata.c

Signed-off-by: Thomas Gleixner 
Cc: Steffen Klassert 
Cc: linux-crypto@vger.kernel.org
---
 include/linux/padata.h |3 ---
 kernel/padata.c|   32 
 2 files changed, 16 insertions(+), 19 deletions(-)

--- a/include/linux/padata.h
+++ b/include/linux/padata.h
@@ -166,9 +166,6 @@ struct padata_instance {
 
 extern struct padata_instance *padata_alloc_possible(
struct workqueue_struct *wq);
-extern struct padata_instance *padata_alloc(struct workqueue_struct *wq,
-   const struct cpumask *pcpumask,
-   const struct cpumask *cbcpumask);
 extern void padata_free(struct padata_instance *pinst);
 extern int padata_do_parallel(struct padata_instance *pinst,
  struct padata_priv *padata, int cb_cpu);
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -939,19 +939,6 @@ static struct kobj_type padata_attr_type
 };
 
 /**
- * padata_alloc_possible - Allocate and initialize padata instance.
- * Use the cpu_possible_mask for serial and
- * parallel workers.
- *
- * @wq: workqueue to use for the allocated padata instance
- */
-struct padata_instance *padata_alloc_possible(struct workqueue_struct *wq)
-{
-   return padata_alloc(wq, cpu_possible_mask, cpu_possible_mask);
-}
-EXPORT_SYMBOL(padata_alloc_possible);
-
-/**
  * padata_alloc - allocate and initialize a padata instance and specify
  *cpumasks for serial and parallel workers.
  *
@@ -959,9 +946,9 @@ EXPORT_SYMBOL(padata_alloc_possible);
  * @pcpumask: cpumask that will be used for padata parallelization
  * @cbcpumask: cpumask that will be used for padata serialization
  */
-struct padata_instance *padata_alloc(struct workqueue_struct *wq,
-const struct cpumask *pcpumask,
-const struct cpumask *cbcpumask)
+static struct padata_instance *padata_alloc(struct workqueue_struct *wq,
+   const struct cpumask *pcpumask,
+   const struct cpumask *cbcpumask)
 {
struct padata_instance *pinst;
struct parallel_data *pd = NULL;
@@ -1016,6 +1003,19 @@ struct padata_instance *padata_alloc(str
 }
 
 /**
+ * padata_alloc_possible - Allocate and initialize padata instance.
+ * Use the cpu_possible_mask for serial and
+ * parallel workers.
+ *
+ * @wq: workqueue to use for the allocated padata instance
+ */
+struct padata_instance *padata_alloc_possible(struct workqueue_struct *wq)
+{
+   return padata_alloc(wq, cpu_possible_mask, cpu_possible_mask);
+}
+EXPORT_SYMBOL(padata_alloc_possible);
+
+/**
  * padata_free - free a padata instance
  *
  * @padata_inst: padata instance to free

Re: export pcie_flr and remove copies of it in drivers V2

[Bjorn Helgaas]

On Fri, Apr 14, 2017 at 09:11:24PM +0200, Christoph Hellwig wrote:
> Hi all,
> 
> this exports the PCI layer pcie_flr helper, and removes various opencoded
> copies of it.
> 
> Changes since V1:
>  - rebase on top of the pci/virtualization branch
>  - fixed the probe case in __pci_dev_reset
>  - added ACKs from Bjorn

Applied the first three patches:
 
  bc13871ef35a PCI: Export pcie_flr()
  e641c375d414 PCI: Call pcie_flr() from reset_intel_82599_sfp_virtfn()
  40e0901ea4bf PCI: Call pcie_flr() from reset_chelsio_generic_dev()

to pci/virtualization for v4.12, thanks!

Re: [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT

[Matthias Kaehlcke]

El Tue, Apr 18, 2017 at 04:35:02PM +0100 Ard Biesheuvel ha dit:

> On 18 April 2017 at 15:47, Paul Gortmaker  
> wrote:
> > On Wed, Apr 5, 2017 at 2:34 PM, Matthias Kaehlcke  wrote:
> >> The operand is an integer constant, make the constness explicit by
> >> adding the modifier. This is needed for clang to generate valid code
> >> and also works with gcc.
> >
> > Actually it doesn't work with all gcc.  I've got an older arm64 toolchain 
> > that I
> > only use for syntax checking (and hence I don't care if it is the latest and
> > greatest) and this commit breaks it:
> >
> > arch/arm64/crypto/sha1-ce-glue.c:21:2: error: invalid 'asm': invalid
> > operand prefix '%c'
> >   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));

Sorry about that :(

> > I'm currently reverting this change locally so I can continue to use the old
> > toolchain:
> >
> > $ aarch64-linux-gnu-gcc --version
> > aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro
> > GCC 2013.11) 4.8.3 20131202 (prerelease)
> > Copyright (C) 2013 Free Software Foundation, Inc.
> >
> > $ aarch64-linux-gnu-as --version
> > GNU assembler (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro GCC
> > 2013.11) 2.24.0.20131220
> > Copyright 2013 Free Software Foundation, Inc.
> >
> > Maybe it is finally too old and nobody cares, but I thought it worth a 
> > mention.
> >
> 
> Thanks for the report. I think we care more about GCC 4.8 than about
> Clang, which argues for reverting this patch.

I agree, we certainly shouldn't break GCC.

> I understand these issues must be frustrating if you are working on
> this stuff, but to me, it is not entirely obvious why we want to
> support Clang in the first place (i.e., what does it buy you if your
> distro/environment is not already using Clang for userland),

There are environments that use Clang for userland, like Chrome OS or
Android. Debian releases with GCC, but also does Clang builds of most
of its repository: http://clang.debian.net/

I would also argue that Linux directly benefits from additional error
checks provided by Clang.

> and why the burden is on Linux to make modifications to support
> Clang, especially when it comes to GCC extensions such as inline
> assembly syntax.

Personally I'm on the pragmatic side. 99.9x% of the kernel already
builds with Clang (at least for arm64 and x86), only a very tiny
fraction of the code needs adaptation, which should ideally result in
the same code for gcc and clang. And it's not like Clang is just
another proprietary or niche compiler, it's the other big open source
compiler out there, since the effort is relatively small it seems
desirable to support it without out-of-tree patches. In parallel there
is also work ongoing with upstream Clang to improve compatiblity.

> It is ultimately up to the maintainers to decide what to do with this
> patch, but my vote would be to revert it, especially given that the %c
> placeholder prefix is not documented anywhere, and appears to simply
> trigger some GCC internals that happen to do the right thing in this
> case.
> 
> However, the I -> i change is arguably an improvement, and considering
> that the following
> 
> asm("foo: .long %0" :: "i"(some value))
> 
> doesn't compile with clang either, I suggest you (Matthias) file a bug
> against Clang to get this fixed, and we can propose another patch just
> for the I->i change.

Ok, I will see if we can get this fixed in Clang.

Thanks

Matthias

Re: [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function

[Logan Gunthorpe]

On 18/04/17 09:50 AM, Konrad Rzeszutek Wilk wrote:
> I am not sure if you know, but you can add on each patch the respective
> maintainer via 'CC'. That way you can have certain maintainers CCed only
> on the subsystems they cover. You put it after (or before) your SoB and
> git send-email happilly picks it up.

Yes, but I've seen some maintainers complain when they receive a patch
with no context (ie. cover letter and first patch). So I chose to do it
this way. I expect in this situation, no matter what you do, someone is
going to complain about the approach chosen.

Thanks anyway for the tip.

Logan

Re: [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function

[Konrad Rzeszutek Wilk]

On Tue, Apr 18, 2017 at 09:42:20AM -0600, Logan Gunthorpe wrote:
> 
> 
> On 18/04/17 08:27 AM, Konrad Rzeszutek Wilk wrote:
> > Interesting that you didn't CC any of the maintainers. Could you 
> > do that in the future please?
> 
> Please read the cover letter. The distribution list for the patchset
> would have been way too large to cc every maintainer (even as limited as
> it was, I had mailing lists yelling at me). My plan was to get buy in

I am not sure if you know, but you can add on each patch the respective
maintainer via 'CC'. That way you can have certain maintainers CCed only
on the subsystems they cover. You put it after (or before) your SoB and
git send-email happilly picks it up.

It does mean that for every patch you have to run something like this:

$ more add_cc 
#!/bin/bash

git diff HEAD^.. > /tmp/a
echo "---"
scripts/get_maintainer.pl --no-l /tmp/a | while read file
do
echo "Cc: $file"
done

Or such.


> for the first patch, get it merged and resend the rest independently to
> their respective maintainers. Of course, though, I'd be open to other
> suggestions.
> 
> >>>
> >>> Signed-off-by: Logan Gunthorpe 
> >>> ---
> >>>  drivers/block/xen-blkfront.c | 33 +++--
> >>>  1 file changed, 27 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> >>> index 5067a0a..7dcf41d 100644
> >>> --- a/drivers/block/xen-blkfront.c
> >>> +++ b/drivers/block/xen-blkfront.c
> >>> @@ -807,8 +807,19 @@ static int blkif_queue_rw_req(struct request *req, 
> >>> struct blkfront_ring_info *ri
> >>>   BUG_ON(sg->offset + sg->length > PAGE_SIZE);
> >>>
> >>>   if (setup.need_copy) {
> >>> - setup.bvec_off = sg->offset;
> >>> - setup.bvec_data = kmap_atomic(sg_page(sg));
> >>> + setup.bvec_off = 0;
> >>> + setup.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
> >>> + if (IS_ERR(setup.bvec_data)) {
> >>> + /*
> >>> +  * This should really never happen unless
> >>> +  * the code is changed to use memory that is
> >>> +  * not mappable in the sg. Seeing there is a
> >>> +  * questionable error path out of here,
> >>> +  * we WARN.
> >>> +  */
> >>> + WARN(1, "Non-mappable memory used in sg!");
> >>> + return 1;
> >>> + }
> >> ...
> >>
> >> Perhaps add a flag to mark failure as 'unexpected' and trace (and panic?)
> >> inside sg_map().
> 
> Thanks, that's a good suggestion. I'll make the change for v2.
> 
> Logan

Re: [PATCH 05/22] drm/i915: Make use of the new sg_map helper function

[Logan Gunthorpe]

On 18/04/17 12:44 AM, Daniel Vetter wrote:
> On Thu, Apr 13, 2017 at 04:05:18PM -0600, Logan Gunthorpe wrote:
>> This is a single straightforward conversion from kmap to sg_map.
>>
>> Signed-off-by: Logan Gunthorpe 
> 
> Acked-by: Daniel Vetter 
> 
> Probably makes sense to merge through some other tree, but please be aware
> of the considerable churn rate in i915 (i.e. make sure your tree is in
> linux-next before you send a pull request for this). Plane B would be to
> get the prep patch in first and then merge the i915 conversion one kernel
> release later.

Yes, as per what I said in my cover letter, I was leaning towards a
"Plan B" style approach.

Logan

Re: [PATCH 0/4] staging: add ccree crypto driver

[Mark Rutland]

On Tue, Apr 18, 2017 at 06:29:22PM +0300, Gilad Ben-Yossef wrote:
> On Tue, Apr 18, 2017 at 6:13 PM, Mark Rutland  wrote:
> > On Tue, Apr 18, 2017 at 05:07:50PM +0300, Gilad Ben-Yossef wrote:
> >> Arm TrustZone CryptoCell 700 is a family of cryptographic hardware
> >> accelerators. It is supported by a long lived series of out of tree
> >> drivers, which I am now in the process of unifying and upstreaming.
> >> This is the first drop, supporting the new CryptoCell 712 REE.
> >>
> >> The code still needs some cleanup before maturing to a proper
> >> upstream driver, which I am in the process of doing. However,
> >> as discussion of some of the capabilities of the hardware and
> >> its application to some dm-crypt and dm-verity features recently
> >> took place I though it is better to do this in the open via the
> >> staging tree.
> >>
> >> A Git repository based off of Linux 4.11-rc7 is available at
> >> https://github.com/gby/linux.git branch ccree.
> >>
> >> Signed-off-by: Gilad Ben-Yossef 
> >> CC: Binoy Jayan 
> >> CC: Ofir Drang 
> >>
> >> Gilad Ben-Yossef (4):
> >>   staging: add ccree crypto driver
> >>   staging: ccree: add TODO list
> >>   staging: ccree: add devicetree bindings
> >>   MAINTAINERS: add Gilad BY as maintainer for ccree
> >>
> >>  .../devicetree/bindings/crypto/arm-cryptocell.txt  |   23 +
> >
> > I'm interested in looking at the DT binding, but for some reason I've
> > only recevied the cover letter so far.
> >
> > Are my mail servers being particularly slow today, or has something gone
> > wrong with the Cc list for the remaining patches?
> >
> 
> Thanks a bunch for the willingness to review this.
> 
> My apologies for not communicating this clearly enough -  Since it is
> an pre-existing driver it is rather big.
> I did not want to inflict a 600K patch on the mailing list so opted to
> provide a git repo instead, as stated in the
> email.

Should this have been a [GIT PULL], then?

I was confused by the [PATCH 0/4].

> The patch in question is here:
> https://github.com/gby/linux/commit/12d92e0bc19aa9bbd891cdab765eaaeabe0b6967
> 
> Do let me know if you prefer that I will send at least the smaller
> patch file via email in the normal fashion.

Sending patches is the usual way to have things reviewed. Linking to an
external site doesn't fit with the workflows of everyone.

If you wish to have patches reviewed, please send them as patches in the
usual fashion.

I will note that on the patch you linked, the compatible string is far
too vague, per common conventions. Per your description above, that
should be something like "arm,cryptocell-712-ree" (and each variant
should have its own string).

I don't have documentation to hand to attempt to review the rest.

I would appreciate if you could ensure that the DT binding was reviewed
as part of the staging TODOs. That needs to follow the usual DT review
process of sending patches to the list. Until that has occurred, it
shouldn't be in Documentation/devicetree/.

Thanks,
Mark.

Re: [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function

[Logan Gunthorpe]

On 18/04/17 08:27 AM, Konrad Rzeszutek Wilk wrote:
> Interesting that you didn't CC any of the maintainers. Could you 
> do that in the future please?

Please read the cover letter. The distribution list for the patchset
would have been way too large to cc every maintainer (even as limited as
it was, I had mailing lists yelling at me). My plan was to get buy in
for the first patch, get it merged and resend the rest independently to
their respective maintainers. Of course, though, I'd be open to other
suggestions.

>>>
>>> Signed-off-by: Logan Gunthorpe 
>>> ---
>>>  drivers/block/xen-blkfront.c | 33 +++--
>>>  1 file changed, 27 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
>>> index 5067a0a..7dcf41d 100644
>>> --- a/drivers/block/xen-blkfront.c
>>> +++ b/drivers/block/xen-blkfront.c
>>> @@ -807,8 +807,19 @@ static int blkif_queue_rw_req(struct request *req, 
>>> struct blkfront_ring_info *ri
>>> BUG_ON(sg->offset + sg->length > PAGE_SIZE);
>>>
>>> if (setup.need_copy) {
>>> -   setup.bvec_off = sg->offset;
>>> -   setup.bvec_data = kmap_atomic(sg_page(sg));
>>> +   setup.bvec_off = 0;
>>> +   setup.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
>>> +   if (IS_ERR(setup.bvec_data)) {
>>> +   /*
>>> +* This should really never happen unless
>>> +* the code is changed to use memory that is
>>> +* not mappable in the sg. Seeing there is a
>>> +* questionable error path out of here,
>>> +* we WARN.
>>> +*/
>>> +   WARN(1, "Non-mappable memory used in sg!");
>>> +   return 1;
>>> +   }
>> ...
>>
>> Perhaps add a flag to mark failure as 'unexpected' and trace (and panic?)
>> inside sg_map().

Thanks, that's a good suggestion. I'll make the change for v2.

Logan

Re: [PATCH 0/4] staging: add ccree crypto driver

[Greg Kroah-Hartman]

On Tue, Apr 18, 2017 at 05:07:50PM +0300, Gilad Ben-Yossef wrote:
> Arm TrustZone CryptoCell 700 is a family of cryptographic hardware
> accelerators. It is supported by a long lived series of out of tree
> drivers, which I am now in the process of unifying and upstreaming.
> This is the first drop, supporting the new CryptoCell 712 REE. 
> 
> The code still needs some cleanup before maturing to a proper
> upstream driver, which I am in the process of doing. However,
> as discussion of some of the capabilities of the hardware and
> its application to some dm-crypt and dm-verity features recently
> took place I though it is better to do this in the open via the
> staging tree.
> 
> A Git repository based off of Linux 4.11-rc7 is available at
> https://github.com/gby/linux.git branch ccree.
> 
> Signed-off-by: Gilad Ben-Yossef 
> CC: Binoy Jayan 
> CC: Ofir Drang 
> 
> Gilad Ben-Yossef (4):
>   staging: add ccree crypto driver
>   staging: ccree: add TODO list
>   staging: ccree: add devicetree bindings
>   MAINTAINERS: add Gilad BY as maintainer for ccree

We can't do much without a real patch, sorry.  Digging in random git
trees doesn't work :(

I can't take this as-is, I need patches.

thanks,

greg k-h

Re: [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT

[Ard Biesheuvel]

On 18 April 2017 at 15:47, Paul Gortmaker  wrote:
> On Wed, Apr 5, 2017 at 2:34 PM, Matthias Kaehlcke  wrote:
>> The operand is an integer constant, make the constness explicit by
>> adding the modifier. This is needed for clang to generate valid code
>> and also works with gcc.
>
> Actually it doesn't work with all gcc.  I've got an older arm64 toolchain 
> that I
> only use for syntax checking (and hence I don't care if it is the latest and
> greatest) and this commit breaks it:
>
> arch/arm64/crypto/sha1-ce-glue.c:21:2: error: invalid 'asm': invalid
> operand prefix '%c'
>   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
>
> I'm currently reverting this change locally so I can continue to use the old
> toolchain:
>
> $ aarch64-linux-gnu-gcc --version
> aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro
> GCC 2013.11) 4.8.3 20131202 (prerelease)
> Copyright (C) 2013 Free Software Foundation, Inc.
>
> $ aarch64-linux-gnu-as --version
> GNU assembler (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro GCC
> 2013.11) 2.24.0.20131220
> Copyright 2013 Free Software Foundation, Inc.
>
> Maybe it is finally too old and nobody cares, but I thought it worth a 
> mention.
>

Thanks for the report. I think we care more about GCC 4.8 than about
Clang, which argues for reverting this patch.

I understand these issues must be frustrating if you are working on
this stuff, but to me, it is not entirely obvious why we want to
support Clang in the first place (i.e., what does it buy you if your
distro/environment is not already using Clang for userland), and why
the burden is on Linux to make modifications to support Clang,
especially when it comes to GCC extensions such as inline assembly
syntax.

It is ultimately up to the maintainers to decide what to do with this
patch, but my vote would be to revert it, especially given that the %c
placeholder prefix is not documented anywhere, and appears to simply
trigger some GCC internals that happen to do the right thing in this
case.

However, the I -> i change is arguably an improvement, and considering
that the following

asm("foo: .long %0" :: "i"(some value))

doesn't compile with clang either, I suggest you (Matthias) file a bug
against Clang to get this fixed, and we can propose another patch just
for the I->i change.

-- 
Ard.


>>
>> Also change the constraint of the operand from 'I' ("Integer constant
>> that is valid as an immediate operand in an ADD instruction", AArch64)
>> to 'i' ("An immediate integer operand").
>>
>> Based-on-patch-from: Greg Hackmann 
>> Signed-off-by: Greg Hackmann 
>> Signed-off-by: Matthias Kaehlcke 
>> ---
>> Changes in v2:
>> - Changed operand constraint from I to i
>> - Updated commit message
>> - Changed 'From' tag to 'Based-on-patch-from'
>>
>>  arch/arm64/crypto/sha1-ce-glue.c | 2 +-
>>  arch/arm64/crypto/sha2-ce-glue.c | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/crypto/sha1-ce-glue.c 
>> b/arch/arm64/crypto/sha1-ce-glue.c
>> index aefda9868627..6b520e3f3ab1 100644
>> --- a/arch/arm64/crypto/sha1-ce-glue.c
>> +++ b/arch/arm64/crypto/sha1-ce-glue.c
>> @@ -18,7 +18,7 @@
>>  #include 
>>
>>  #define ASM_EXPORT(sym, val) \
>> -   asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
>> +   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
>>
>>  MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
>>  MODULE_AUTHOR("Ard Biesheuvel ");
>> diff --git a/arch/arm64/crypto/sha2-ce-glue.c 
>> b/arch/arm64/crypto/sha2-ce-glue.c
>> index 7cd587564a41..e3abe11de48c 100644
>> --- a/arch/arm64/crypto/sha2-ce-glue.c
>> +++ b/arch/arm64/crypto/sha2-ce-glue.c
>> @@ -18,7 +18,7 @@
>>  #include 
>>
>>  #define ASM_EXPORT(sym, val) \
>> -   asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
>> +   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
>>
>>  MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto 
>> Extensions");
>>  MODULE_AUTHOR("Ard Biesheuvel ");
>> --
>> 2.12.2.715.g7642488e1d-goog
>>

Re: [PATCH 0/4] staging: add ccree crypto driver

[Gilad Ben-Yossef]

Hi Mark,

On Tue, Apr 18, 2017 at 6:13 PM, Mark Rutland  wrote:
> Hi,
>
> On Tue, Apr 18, 2017 at 05:07:50PM +0300, Gilad Ben-Yossef wrote:
>> Arm TrustZone CryptoCell 700 is a family of cryptographic hardware
>> accelerators. It is supported by a long lived series of out of tree
>> drivers, which I am now in the process of unifying and upstreaming.
>> This is the first drop, supporting the new CryptoCell 712 REE.
>>
>> The code still needs some cleanup before maturing to a proper
>> upstream driver, which I am in the process of doing. However,
>> as discussion of some of the capabilities of the hardware and
>> its application to some dm-crypt and dm-verity features recently
>> took place I though it is better to do this in the open via the
>> staging tree.
>>
>> A Git repository based off of Linux 4.11-rc7 is available at
>> https://github.com/gby/linux.git branch ccree.
>>
>> Signed-off-by: Gilad Ben-Yossef 
>> CC: Binoy Jayan 
>> CC: Ofir Drang 
>>
>> Gilad Ben-Yossef (4):
>>   staging: add ccree crypto driver
>>   staging: ccree: add TODO list
>>   staging: ccree: add devicetree bindings
>>   MAINTAINERS: add Gilad BY as maintainer for ccree
>>
>>  .../devicetree/bindings/crypto/arm-cryptocell.txt  |   23 +
>
> I'm interested in looking at the DT binding, but for some reason I've
> only recevied the cover letter so far.
>
> Are my mail servers being particularly slow today, or has something gone
> wrong with the Cc list for the remaining patches?
>

Thanks a bunch for the willingness to review this.

My apologies for not communicating this clearly enough -  Since it is
an pre-existing driver it is rather big.
I did not want to inflict a 600K patch on the mailing list so opted to
provide a git repo instead, as stated in the
email.

The patch in question is here:
https://github.com/gby/linux/commit/12d92e0bc19aa9bbd891cdab765eaaeabe0b6967

Do let me know if you prefer that I will send at least the smaller
patch file via email in the normal fashion.

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

Re: [PATCH 0/4] staging: add ccree crypto driver

[Mark Rutland]

Hi,

On Tue, Apr 18, 2017 at 05:07:50PM +0300, Gilad Ben-Yossef wrote:
> Arm TrustZone CryptoCell 700 is a family of cryptographic hardware
> accelerators. It is supported by a long lived series of out of tree
> drivers, which I am now in the process of unifying and upstreaming.
> This is the first drop, supporting the new CryptoCell 712 REE. 
> 
> The code still needs some cleanup before maturing to a proper
> upstream driver, which I am in the process of doing. However,
> as discussion of some of the capabilities of the hardware and
> its application to some dm-crypt and dm-verity features recently
> took place I though it is better to do this in the open via the
> staging tree.
> 
> A Git repository based off of Linux 4.11-rc7 is available at
> https://github.com/gby/linux.git branch ccree.
> 
> Signed-off-by: Gilad Ben-Yossef 
> CC: Binoy Jayan 
> CC: Ofir Drang 
> 
> Gilad Ben-Yossef (4):
>   staging: add ccree crypto driver
>   staging: ccree: add TODO list
>   staging: ccree: add devicetree bindings
>   MAINTAINERS: add Gilad BY as maintainer for ccree
> 
>  .../devicetree/bindings/crypto/arm-cryptocell.txt  |   23 +

I'm interested in looking at the DT binding, but for some reason I've
only recevied the cover letter so far.

Are my mail servers being particularly slow today, or has something gone
wrong with the Cc list for the remaining patches?

Thanks,
Mark.

Re: [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT

[Paul Gortmaker]

On Wed, Apr 5, 2017 at 2:34 PM, Matthias Kaehlcke  wrote:
> The operand is an integer constant, make the constness explicit by
> adding the modifier. This is needed for clang to generate valid code
> and also works with gcc.

Actually it doesn't work with all gcc.  I've got an older arm64 toolchain that I
only use for syntax checking (and hence I don't care if it is the latest and
greatest) and this commit breaks it:

arch/arm64/crypto/sha1-ce-glue.c:21:2: error: invalid 'asm': invalid
operand prefix '%c'
  asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));

I'm currently reverting this change locally so I can continue to use the old
toolchain:

$ aarch64-linux-gnu-gcc --version
aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro
GCC 2013.11) 4.8.3 20131202 (prerelease)
Copyright (C) 2013 Free Software Foundation, Inc.

$ aarch64-linux-gnu-as --version
GNU assembler (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro GCC
2013.11) 2.24.0.20131220
Copyright 2013 Free Software Foundation, Inc.

Maybe it is finally too old and nobody cares, but I thought it worth a mention.

Paul.
--

>
> Also change the constraint of the operand from 'I' ("Integer constant
> that is valid as an immediate operand in an ADD instruction", AArch64)
> to 'i' ("An immediate integer operand").
>
> Based-on-patch-from: Greg Hackmann 
> Signed-off-by: Greg Hackmann 
> Signed-off-by: Matthias Kaehlcke 
> ---
> Changes in v2:
> - Changed operand constraint from I to i
> - Updated commit message
> - Changed 'From' tag to 'Based-on-patch-from'
>
>  arch/arm64/crypto/sha1-ce-glue.c | 2 +-
>  arch/arm64/crypto/sha2-ce-glue.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/crypto/sha1-ce-glue.c 
> b/arch/arm64/crypto/sha1-ce-glue.c
> index aefda9868627..6b520e3f3ab1 100644
> --- a/arch/arm64/crypto/sha1-ce-glue.c
> +++ b/arch/arm64/crypto/sha1-ce-glue.c
> @@ -18,7 +18,7 @@
>  #include 
>
>  #define ASM_EXPORT(sym, val) \
> -   asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
> +   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
>
>  MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
>  MODULE_AUTHOR("Ard Biesheuvel ");
> diff --git a/arch/arm64/crypto/sha2-ce-glue.c 
> b/arch/arm64/crypto/sha2-ce-glue.c
> index 7cd587564a41..e3abe11de48c 100644
> --- a/arch/arm64/crypto/sha2-ce-glue.c
> +++ b/arch/arm64/crypto/sha2-ce-glue.c
> @@ -18,7 +18,7 @@
>  #include 
>
>  #define ASM_EXPORT(sym, val) \
> -   asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
> +   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
>
>  MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto 
> Extensions");
>  MODULE_AUTHOR("Ard Biesheuvel ");
> --
> 2.12.2.715.g7642488e1d-goog
>

Re: [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function

[Konrad Rzeszutek Wilk]

On Tue, Apr 18, 2017 at 02:13:59PM +, David Laight wrote:
> From: Logan Gunthorpe
> > Sent: 13 April 2017 23:05
> > Straightforward conversion to the new helper, except due to
> > the lack of error path, we have to warn if unmapable memory
> > is ever present in the sgl.

Interesting that you didn't CC any of the maintainers. Could you 
do that in the future please?

> > 
> > Signed-off-by: Logan Gunthorpe 
> > ---
> >  drivers/block/xen-blkfront.c | 33 +++--
> >  1 file changed, 27 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> > index 5067a0a..7dcf41d 100644
> > --- a/drivers/block/xen-blkfront.c
> > +++ b/drivers/block/xen-blkfront.c
> > @@ -807,8 +807,19 @@ static int blkif_queue_rw_req(struct request *req, 
> > struct blkfront_ring_info *ri
> > BUG_ON(sg->offset + sg->length > PAGE_SIZE);
> > 
> > if (setup.need_copy) {
> > -   setup.bvec_off = sg->offset;
> > -   setup.bvec_data = kmap_atomic(sg_page(sg));
> > +   setup.bvec_off = 0;
> > +   setup.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
> > +   if (IS_ERR(setup.bvec_data)) {
> > +   /*
> > +* This should really never happen unless
> > +* the code is changed to use memory that is
> > +* not mappable in the sg. Seeing there is a
> > +* questionable error path out of here,
> > +* we WARN.
> > +*/
> > +   WARN(1, "Non-mappable memory used in sg!");
> > +   return 1;
> > +   }
> ...
> 
> Perhaps add a flag to mark failure as 'unexpected' and trace (and panic?)
> inside sg_map().
> 
>   David
> 
> 
> ___
> Linux-nvdimm mailing list
> linux-nvd...@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

RE: [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function

[David Laight]

From: Logan Gunthorpe
> Sent: 13 April 2017 23:05
> Straightforward conversion to the new helper, except due to
> the lack of error path, we have to warn if unmapable memory
> is ever present in the sgl.
> 
> Signed-off-by: Logan Gunthorpe 
> ---
>  drivers/block/xen-blkfront.c | 33 +++--
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 5067a0a..7dcf41d 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -807,8 +807,19 @@ static int blkif_queue_rw_req(struct request *req, 
> struct blkfront_ring_info *ri
>   BUG_ON(sg->offset + sg->length > PAGE_SIZE);
> 
>   if (setup.need_copy) {
> - setup.bvec_off = sg->offset;
> - setup.bvec_data = kmap_atomic(sg_page(sg));
> + setup.bvec_off = 0;
> + setup.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
> + if (IS_ERR(setup.bvec_data)) {
> + /*
> +  * This should really never happen unless
> +  * the code is changed to use memory that is
> +  * not mappable in the sg. Seeing there is a
> +  * questionable error path out of here,
> +  * we WARN.
> +  */
> + WARN(1, "Non-mappable memory used in sg!");
> + return 1;
> + }
...

Perhaps add a flag to mark failure as 'unexpected' and trace (and panic?)
inside sg_map().

David

[PATCH 0/4] staging: add ccree crypto driver

[Gilad Ben-Yossef]

Arm TrustZone CryptoCell 700 is a family of cryptographic hardware
accelerators. It is supported by a long lived series of out of tree
drivers, which I am now in the process of unifying and upstreaming.
This is the first drop, supporting the new CryptoCell 712 REE. 

The code still needs some cleanup before maturing to a proper
upstream driver, which I am in the process of doing. However,
as discussion of some of the capabilities of the hardware and
its application to some dm-crypt and dm-verity features recently
took place I though it is better to do this in the open via the
staging tree.

A Git repository based off of Linux 4.11-rc7 is available at
https://github.com/gby/linux.git branch ccree.

Signed-off-by: Gilad Ben-Yossef 
CC: Binoy Jayan 
CC: Ofir Drang 

Gilad Ben-Yossef (4):
  staging: add ccree crypto driver
  staging: ccree: add TODO list
  staging: ccree: add devicetree bindings
  MAINTAINERS: add Gilad BY as maintainer for ccree

 .../devicetree/bindings/crypto/arm-cryptocell.txt  |   23 +
 MAINTAINERS|8 +
 drivers/staging/Kconfig|2 +
 drivers/staging/Makefile   |2 +-
 drivers/staging/ccree/Kconfig  |   52 +
 drivers/staging/ccree/Makefile |3 +
 drivers/staging/ccree/TODO |   27 +
 drivers/staging/ccree/bsp.h|   21 +
 drivers/staging/ccree/cc_bitops.h  |   62 +
 drivers/staging/ccree/cc_crypto_ctx.h  |  299 +++
 drivers/staging/ccree/cc_hal.h |   35 +
 drivers/staging/ccree/cc_hw_queue_defs.h   |  603 +
 drivers/staging/ccree/cc_lli_defs.h|   57 +
 drivers/staging/ccree/cc_pal_log.h |  188 ++
 drivers/staging/ccree/cc_pal_log_plat.h|   33 +
 drivers/staging/ccree/cc_pal_types.h   |   97 +
 drivers/staging/ccree/cc_pal_types_plat.h  |   29 +
 drivers/staging/ccree/cc_regs.h|  106 +
 drivers/staging/ccree/dx_crys_kernel.h |  180 ++
 drivers/staging/ccree/dx_env.h |  224 ++
 drivers/staging/ccree/dx_host.h|  155 ++
 drivers/staging/ccree/dx_reg_base_host.h   |   34 +
 drivers/staging/ccree/dx_reg_common.h  |   26 +
 drivers/staging/ccree/hash_defs.h  |   78 +
 drivers/staging/ccree/hw_queue_defs_plat.h |   43 +
 drivers/staging/ccree/ssi_aead.c   | 2832 
 drivers/staging/ccree/ssi_aead.h   |  120 +
 drivers/staging/ccree/ssi_buffer_mgr.c | 1876 +
 drivers/staging/ccree/ssi_buffer_mgr.h |  105 +
 drivers/staging/ccree/ssi_cipher.c | 1503 +++
 drivers/staging/ccree/ssi_cipher.h |   89 +
 drivers/staging/ccree/ssi_config.h |   61 +
 drivers/staging/ccree/ssi_driver.c |  557 
 drivers/staging/ccree/ssi_driver.h |  228 ++
 drivers/staging/ccree/ssi_fips.c   |   65 +
 drivers/staging/ccree/ssi_fips.h   |   70 +
 drivers/staging/ccree/ssi_fips_data.h  |  315 +++
 drivers/staging/ccree/ssi_fips_ext.c   |   96 +
 drivers/staging/ccree/ssi_fips_ll.c| 1681 
 drivers/staging/ccree/ssi_fips_local.c |  369 +++
 drivers/staging/ccree/ssi_fips_local.h |   77 +
 drivers/staging/ccree/ssi_hash.c   | 2751 +++
 drivers/staging/ccree/ssi_hash.h   |  101 +
 drivers/staging/ccree/ssi_ivgen.c  |  301 +++
 drivers/staging/ccree/ssi_ivgen.h  |   72 +
 drivers/staging/ccree/ssi_pm.c |  150 ++
 drivers/staging/ccree/ssi_pm.h |   46 +
 drivers/staging/ccree/ssi_pm_ext.c |   60 +
 drivers/staging/ccree/ssi_pm_ext.h |   33 +
 drivers/staging/ccree/ssi_request_mgr.c|  713 +
 drivers/staging/ccree/ssi_request_mgr.h|   60 +
 drivers/staging/ccree/ssi_sram_mgr.c   |  138 +
 drivers/staging/ccree/ssi_sram_mgr.h   |   80 +
 drivers/staging/ccree/ssi_sysfs.c  |  440 +++
 drivers/staging/ccree/ssi_sysfs.h  |   54 +
 55 files changed, 17429 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
 create mode 100644 drivers/staging/ccree/Kconfig
 create mode 100644 drivers/staging/ccree/Makefile
 create mode 100644 drivers/staging/ccree/TODO
 create mode 100644 drivers/staging/ccree/bsp.h
 create mode 100644 drivers/staging/ccree/cc_bitops.h
 create mode 100644 drivers/staging/ccree/cc_crypto_ctx.h
 create mode 100644 drivers/staging/ccree/cc_hal.h
 create mode 100644 

Re: [PATCH] padata: allow caller to control queue length

[Herbert Xu]

Jason A. Donenfeld  wrote:
> On Fri, Apr 14, 2017 at 9:57 AM, Steffen Klassert
>  wrote:
>> Why do we need this? As long as we don't have a user that needs a
>> different limit, this patch adds just some useless code.
> 
> My [not-yet-mainlined] code wants it.

Well then you should submit this along with your code that uses
it.

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

Crypto Fixes for 4.11

[Herbert Xu]

Hi Linus:

This push fixes the following problems:

- Regression in new XTS/LRW code when used with async crypto.
- Long-standing bug in ahash API when used with certain algos.
- Bogus memory dereference in async algif_aead with certain algos.


Please pull from

git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git linus


Herbert Xu (4):
  crypto: xts - Fix use-after-free on EINPROGRESS
  crypto: lrw - Fix use-after-free on EINPROGRESS
  crypto: ahash - Fix EINPROGRESS notification callback
  crypto: algif_aead - Fix bogus request dereference in completion function

 crypto/ahash.c |   79 +---
 crypto/algif_aead.c|   12 +++---
 crypto/lrw.c   |   16 
 crypto/xts.c   |   16 
 include/crypto/internal/hash.h |   10 +
 5 files changed, 98 insertions(+), 35 deletions(-)

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

Re: [RFC PATCH v1 1/1] crypto: algif_compression - User-space interface for compression

[Stephan Müller]

Am Donnerstag, 13. April 2017, 20:34:54 CEST schrieb Abed Kamaluddin:

Hi Abed,

> crypto: algif_compression - User-space interface for compression
> 
> This patch adds af_alg plugin for compression algorithms of type scomp/acomp
> registered to the kernel crypto layer.
> 
> The user needs to set operation (compression/decompression) as a control
> message to sendmsg, identical to selecting the cipher operation type in case
> of ciphers. Once a sendmsg call occurs, no further writes can be made to
> the socket until all previous data has been processed and read. Therefore
> the interface only supports one request at a time.
> 
> The interface is completely synchronous; all operations are carried out in
> recvmsg and will complete prior to the system call returning.
> 
> The sendmsg and recvmsg interface supports directly reading/writing to
> user-space without additional copying, i.e., the kernel crypto interface
> will receive the user-space address as its input/output SG list. The scomp
> interface or crypto drivers may copy the data as required.
> 
> Signed-off-by: Abed Kamaluddin 
> Signed-off-by: Mahipal Challa 
> 
> ---
>  crypto/Kconfig  |  11 ++
>  crypto/Makefile |   1 +
>  crypto/algif_compression.c  | 272
>  include/uapi/linux/if_alg.h | 
>  2 +
>  4 files changed, 286 insertions(+)
> 
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index f37e9cc..13b03ba 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1741,6 +1741,17 @@ config CRYPTO_USER_API_AEAD
> This option enables the user-spaces interface for AEAD
> cipher algorithms.
> 
> +config CRYPTO_USER_API_COMPRESSION
> + tristate "User-space interface for compression algorithms"
> + depends on NET
> + select CRYPTO_ACOMP
> + select CRYPTO_USER_API
> + help
> +   This option enables the user-space interface for compression
> +   algorithms. Enable this option for access to compression algorithms
> +   of type scomp/acomp exported by the kernel crypto layer through
> +   AF_ALG interface.
> +
>  config CRYPTO_HASH_INFO
>   bool
> 
> diff --git a/crypto/Makefile b/crypto/Makefile
> index 8a44057..1469e06 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -134,6 +134,7 @@ obj-$(CONFIG_CRYPTO_TEST) += tcrypt.o
>  obj-$(CONFIG_CRYPTO_GHASH) += ghash-generic.o
>  obj-$(CONFIG_CRYPTO_USER_API) += af_alg.o
>  obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
> +obj-$(CONFIG_CRYPTO_USER_API_COMPRESSION) += algif_compression.o
>  obj-$(CONFIG_CRYPTO_USER_API_SKCIPHER) += algif_skcipher.o
>  obj-$(CONFIG_CRYPTO_USER_API_RNG) += algif_rng.o
>  obj-$(CONFIG_CRYPTO_USER_API_AEAD) += algif_aead.o
> diff --git a/crypto/algif_compression.c b/crypto/algif_compression.c
> new file mode 100644
> index 000..0ba6d1e
> --- /dev/null
> +++ b/crypto/algif_compression.c
> @@ -0,0 +1,272 @@
> +/*
> + * algif_compression: User-space interface for COMPRESSION algorithms
> + *
> + * This file provides user-space API support for compression algorithms
> + * registered through the kernel crypto layer.
> + *
> + * Copyright (C) 2017 Cavium, Inc.
> + *
> + * Original Authors: Abed Kamaluddin 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> Free + * Software Foundation; either version 2 of the License, or (at your
> option) + * any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* scomp scratch is currently 128KB */
> +#define COMP_BUFFER_SIZE 65535
> +
> +struct comp_ctx {
> + struct af_alg_sgl tsgl;
> + struct af_alg_sgl rsgl;
> + struct af_alg_completion completion;
> + unsigned int clen;
> + unsigned int slen;
> + unsigned int dlen;
> + bool comp;
> + bool used;
> + struct acomp_req *acomp_req;
> +};
> +
> +struct comp_tfm {
> + struct crypto_acomp *acomp;
> +};
> +
> +static int comp_sendmsg(struct socket *sock, struct msghdr *msg,
> + size_t ignored)
> +{
> + struct sock *sk = sock->sk;
> + struct alg_sock *ask = alg_sk(sk);
> + struct comp_ctx *ctx = ask->private;
> + struct af_alg_control con = {};
> + int limit = COMP_BUFFER_SIZE;
> + int len;
> + int err = -EINVAL;
> +
> + if (msg->msg_controllen) {
> + err = af_alg_cmsg_send(msg, );
> + if (err)
> + return err;
> +
> + switch (con.op) {
> + case ALG_OP_COMPRESS:
> + ctx->comp = 1;
> + break;
> +
> + case ALG_OP_DECOMPRESS:
> + ctx->comp = 0;

You set the ctx without locking. I guess you want to move the ctx->comp 
setting below the lock.

> +  

Re: [PATCH 2/7] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

[Antoine Tenart]

Hi Robin,

On Wed, Apr 12, 2017 at 02:54:13PM +0100, Robin Murphy wrote:
> 
> Bit of a drive-by, but since I have it in my head that crypto drivers
> are a hotspot for dodgy DMA usage (in part due to the hardware often
> being a bit esoteric with embedded RAMs and such), this caught my eye
> and I thought I'd give this a quick once-over to check for anything
> smelly. Unfortunately, I was not disappointed... ;)

:-)

> On 29/03/17 13:44, Antoine Tenart wrote:
> [...]
> > diff --git a/drivers/crypto/inside-secure/safexcel.c 
> > b/drivers/crypto/inside-secure/safexcel.c
> > new file mode 100644
> > index ..00f3f2c85d05
> > --- /dev/null
> > +++ b/drivers/crypto/inside-secure/safexcel.c
> [...]
> > +int safexcel_invalidate_cache(struct crypto_async_request *async,
> > + struct safexcel_context *ctx,
> > + struct safexcel_crypto_priv *priv,
> > + dma_addr_t ctxr_dma, int ring,
> > + struct safexcel_request *request)
> > +{
> > +   struct safexcel_command_desc *cdesc;
> > +   struct safexcel_result_desc *rdesc;
> > +   phys_addr_t ctxr_phys;
> > +   int ret = 0;
> > +
> > +   ctxr_phys = dma_to_phys(priv->dev, ctxr_dma);
> 
> No no no. This is a SWIOTLB-specific (or otherwise arch-private,
> depending on implementation) helper, not a DMA API function, and should
> not be called directly by drivers. There is no guarantee it will give
> the result you expect, or even compile, in all cases (if the kbuild
> robot hasn't already revealed via the COMPILE_TEST dependency).
> 
> That said, AFAICS ctxr_phys ends up in a descriptor which is given to
> the device, so trying to use a physical address is wrong and it should
> still be the DMA address anyway.

I see. The cryptographic engine should always use dma addresses. I'll
update the descriptors structure members from "phys" to "dma" as well.

> [...]
> > +static int safexcel_probe(struct platform_device *pdev)
> > +{
> > +   struct device *dev = >dev;
> > +   struct resource *res;
> > +   struct safexcel_crypto_priv *priv;
> > +   int i, ret;
> > +
> > +   priv = devm_kzalloc(dev, sizeof(struct safexcel_crypto_priv),
> > +   GFP_KERNEL);
> > +   if (!priv)
> > +   return -ENOMEM;
> > +
> > +   priv->dev = dev;
> > +
> > +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   priv->base = devm_ioremap_resource(dev, res);
> > +   if (IS_ERR(priv->base)) {
> > +   dev_err(dev, "failed to get resource\n");
> > +   return PTR_ERR(priv->base);
> > +   }
> > +
> > +   priv->clk = of_clk_get(dev->of_node, 0);
> > +   if (!IS_ERR(priv->clk)) {
> > +   ret = clk_prepare_enable(priv->clk);
> > +   if (ret) {
> > +   dev_err(dev, "unable to enable clk (%d)\n", ret);
> > +   return ret;
> > +   }
> > +   } else {
> > +   /* The clock isn't mandatory */
> > +   if (PTR_ERR(priv->clk) == -EPROBE_DEFER)
> > +   return -EPROBE_DEFER;
> > +   }
> 
> You should call dma_set_mask_and_coherent() before any DMA API calls,
> both to confirm DMA really is going to work all, and also (since this IP
> apparently supports >32-bit addresses) to describe the full inherent
> addressing capability, not least to avoid wasting time/space with
> unnecessary bounce buffering otherwise.

OK, I'll add a call to this helper before using DMA API calls.

> > +
> > +err_pool:
> > +   dma_pool_destroy(priv->context_pool);
> 
> You used dmam_pool_create(), so not only is an explicit destroy
> unnecessary, but doing so with the non-managed version is actively
> harmful, since devres would end up double-freeing the pool after you return.

I saw this and fixed it in my local version.

> > +struct safexcel_token {
> > +   u32 packet_length:17;
> > +   u8 stat:2;
> > +   u16 instructions:9;
> > +   u8 opcode:4;
> > +} __packed;
> 
> Using bitfields in hardware descriptors seems pretty risky, since you've
> no real guarantee two compilers will lay them out the same way (or that
> either would be correct WRT what the hardware expects). I'd be more
> inclined to construct all these fields with explicit masking and shifting.

Hmm, I'm not sure to follow you here. If I use bitfields in a __packed
structure, we should be safe. Why would the compiler have a different
behaviour?

> > +static int safexcel_aes_send(struct crypto_async_request *async,
> > +int ring, struct safexcel_request *request,
> > +int *commands, int *results)
> > +{
> > +   struct ablkcipher_request *req = ablkcipher_request_cast(async);
> > +   struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
> > +   struct safexcel_crypto_priv *priv = ctx->priv;
> > +   struct safexcel_command_desc *cdesc;
> > +   struct safexcel_result_desc *rdesc;
> > +   struct scatterlist *sg;
> > +   phys_addr_t ctxr_phys;
> > +   int nr_src, nr_dst, n_cdesc = 0, 

Re: [PATCH 4/7] arm64: marvell: dts: add crypto engine description for 7k/8k

[Antoine Tenart]

Hi Thomas,

On Wed, Apr 12, 2017 at 10:56:08AM +0200, Thomas Petazzoni wrote:
> On Wed, 29 Mar 2017 14:44:29 +0200, Antoine Tenart wrote:
> 
> > +   cpm_crypto: crypto@80 {
> > +   compatible = "inside-secure,safexcel-eip197";
> > +   reg = <0x80 0x20>;
> > +   interrupts =  > | IRQ_TYPE_LEVEL_HIGH)>,
> 
> Now that I look into this, does it makes sense for an interrupt to be
> both an edge interrupt and a level interrupt at the same time? This
> looks odd.

I agree this looks odd. I took it from Russel's ICU mapping:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/489040.html

Also note the driver does not use it (yet?).

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH 05/22] drm/i915: Make use of the new sg_map helper function

[Daniel Vetter]

On Thu, Apr 13, 2017 at 04:05:18PM -0600, Logan Gunthorpe wrote:
> This is a single straightforward conversion from kmap to sg_map.
> 
> Signed-off-by: Logan Gunthorpe 

Acked-by: Daniel Vetter 

Probably makes sense to merge through some other tree, but please be aware
of the considerable churn rate in i915 (i.e. make sure your tree is in
linux-next before you send a pull request for this). Plane B would be to
get the prep patch in first and then merge the i915 conversion one kernel
release later.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 27 ---
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 67b1fc5..1b1b91a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2188,6 +2188,15 @@ static void __i915_gem_object_reset_page_iter(struct 
> drm_i915_gem_object *obj)
>   radix_tree_delete(>mm.get_page.radix, iter.index);
>  }
>  
> +static void i915_gem_object_unmap(const struct drm_i915_gem_object *obj,
> +   void *ptr)
> +{
> + if (is_vmalloc_addr(ptr))
> + vunmap(ptr);
> + else
> + sg_unmap(obj->mm.pages->sgl, ptr, SG_KMAP);
> +}
> +
>  void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
>enum i915_mm_subclass subclass)
>  {
> @@ -2215,10 +2224,7 @@ void __i915_gem_object_put_pages(struct 
> drm_i915_gem_object *obj,
>   void *ptr;
>  
>   ptr = ptr_mask_bits(obj->mm.mapping);
> - if (is_vmalloc_addr(ptr))
> - vunmap(ptr);
> - else
> - kunmap(kmap_to_page(ptr));
> + i915_gem_object_unmap(obj, ptr);
>  
>   obj->mm.mapping = NULL;
>   }
> @@ -2475,8 +2481,11 @@ static void *i915_gem_object_map(const struct 
> drm_i915_gem_object *obj,
>   void *addr;
>  
>   /* A single page can always be kmapped */
> - if (n_pages == 1 && type == I915_MAP_WB)
> - return kmap(sg_page(sgt->sgl));
> + if (n_pages == 1 && type == I915_MAP_WB) {
> + addr = sg_map(sgt->sgl, SG_KMAP);
> + if (IS_ERR(addr))
> + return NULL;
> + }
>  
>   if (n_pages > ARRAY_SIZE(stack_pages)) {
>   /* Too big for stack -- allocate temporary array instead */
> @@ -2543,11 +2552,7 @@ void *i915_gem_object_pin_map(struct 
> drm_i915_gem_object *obj,
>   goto err_unpin;
>   }
>  
> - if (is_vmalloc_addr(ptr))
> - vunmap(ptr);
> - else
> - kunmap(kmap_to_page(ptr));
> -
> + i915_gem_object_unmap(obj, ptr);
>   ptr = obj->mm.mapping = NULL;
>   }
>  
> -- 
> 2.1.4
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch