Re: [PATCH] ccree: Removed a sparse warning in ssi_fips.h

2017-07-30 Thread Gilad Ben-Yossef 
Hi Rishabh,

Thanks you for spotting this.

On Sun, Jul 30, 2017 at 5:15 PM, rishabh hardas
 wrote:
> Sparse was giving out a warning 'warning: symbol 'cc_set_ree_fips_status' was 
> not declared. Should it be static?'
> which has been removed by making it static.
>
> Signed-off-by: rishabh hardas 
> ---
>  drivers/staging/ccree/ssi_fips.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/ccree/ssi_fips.h 
> b/drivers/staging/ccree/ssi_fips.h
> index 369ddf9..846bd55 100644
> --- a/drivers/staging/ccree/ssi_fips.h
> +++ b/drivers/staging/ccree/ssi_fips.h
> @@ -40,7 +40,7 @@ static inline int ssi_fips_init(struct ssi_drvdata 
> *p_drvdata)
>  }
>
>  static inline void ssi_fips_fini(struct ssi_drvdata *drvdata) {}
> -void cc_set_ree_fips_status(struct ssi_drvdata *drvdata, bool ok) {}
> +static void cc_set_ree_fips_status(struct ssi_drvdata *drvdata, bool ok) {}
>  void fips_handler(struct ssi_drvdata *drvdata) {}
>
>  #endif /* CONFIG_CRYPTO_FIPS */
> --
> 2.7.4
>

A better modifier here would be "static inline". While you're at it
you can also apply it to the fips_handler() function below.

Also, please use a descriptive title, maybe: "declare compiled out
func  static inline"

Last, please CC ALL the appropriate mailing lists. Use the kernel
get_maintainer script to find out which if you are unsure.

Thanks again,
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

[PATCH] staging: ccree: Use sizeof(variable) in memory allocs

2017-07-30 Thread Simon Sandström 
Fixes 9 checkpatch.pl warnings of type
"Prefer kmalloc(sizeof(variable)...) over kmalloc(sizeof(type)...)"
in staging/ccree.

Signed-off-by: Simon Sandström 
---
 drivers/staging/ccree/ssi_aead.c| 4 ++--
 drivers/staging/ccree/ssi_cipher.c  | 5 ++---
 drivers/staging/ccree/ssi_driver.c  | 3 ++-
 drivers/staging/ccree/ssi_hash.c| 8 
 drivers/staging/ccree/ssi_ivgen.c   | 5 +++--
 drivers/staging/ccree/ssi_request_mgr.c | 2 +-
 6 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c
index f5ca0e35c5d3..5abe6b24ff8c 100644
--- a/drivers/staging/ccree/ssi_aead.c
+++ b/drivers/staging/ccree/ssi_aead.c
@@ -2659,7 +2659,7 @@ static struct ssi_crypto_alg *ssi_aead_create_alg(struct 
ssi_alg_template *templ
struct ssi_crypto_alg *t_alg;
struct aead_alg *alg;
 
-   t_alg = kzalloc(sizeof(struct ssi_crypto_alg), GFP_KERNEL);
+   t_alg = kzalloc(sizeof(*t_alg), GFP_KERNEL);
if (!t_alg) {
SSI_LOG_ERR("failed to allocate t_alg\n");
return ERR_PTR(-ENOMEM);
@@ -2714,7 +2714,7 @@ int ssi_aead_alloc(struct ssi_drvdata *drvdata)
int rc = -ENOMEM;
int alg;
 
-   aead_handle = kmalloc(sizeof(struct ssi_aead_handle), GFP_KERNEL);
+   aead_handle = kmalloc(sizeof(*aead_handle), GFP_KERNEL);
if (!aead_handle) {
rc = -ENOMEM;
goto fail0;
diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index 6219a92184aa..af9afead7bcb 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -1218,7 +1218,7 @@ struct ssi_crypto_alg *ssi_ablkcipher_create_alg(struct 
ssi_alg_template *templa
struct ssi_crypto_alg *t_alg;
struct crypto_alg *alg;
 
-   t_alg = kzalloc(sizeof(struct ssi_crypto_alg), GFP_KERNEL);
+   t_alg = kzalloc(sizeof(*t_alg), GFP_KERNEL);
if (!t_alg) {
SSI_LOG_ERR("failed to allocate t_alg\n");
return ERR_PTR(-ENOMEM);
@@ -1279,8 +1279,7 @@ int ssi_ablkcipher_alloc(struct ssi_drvdata *drvdata)
int rc = -ENOMEM;
int alg;
 
-   ablkcipher_handle = kmalloc(sizeof(struct ssi_blkcipher_handle),
-   GFP_KERNEL);
+   ablkcipher_handle = kmalloc(sizeof(*ablkcipher_handle), GFP_KERNEL);
if (!ablkcipher_handle)
return -ENOMEM;
 
diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index a4ab9ef4baf7..88b68c81556c 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -223,7 +223,8 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
struct resource *req_mem_cc_regs = NULL;
void __iomem *cc_base = NULL;
bool irq_registered = false;
-   struct ssi_drvdata *new_drvdata = kzalloc(sizeof(struct ssi_drvdata), 
GFP_KERNEL);
+   struct ssi_drvdata *new_drvdata = kzalloc(sizeof(*new_drvdata),
+ GFP_KERNEL);
struct device *dev = _dev->dev;
struct device_node *np = dev->of_node;
u32 signature_val;
diff --git a/drivers/staging/ccree/ssi_hash.c b/drivers/staging/ccree/ssi_hash.c
index 1a405bbadf6d..13291aeaf350 100644
--- a/drivers/staging/ccree/ssi_hash.c
+++ b/drivers/staging/ccree/ssi_hash.c
@@ -2060,9 +2060,9 @@ ssi_hash_create_alg(struct ssi_hash_template *template, 
bool keyed)
struct crypto_alg *alg;
struct ahash_alg *halg;
 
-   t_crypto_alg = kzalloc(sizeof(struct ssi_hash_alg), GFP_KERNEL);
+   t_crypto_alg = kzalloc(sizeof(*t_crypto_alg), GFP_KERNEL);
if (!t_crypto_alg) {
-   SSI_LOG_ERR("failed to allocate t_alg\n");
+   SSI_LOG_ERR("failed to allocate t_crypto_alg\n");
return ERR_PTR(-ENOMEM);
}
 
@@ -2226,10 +2226,10 @@ int ssi_hash_alloc(struct ssi_drvdata *drvdata)
int rc = 0;
int alg;
 
-   hash_handle = kzalloc(sizeof(struct ssi_hash_handle), GFP_KERNEL);
+   hash_handle = kzalloc(sizeof(*hash_handle), GFP_KERNEL);
if (!hash_handle) {
SSI_LOG_ERR("kzalloc failed to allocate %zu B\n",
-   sizeof(struct ssi_hash_handle));
+   sizeof(*hash_handle));
rc = -ENOMEM;
goto fail;
}
diff --git a/drivers/staging/ccree/ssi_ivgen.c 
b/drivers/staging/ccree/ssi_ivgen.c
index 86364f81acab..b01e03231947 100644
--- a/drivers/staging/ccree/ssi_ivgen.c
+++ b/drivers/staging/ccree/ssi_ivgen.c
@@ -191,10 +191,11 @@ int ssi_ivgen_init(struct ssi_drvdata *drvdata)
int rc;
 
/* Allocate "this" context */
-   drvdata->ivgen_handle = kzalloc(sizeof(struct ssi_ivgen_ctx), 
GFP_KERNEL);
+   drvdata->ivgen_handle = kzalloc(sizeof(*drvdata->ivgen_handle),
+

[PATCH] crypto: AF_ALG - return error code when no data was processed

2017-07-30 Thread Stephan Müller 
If no data has been processed during recvmsg, return the error code.
This covers all errors received during non-AIO operations.

If any error occurs during a synchronous operation in addition to
-EIOCBQUEUED or -EBADMSG (like -ENOMEM), it should be relayed to the
caller.

Signed-off-by: Stephan Mueller 
---
 crypto/algif_aead.c | 4 +++-
 crypto/algif_skcipher.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 9755aac0fe26..2de056c3139c 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -791,9 +791,11 @@ static int aead_recvmsg(struct socket *sock, struct msghdr 
*msg,
 * only handle one AIO request. If the caller wants to have
 * multiple AIO requests in parallel, he must make multiple
 * separate AIO calls.
+*
+* Also return the error if no data has been processed so far.
 */
if (err <= 0) {
-   if (err == -EIOCBQUEUED || err == -EBADMSG)
+   if (err == -EIOCBQUEUED || err == -EBADMSG || !ret)
ret = err;
goto out;
}
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 968d094f0bcc..ce3b5fba2279 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -714,9 +714,11 @@ static int skcipher_recvmsg(struct socket *sock, struct 
msghdr *msg,
 * only handle one AIO request. If the caller wants to have
 * multiple AIO requests in parallel, he must make multiple
 * separate AIO calls.
+*
+* Also return the error if no data has been processed so far.
 */
if (err <= 0) {
-   if (err == -EIOCBQUEUED)
+   if (err == -EIOCBQUEUED || !ret)
ret = err;
goto out;
}
-- 
2.13.3

[PATCH] crypto: algif_aead - copy AAD from src to dst

2017-07-30 Thread Stephan Müller 
Use the NULL cipher to copy the AAD and PT/CT from the TX SGL
to the RX SGL. This allows an in-place crypto operation on the
RX SGL for encryption, because the TX data is always smaller or
equal to the RX data (the RX data will hold the tag).

For decryption, a per-request TX SGL is created which will only hold
the tag value. As the RX SGL will have no space for the tag value and
an in-place operation will not write the tag buffer, the TX SGL with the
tag value is chained to the RX SGL. This now allows an in-place
crypto operation.

For example:

* without the patch:
kcapi -x 2 -e -c "gcm(aes)" -p 89154d0d4129d322e4487bafaa4f6b46 -k 
c0ece3e63198af382b5603331cc23fa8 -i 7e489b83622e7228314d878d -a 
afcd7202d621e06ca53b70c2bdff7fb2 -l 16 -u -s
f4a3eacfbdadd3b1a17117b1d67ffc1f1e21efbbc6d83724a8c296e3bb8cda0c

* with the patch:
kcapi -x 2 -e -c "gcm(aes)" -p 89154d0d4129d322e4487bafaa4f6b46 -k 
c0ece3e63198af382b5603331cc23fa8 -i 7e489b83622e7228314d878d -a 
afcd7202d621e06ca53b70c2bdff7fb2 -l 16 -u -s
afcd7202d621e06ca53b70c2bdff7fb2f4a3eacfbdadd3b1a17117b1d67ffc1f1e21efbbc6d83724a8c296e3bb8cda0c

Tests covering this functionality have been added to libkcapi.

Signed-off-by: Stephan Mueller 
---
 crypto/Kconfig  |   2 +
 crypto/algif_aead.c | 183 +---
 2 files changed, 162 insertions(+), 23 deletions(-)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index caa770e535a2..0a121f9ddf8e 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1753,6 +1753,8 @@ config CRYPTO_USER_API_AEAD
tristate "User-space interface for AEAD cipher algorithms"
depends on NET
select CRYPTO_AEAD
+   select CRYPTO_BLKCIPHER
+   select CRYPTO_NULL
select CRYPTO_USER_API
help
  This option enables the user-spaces interface for AEAD
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 2de056c3139c..1f0696dd64f4 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -30,6 +30,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -70,6 +72,7 @@ struct aead_async_req {
 struct aead_tfm {
struct crypto_aead *aead;
bool has_key;
+   struct crypto_skcipher *null_tfm;
 };
 
 struct aead_ctx {
@@ -168,7 +171,12 @@ static int aead_alloc_tsgl(struct sock *sk)
return 0;
 }
 
-static unsigned int aead_count_tsgl(struct sock *sk, size_t bytes)
+/**
+ * Count number of SG entries from the beginning of the SGL to @bytes. If
+ * an offset is provided, the counting of the SG entries starts at the offset.
+ */
+static unsigned int aead_count_tsgl(struct sock *sk, size_t bytes,
+   size_t offset)
 {
struct alg_sock *ask = alg_sk(sk);
struct aead_ctx *ctx = ask->private;
@@ -183,32 +191,55 @@ static unsigned int aead_count_tsgl(struct sock *sk, 
size_t bytes)
struct scatterlist *sg = sgl->sg;
 
for (i = 0; i < sgl->cur; i++) {
+   size_t bytes_count;
+
+   /* Skip offset */
+   if (offset >= sg[i].length) {
+   offset -= sg[i].length;
+   bytes -= sg[i].length;
+   continue;
+   }
+
+   bytes_count = sg[i].length - offset;
+
+   offset = 0;
sgl_count++;
-   if (sg[i].length >= bytes)
+
+   /* If we have seen requested number of bytes, stop */
+   if (bytes_count >= bytes)
return sgl_count;
 
-   bytes -= sg[i].length;
+   bytes -= bytes_count;
}
}
 
return sgl_count;
 }
 
+/**
+ * Release the specified buffers from TX SGL pointed to by ctx->tsgl_list for
+ * @used bytes.
+ *
+ * If @dst is non-null, reassign the pages to dst. The caller must release
+ * the pages. If @dst_offset is given only reassign the pages to @dst starting
+ * at the @dst_offset (byte). The caller must ensure that @dst is large
+ * enough (e.g. by using aead_count_tsgl with the same offset).
+ */
 static void aead_pull_tsgl(struct sock *sk, size_t used,
-  struct scatterlist *dst)
+  struct scatterlist *dst, size_t dst_offset)
 {
struct alg_sock *ask = alg_sk(sk);
struct aead_ctx *ctx = ask->private;
struct aead_tsgl *sgl;
struct scatterlist *sg;
-   unsigned int i;
+   unsigned int i, j;
 
while (!list_empty(>tsgl_list)) {
sgl = list_first_entry(>tsgl_list, struct aead_tsgl,
   list);
sg = sgl->sg;
 
-   for (i = 0; i < sgl->cur; i++) {
+   for (i = 0, j = 0; i < sgl->cur; i++) {

[ANNOUNCE] libkcapi v0.14.0 released

2017-07-30 Thread Stephan Müller 
Hi,

The Linux kernel exports a network interface of type AF_ALG to allow user 
space to utilize the kernel crypto API. libkcapi uses this network interface 
and exports an easy to use API so that a developer does not need to consider 
the low-level network interface handling. 

The library does not implement any low level cipher algorithms. All consumer 
requests are sent to the kernel for processing. Results from the kernel crypto 
API are returned to the consumer via the library API.

The kernel interface and therefore this library can be used by unprivileged 
processes.

The library code archive also provides a drop-in replacement for the command 
line tools of sha*sum, fipscheck/fipshmac and sha512hmac.

The source code and the documentation is available at [1].

[1] http://www.chronox.de/libkcapi.html

Changes 0.14.0
 * AIO: fix tracking of completed IOCBs
 * speed-test: fix AEAD handling
 * speed-test: fix time calculation
 * compiler now warns a user of deprecated API calls
 * AIO: handle kernel errors for algif_skcipher gracefully
 * AIO: using multiple IOCB if algif_aead interface supports it
 * ASYM: add PKCS1 tests
 * AIO: add ASYM AIO support
 * AIO: fix AEAD AIO fallback
 * AIO: add AIO fallback testing
 * replace enforcement of symmetric cipher limits with a log message only
   (the underlying kernel implementations should catch any errors)
 * add fuzzing tests
 * use autotools build system as provided by Georges Savoundararadj with
   additional considerations from Marcin Nowakowski (thanks a lot)
 * ALG_MAX_PAGES restriction is gone with current AF_ALG interface
 * add HKDF (RFC5869)
 * add apps/kcapi-rng
 * add support for multiple accepts where the caller maintains the opfd
 * fix memleak in error case in PBKDF
 * add multithreaded symmetric cipher tests
 * enable full AIO support for kernels 4.13 and higher (fallback AIO
   implementation using synchronous support for earlier kernels) -- this
   is due to the broken AIO support for earlier kernels
 * Add tests for the AAD copy operation to be supported for kernel 4.13

Ciao
Stephan

[PATCH] staging: ccree: Fix format/argument mismatches

2017-07-30 Thread Joe Perches 
By default, debug logging is disabled by CC_DEBUG not being defined.

Convert SSI_LOG_DEBUG to use no_printk instead of an empty define
to validate formats and arguments.

Fix fallout.

Miscellanea:

o One of the conversions now uses %pR instead of multiple uses of %pad

Signed-off-by: Joe Perches 
---
 drivers/staging/ccree/ssi_aead.c|  8 
 drivers/staging/ccree/ssi_buffer_mgr.c  | 29 +
 drivers/staging/ccree/ssi_cipher.c  | 10 +-
 drivers/staging/ccree/ssi_driver.c  |  5 ++---
 drivers/staging/ccree/ssi_driver.h  |  2 +-
 drivers/staging/ccree/ssi_hash.c| 32 
 drivers/staging/ccree/ssi_request_mgr.c |  6 +++---
 7 files changed, 44 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c
index ea29b8a1a71d..9376bf8b8c61 100644
--- a/drivers/staging/ccree/ssi_aead.c
+++ b/drivers/staging/ccree/ssi_aead.c
@@ -103,7 +103,7 @@ static void ssi_aead_exit(struct crypto_aead *tfm)
if (ctx->enckey) {
dma_free_coherent(dev, AES_MAX_KEY_SIZE, ctx->enckey, 
ctx->enckey_dma_addr);
SSI_LOG_DEBUG("Freed enckey DMA buffer enckey_dma_addr=%pad\n",
- ctx->enckey_dma_addr);
+ >enckey_dma_addr);
ctx->enckey_dma_addr = 0;
ctx->enckey = NULL;
}
@@ -117,7 +117,7 @@ static void ssi_aead_exit(struct crypto_aead *tfm)
  xcbc->xcbc_keys_dma_addr);
}
SSI_LOG_DEBUG("Freed xcbc_keys DMA buffer 
xcbc_keys_dma_addr=%pad\n",
- xcbc->xcbc_keys_dma_addr);
+ >xcbc_keys_dma_addr);
xcbc->xcbc_keys_dma_addr = 0;
xcbc->xcbc_keys = NULL;
} else if (ctx->auth_mode != DRV_HASH_NULL) { /* HMAC auth. */
@@ -128,7 +128,7 @@ static void ssi_aead_exit(struct crypto_aead *tfm)
  hmac->ipad_opad,
  hmac->ipad_opad_dma_addr);
SSI_LOG_DEBUG("Freed ipad_opad DMA buffer 
ipad_opad_dma_addr=%pad\n",
- hmac->ipad_opad_dma_addr);
+ >ipad_opad_dma_addr);
hmac->ipad_opad_dma_addr = 0;
hmac->ipad_opad = NULL;
}
@@ -137,7 +137,7 @@ static void ssi_aead_exit(struct crypto_aead *tfm)
  hmac->padded_authkey,
  hmac->padded_authkey_dma_addr);
SSI_LOG_DEBUG("Freed padded_authkey DMA buffer 
padded_authkey_dma_addr=%pad\n",
- hmac->padded_authkey_dma_addr);
+ >padded_authkey_dma_addr);
hmac->padded_authkey_dma_addr = 0;
hmac->padded_authkey = NULL;
}
diff --git a/drivers/staging/ccree/ssi_buffer_mgr.c 
b/drivers/staging/ccree/ssi_buffer_mgr.c
index 6579a54f9dc4..e13184d1d165 100644
--- a/drivers/staging/ccree/ssi_buffer_mgr.c
+++ b/drivers/staging/ccree/ssi_buffer_mgr.c
@@ -14,6 +14,7 @@
  * along with this program; if not, see .
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -33,14 +34,10 @@
 #include "ssi_hash.h"
 #include "ssi_aead.h"
 
-#ifdef CC_DEBUG
 #define GET_DMA_BUFFER_TYPE(buff_type) ( \
((buff_type) == SSI_DMA_BUF_NULL) ? "BUF_NULL" : \
((buff_type) == SSI_DMA_BUF_DLLI) ? "BUF_DLLI" : \
((buff_type) == SSI_DMA_BUF_MLLI) ? "BUF_MLLI" : "BUF_INVALID")
-#else
-#define GET_DMA_BUFFER_TYPE(buff_type)
-#endif
 
 enum dma_buffer_type {
DMA_NULL_TYPE = -1,
@@ -262,7 +259,7 @@ static int ssi_buffer_mgr_generate_mlli(
SSI_LOG_DEBUG("MLLI params: "
 "virt_addr=%pK dma_addr=%pad mlli_len=0x%X\n",
   mlli_params->mlli_virt_addr,
-  mlli_params->mlli_dma_addr,
+  _params->mlli_dma_addr,
   mlli_params->mlli_len);
 
 build_mlli_exit:
@@ -278,7 +275,7 @@ static inline void ssi_buffer_mgr_add_buffer_entry(
 
SSI_LOG_DEBUG("index=%u single_buff=%pad "
 "buffer_len=0x%08X is_last=%d\n",
-index, buffer_dma, buffer_len, is_last_entry);
+index, _dma, buffer_len, is_last_entry);
sgl_data->nents[index] = 1;
sgl_data->entry[index].buffer_dma = buffer_dma;
sgl_data->offset[index] = 0;
@@ -362,7 +359,7 @@ static int ssi_buffer_mgr_map_scatterlist(
SSI_LOG_DEBUG("Mapped sg: dma_address=%pad "
 "page=%p addr=%pK offset=%u "
 "length=%u\n",
-sg_dma_address(sg),
+

Re: [PATCH 2/3] staging: ccree: Convert to devm_ioremap_resource for map, unmap

2017-07-30 Thread Gilad Ben-Yossef 
On Thu, Jul 27, 2017 at 10:48 PM, Dan Carpenter
 wrote:
> On Thu, Jul 27, 2017 at 05:27:33PM +0300, Gilad Ben-Yossef wrote:
>> + new_drvdata->cc_base = devm_ioremap_resource(_dev->dev,
>> +  req_mem_cc_regs);
>> + if (IS_ERR(new_drvdata->cc_base)) {
>> + rc = PTR_ERR(new_drvdata->cc_base);
>>   goto init_cc_res_err;
> 
> (This code was in the original and not introduced by the patch.)
>
> Ideally, the goto name should say what the goto does.  In this case it
> does everything.  Unfortunately trying to do everything is very
> complicated so obviously the error handling is going to be full of bugs.

Thank you. This review is most helpful. I've added a patch to address
the specific
issues you've raised.

I also see some additional places that can get similar treatment to
both what you
suggested as well as what Suneil has done. I'll address those in
follow up patches.

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

[PATCH v2 3/6] staging: ccree: Replace kzalloc with devm_kzalloc

2017-07-30 Thread Gilad Ben-Yossef 
From: Suniel Mahesh 

It is recommended to use managed function devm_kzalloc, which
simplifies driver cleanup paths and driver code.
This patch does the following:
(a) replace kzalloc with devm_kzalloc.
(b) drop kfree(), because memory allocated with devm_kzalloc() is
automatically freed on driver detach, otherwise it leads to a double
free.
(c) remove unnecessary blank lines.

Signed-off-by: Suniel Mahesh 
[gby: rebase on top of latest coding style fixes changes]
Acked-by: Gilad Ben-Yossef 
---
 drivers/staging/ccree/ssi_driver.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index 1cae2b7..97dfc2c 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -223,13 +223,15 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
struct resource *req_mem_cc_regs = NULL;
void __iomem *cc_base = NULL;
bool irq_registered = false;
-   struct ssi_drvdata *new_drvdata = kzalloc(sizeof(*new_drvdata), 
GFP_KERNEL);
+   struct ssi_drvdata *new_drvdata;
struct device *dev = _dev->dev;
struct device_node *np = dev->of_node;
u32 signature_val;
int rc = 0;
 
-   if (unlikely(!new_drvdata)) {
+   new_drvdata = devm_kzalloc(_dev->dev, sizeof(*new_drvdata),
+  GFP_KERNEL);
+   if (!new_drvdata) {
SSI_LOG_ERR("Failed to allocate drvdata");
rc = -ENOMEM;
goto init_cc_res_err;
@@ -434,10 +436,8 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
   resource_size(new_drvdata->res_mem));
new_drvdata->res_mem = NULL;
}
-   kfree(new_drvdata);
dev_set_drvdata(_dev->dev, NULL);
}
-
return rc;
 }
 
@@ -478,8 +478,6 @@ static void cleanup_cc_resources(struct platform_device 
*plat_dev)
drvdata->cc_base = NULL;
drvdata->res_mem = NULL;
}
-
-   kfree(drvdata);
dev_set_drvdata(_dev->dev, NULL);
 }
 
-- 
2.1.4

[PATCH v2 5/6] staging: ccree: Use platform_get_irq and devm_request_irq

2017-07-30 Thread Gilad Ben-Yossef 
From: Suniel Mahesh 

It is recommended to use managed function devm_request_irq(),
which simplifies driver cleanup paths and driver code.
This patch does the following:
(a) replace platform_get_resource(), request_irq() and corresponding
error handling with platform_get_irq() and devm_request_irq().
(b) remove struct resource pointer(res_irq) in struct ssi_drvdata as
it seems redundant.
(c) change type of member irq in struct ssi_drvdata from unsigned int
to int, as return type of platform_get_irq is int and can be used in
error handling.
(d) remove irq_registered variable from driver probe as it seems
redundant.
(e) free_irq is not required any more, devm_request_irq() free's it
on driver detach.
(f) adjust log messages accordingly and remove any blank lines.

Signed-off-by: Suniel Mahesh 
Acked-by: Gilad Ben-Yossef 
---
 drivers/staging/ccree/ssi_driver.c | 30 +-
 drivers/staging/ccree/ssi_driver.h |  3 +--
 2 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index 603eb03..c18e7e3 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -222,7 +222,6 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
 {
struct resource *req_mem_cc_regs = NULL;
void __iomem *cc_base = NULL;
-   bool irq_registered = false;
struct ssi_drvdata *new_drvdata;
struct device *dev = _dev->dev;
struct device_node *np = dev->of_node;
@@ -262,26 +261,22 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
  _mem_cc_regs->start, new_drvdata->cc_base);
cc_base = new_drvdata->cc_base;
/* Then IRQ */
-   new_drvdata->res_irq = platform_get_resource(plat_dev, IORESOURCE_IRQ, 
0);
-   if (unlikely(!new_drvdata->res_irq)) {
+   new_drvdata->irq = platform_get_irq(plat_dev, 0);
+   if (new_drvdata->irq < 0) {
SSI_LOG_ERR("Failed getting IRQ resource\n");
-   rc = -ENODEV;
+   rc = new_drvdata->irq;
goto init_cc_res_err;
}
-   rc = request_irq(new_drvdata->res_irq->start, cc_isr,
-IRQF_SHARED, "arm_cc7x", new_drvdata);
-   if (unlikely(rc != 0)) {
-   SSI_LOG_ERR("Could not register to interrupt %llu\n",
-   (unsigned long long)new_drvdata->res_irq->start);
+   rc = devm_request_irq(_dev->dev, new_drvdata->irq, cc_isr,
+ IRQF_SHARED, "arm_cc7x", new_drvdata);
+   if (rc) {
+   SSI_LOG_ERR("Could not register to interrupt %d\n",
+   new_drvdata->irq);
goto init_cc_res_err;
}
init_completion(_drvdata->icache_setup_completion);
 
-   irq_registered = true;
-   SSI_LOG_DEBUG("Registered to IRQ (%s) %llu\n",
- new_drvdata->res_irq->name,
- (unsigned long long)new_drvdata->res_irq->start);
-
+   SSI_LOG_DEBUG("Registered to IRQ: %d\n", new_drvdata->irq);
new_drvdata->plat_dev = plat_dev;
 
rc = cc_clk_on(new_drvdata);
@@ -410,10 +405,6 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
 #ifdef ENABLE_CC_SYSFS
ssi_sysfs_fini();
 #endif
-   if (irq_registered) {
-   free_irq(new_drvdata->res_irq->start, new_drvdata);
-   new_drvdata->res_irq = NULL;
-   }
dev_set_drvdata(_dev->dev, NULL);
}
return rc;
@@ -443,11 +434,8 @@ static void cleanup_cc_resources(struct platform_device 
*plat_dev)
 #ifdef ENABLE_CC_SYSFS
ssi_sysfs_fini();
 #endif
-
fini_cc_regs(drvdata);
cc_clk_off(drvdata);
-   free_irq(drvdata->res_irq->start, drvdata);
-   drvdata->res_irq = NULL;
dev_set_drvdata(_dev->dev, NULL);
 }
 
diff --git a/drivers/staging/ccree/ssi_driver.h 
b/drivers/staging/ccree/ssi_driver.h
index 518c0bf..88ef370 100644
--- a/drivers/staging/ccree/ssi_driver.h
+++ b/drivers/staging/ccree/ssi_driver.h
@@ -128,9 +128,8 @@ struct ssi_crypto_req {
  * @fw_ver:SeP loaded firmware version
  */
 struct ssi_drvdata {
-   struct resource *res_irq;
void __iomem *cc_base;
-   unsigned int irq;
+   int irq;
u32 irq_mask;
u32 fw_ver;
/* Calibration time of start/stop
-- 
2.1.4

[PATCH v2 2/6] staging: ccree: kmalloc by sizeof var not type

2017-07-30 Thread Gilad Ben-Yossef 
Change places where we alloc memory by sizeof type to sizeof var.

Signed-off-by: Gilad Ben-Yossef 
---
 drivers/staging/ccree/ssi_aead.c| 4 ++--
 drivers/staging/ccree/ssi_cipher.c  | 4 ++--
 drivers/staging/ccree/ssi_driver.c  | 2 +-
 drivers/staging/ccree/ssi_hash.c| 4 ++--
 drivers/staging/ccree/ssi_ivgen.c   | 2 +-
 drivers/staging/ccree/ssi_request_mgr.c | 2 +-
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c
index d8f2249..a8cb432 100644
--- a/drivers/staging/ccree/ssi_aead.c
+++ b/drivers/staging/ccree/ssi_aead.c
@@ -2658,7 +2658,7 @@ static struct ssi_crypto_alg *ssi_aead_create_alg(struct 
ssi_alg_template *templ
struct ssi_crypto_alg *t_alg;
struct aead_alg *alg;
 
-   t_alg = kzalloc(sizeof(struct ssi_crypto_alg), GFP_KERNEL);
+   t_alg = kzalloc(sizeof(*t_alg), GFP_KERNEL);
if (!t_alg) {
SSI_LOG_ERR("failed to allocate t_alg\n");
return ERR_PTR(-ENOMEM);
@@ -2713,7 +2713,7 @@ int ssi_aead_alloc(struct ssi_drvdata *drvdata)
int rc = -ENOMEM;
int alg;
 
-   aead_handle = kmalloc(sizeof(struct ssi_aead_handle), GFP_KERNEL);
+   aead_handle = kmalloc(sizeof(*aead_handle), GFP_KERNEL);
if (!aead_handle) {
rc = -ENOMEM;
goto fail0;
diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index 068b10b..d98178d 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -1215,7 +1215,7 @@ struct ssi_crypto_alg *ssi_ablkcipher_create_alg(struct 
ssi_alg_template *templa
struct ssi_crypto_alg *t_alg;
struct crypto_alg *alg;
 
-   t_alg = kzalloc(sizeof(struct ssi_crypto_alg), GFP_KERNEL);
+   t_alg = kzalloc(sizeof(*t_alg), GFP_KERNEL);
if (!t_alg) {
SSI_LOG_ERR("failed to allocate t_alg\n");
return ERR_PTR(-ENOMEM);
@@ -1276,7 +1276,7 @@ int ssi_ablkcipher_alloc(struct ssi_drvdata *drvdata)
int rc = -ENOMEM;
int alg;
 
-   ablkcipher_handle = kmalloc(sizeof(struct ssi_blkcipher_handle),
+   ablkcipher_handle = kmalloc(sizeof(*ablkcipher_handle),
GFP_KERNEL);
if (!ablkcipher_handle)
return -ENOMEM;
diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index d104dbd..1cae2b7 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -223,7 +223,7 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
struct resource *req_mem_cc_regs = NULL;
void __iomem *cc_base = NULL;
bool irq_registered = false;
-   struct ssi_drvdata *new_drvdata = kzalloc(sizeof(struct ssi_drvdata), 
GFP_KERNEL);
+   struct ssi_drvdata *new_drvdata = kzalloc(sizeof(*new_drvdata), 
GFP_KERNEL);
struct device *dev = _dev->dev;
struct device_node *np = dev->of_node;
u32 signature_val;
diff --git a/drivers/staging/ccree/ssi_hash.c b/drivers/staging/ccree/ssi_hash.c
index 3a734df..6c08b1d 100644
--- a/drivers/staging/ccree/ssi_hash.c
+++ b/drivers/staging/ccree/ssi_hash.c
@@ -2055,7 +2055,7 @@ ssi_hash_create_alg(struct ssi_hash_template *template, 
bool keyed)
struct crypto_alg *alg;
struct ahash_alg *halg;
 
-   t_crypto_alg = kzalloc(sizeof(struct ssi_hash_alg), GFP_KERNEL);
+   t_crypto_alg = kzalloc(sizeof(*t_crypto_alg), GFP_KERNEL);
if (!t_crypto_alg) {
SSI_LOG_ERR("failed to allocate t_alg\n");
return ERR_PTR(-ENOMEM);
@@ -2221,7 +2221,7 @@ int ssi_hash_alloc(struct ssi_drvdata *drvdata)
int rc = 0;
int alg;
 
-   hash_handle = kzalloc(sizeof(struct ssi_hash_handle), GFP_KERNEL);
+   hash_handle = kzalloc(sizeof(*hash_handle), GFP_KERNEL);
if (!hash_handle) {
SSI_LOG_ERR("kzalloc failed to allocate %zu B\n",
sizeof(struct ssi_hash_handle));
diff --git a/drivers/staging/ccree/ssi_ivgen.c 
b/drivers/staging/ccree/ssi_ivgen.c
index bca44af..93a2a94 100644
--- a/drivers/staging/ccree/ssi_ivgen.c
+++ b/drivers/staging/ccree/ssi_ivgen.c
@@ -191,7 +191,7 @@ int ssi_ivgen_init(struct ssi_drvdata *drvdata)
int rc;
 
/* Allocate "this" context */
-   drvdata->ivgen_handle = kzalloc(sizeof(struct ssi_ivgen_ctx), 
GFP_KERNEL);
+   drvdata->ivgen_handle = kzalloc(sizeof(*drvdata->ivgen_handle), 
GFP_KERNEL);
if (!drvdata->ivgen_handle) {
SSI_LOG_ERR("Not enough memory to allocate IVGEN context (%zu 
B)\n",
sizeof(struct ssi_ivgen_ctx));
diff --git a/drivers/staging/ccree/ssi_request_mgr.c 
b/drivers/staging/ccree/ssi_request_mgr.c
index 9a4bb5c..cae9904 100644
--- a/drivers/staging/ccree/ssi_request_mgr.c
+++

[PATCH v2 6/6] staging: ccree: simplify resource release on error

2017-07-30 Thread Gilad Ben-Yossef 
The resource release on probe/init error was being handled
in an awkward manner and possibly leaking memory on certain
(unlikely) error path.

Fix it by simplifying the error resource release and making
it easier to track.

Reported-by: Dan Carpenter 
Signed-off-by: Gilad Ben-Yossef 
---
 drivers/staging/ccree/ssi_aead.c   |   3 +-
 drivers/staging/ccree/ssi_cipher.c |   3 +-
 drivers/staging/ccree/ssi_driver.c | 102 -
 drivers/staging/ccree/ssi_hash.c   |   3 +-
 4 files changed, 59 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c
index a8cb432..66eedbe 100644
--- a/drivers/staging/ccree/ssi_aead.c
+++ b/drivers/staging/ccree/ssi_aead.c
@@ -2719,6 +2719,7 @@ int ssi_aead_alloc(struct ssi_drvdata *drvdata)
goto fail0;
}
 
+   INIT_LIST_HEAD(_handle->aead_list);
drvdata->aead_handle = aead_handle;
 
aead_handle->sram_workspace_addr = ssi_sram_mgr_alloc(
@@ -2729,8 +2730,6 @@ int ssi_aead_alloc(struct ssi_drvdata *drvdata)
goto fail1;
}
 
-   INIT_LIST_HEAD(_handle->aead_list);
-
/* Linux crypto */
for (alg = 0; alg < ARRAY_SIZE(aead_algs); alg++) {
t_alg = ssi_aead_create_alg(_algs[alg]);
diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index d98178d..712b21d 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -1281,9 +1281,8 @@ int ssi_ablkcipher_alloc(struct ssi_drvdata *drvdata)
if (!ablkcipher_handle)
return -ENOMEM;
 
-   drvdata->blkcipher_handle = ablkcipher_handle;
-
INIT_LIST_HEAD(_handle->blkcipher_alg_list);
+   drvdata->blkcipher_handle = ablkcipher_handle;
 
/* Linux crypto */
SSI_LOG_DEBUG("Number of algorithms = %zu\n", 
ARRAY_SIZE(blkcipher_algs));
diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index c18e7e3..1b95f90 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -233,16 +233,14 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
if (!new_drvdata) {
SSI_LOG_ERR("Failed to allocate drvdata");
rc = -ENOMEM;
-   goto init_cc_res_err;
+   goto post_drvdata_err;
}
+   dev_set_drvdata(_dev->dev, new_drvdata);
+   new_drvdata->plat_dev = plat_dev;
 
new_drvdata->clk = of_clk_get(np, 0);
new_drvdata->coherent = of_dma_is_coherent(np);
 
-   /*Initialize inflight counter used in dx_ablkcipher_secure_complete 
used for count of BYSPASS blocks operations*/
-   new_drvdata->inflight_counter = 0;
-
-   dev_set_drvdata(_dev->dev, new_drvdata);
/* Get device resources */
/* First CC registers space */
req_mem_cc_regs = platform_get_resource(plat_dev, IORESOURCE_MEM, 0);
@@ -250,38 +248,42 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
new_drvdata->cc_base = devm_ioremap_resource(_dev->dev,
 req_mem_cc_regs);
if (IS_ERR(new_drvdata->cc_base)) {
+   SSI_LOG_ERR("Failed to ioremap registers");
rc = PTR_ERR(new_drvdata->cc_base);
-   goto init_cc_res_err;
+   goto post_drvdata_err;
}
+
SSI_LOG_DEBUG("Got MEM resource (%s): start=%pad end=%pad\n",
  req_mem_cc_regs->name,
  req_mem_cc_regs->start,
  req_mem_cc_regs->end);
SSI_LOG_DEBUG("CC registers mapped from %pa to 0x%p\n",
  _mem_cc_regs->start, new_drvdata->cc_base);
+
cc_base = new_drvdata->cc_base;
+
/* Then IRQ */
new_drvdata->irq = platform_get_irq(plat_dev, 0);
if (new_drvdata->irq < 0) {
SSI_LOG_ERR("Failed getting IRQ resource\n");
rc = new_drvdata->irq;
-   goto init_cc_res_err;
+   goto post_drvdata_err;
}
+
rc = devm_request_irq(_dev->dev, new_drvdata->irq, cc_isr,
  IRQF_SHARED, "arm_cc7x", new_drvdata);
if (rc) {
SSI_LOG_ERR("Could not register to interrupt %d\n",
new_drvdata->irq);
-   goto init_cc_res_err;
+   goto post_drvdata_err;
}
-   init_completion(_drvdata->icache_setup_completion);
-
SSI_LOG_DEBUG("Registered to IRQ: %d\n", new_drvdata->irq);
-   new_drvdata->plat_dev = plat_dev;
+
+   init_completion(_drvdata->icache_setup_completion);
 
rc = cc_clk_on(new_drvdata);
if (rc)
-   goto init_cc_res_err;
+   goto post_drvdata_err;
 
if (!new_drvdata->plat_dev->dev.dma_mask)

[PATCH v2 0/6] ccree style fixes and simplification

2017-07-30 Thread Gilad Ben-Yossef 
Various style code fixes and simplification from myself and Suniel.

Changes from v2:

- Rebase on top of current staging-next.
- Added resource release on error simplification patch suggested by
  Dan C. as part of the review.

I made sure the code applies on top both current staging-next
(commit 0d0d4d21a09981e65b2bd386c999e8c0ecc6444e) as well as
staging-testing and of course compiles and runs. If this doesn't
apply cleanly I'm going to start suspecting Gremlins at out IT dept.

Gilad Ben-Yossef (3):
  staging: ccree: fix split strings
  staging: ccree: kmalloc by sizeof var not type
  staging: ccree: simplify resource release on error

Suniel Mahesh (3):
  staging: ccree: Replace kzalloc with devm_kzalloc
  staging: ccree: Convert to devm_ioremap_resource for map, unmap
  staging: ccree: Use platform_get_irq and devm_request_irq

 drivers/staging/ccree/ssi_aead.c|  16 ++-
 drivers/staging/ccree/ssi_buffer_mgr.c  |  86 ++
 drivers/staging/ccree/ssi_cipher.c  |  34 +++---
 drivers/staging/ccree/ssi_driver.c  | 192 +---
 drivers/staging/ccree/ssi_driver.h  |   4 +-
 drivers/staging/ccree/ssi_hash.c|  50 -
 drivers/staging/ccree/ssi_ivgen.c   |  10 +-
 drivers/staging/ccree/ssi_request_mgr.c |  15 +--
 8 files changed, 170 insertions(+), 237 deletions(-)

-- 
2.1.4

[PATCH v2 4/6] staging: ccree: Convert to devm_ioremap_resource for map, unmap

2017-07-30 Thread Gilad Ben-Yossef 
From: Suniel Mahesh 

It is recommended to use managed function devm_ioremap_resource(),
which simplifies driver cleanup paths and driver code.
This patch does the following:
(a) replace request_mem_region(), ioremap() and corresponding error
handling with devm_ioremap_resource().
(b) remove struct resource pointer(res_mem) in struct ssi_drvdata as it
seems redundant, use struct resource pointer which is defined locally and
adjust return value of platform_get_resource() accordingly.
(c) release_mem_region() and iounmap() are dropped, since devm_ioremap_
resource() releases and unmaps mem region on driver detach.
(d) adjust log messages accordingly and remove any blank lines.

Signed-off-by: Suniel Mahesh 
[gby: rebase on top of latest coding style fixes changes]
Acked-by: Gilad Ben-Yossef 
---
 drivers/staging/ccree/ssi_driver.c | 60 ++
 drivers/staging/ccree/ssi_driver.h |  1 -
 2 files changed, 15 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index 97dfc2c..603eb03 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -246,35 +246,21 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
dev_set_drvdata(_dev->dev, new_drvdata);
/* Get device resources */
/* First CC registers space */
-   new_drvdata->res_mem = platform_get_resource(plat_dev, IORESOURCE_MEM, 
0);
-   if (unlikely(!new_drvdata->res_mem)) {
-   SSI_LOG_ERR("Failed getting IO memory resource\n");
-   rc = -ENODEV;
-   goto init_cc_res_err;
-   }
-   SSI_LOG_DEBUG("Got MEM resource (%s): start=%pad end=%pad\n",
- new_drvdata->res_mem->name,
- new_drvdata->res_mem->start,
- new_drvdata->res_mem->end);
+   req_mem_cc_regs = platform_get_resource(plat_dev, IORESOURCE_MEM, 0);
/* Map registers space */
-   req_mem_cc_regs = request_mem_region(new_drvdata->res_mem->start, 
resource_size(new_drvdata->res_mem), "arm_cc7x_regs");
-   if (unlikely(!req_mem_cc_regs)) {
-   SSI_LOG_ERR("Couldn't allocate registers memory region at 
0x%08X\n",
-   (unsigned int)new_drvdata->res_mem->start);
-   rc = -EBUSY;
-   goto init_cc_res_err;
-   }
-   cc_base = ioremap(new_drvdata->res_mem->start, 
resource_size(new_drvdata->res_mem));
-   if (unlikely(!cc_base)) {
-   SSI_LOG_ERR("ioremap[CC](0x%08X,0x%08X) failed\n",
-   (unsigned int)new_drvdata->res_mem->start,
-   (unsigned int)resource_size(new_drvdata->res_mem));
-   rc = -ENOMEM;
+   new_drvdata->cc_base = devm_ioremap_resource(_dev->dev,
+req_mem_cc_regs);
+   if (IS_ERR(new_drvdata->cc_base)) {
+   rc = PTR_ERR(new_drvdata->cc_base);
goto init_cc_res_err;
}
-   SSI_LOG_DEBUG("CC registers mapped from %pa to 0x%p\n", 
_drvdata->res_mem->start, cc_base);
-   new_drvdata->cc_base = cc_base;
-
+   SSI_LOG_DEBUG("Got MEM resource (%s): start=%pad end=%pad\n",
+ req_mem_cc_regs->name,
+ req_mem_cc_regs->start,
+ req_mem_cc_regs->end);
+   SSI_LOG_DEBUG("CC registers mapped from %pa to 0x%p\n",
+ _mem_cc_regs->start, new_drvdata->cc_base);
+   cc_base = new_drvdata->cc_base;
/* Then IRQ */
new_drvdata->res_irq = platform_get_resource(plat_dev, IORESOURCE_IRQ, 
0);
if (unlikely(!new_drvdata->res_irq)) {
@@ -424,17 +410,9 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
 #ifdef ENABLE_CC_SYSFS
ssi_sysfs_fini();
 #endif
-
-   if (req_mem_cc_regs) {
-   if (irq_registered) {
-   free_irq(new_drvdata->res_irq->start, 
new_drvdata);
-   new_drvdata->res_irq = NULL;
-   iounmap(cc_base);
-   new_drvdata->cc_base = NULL;
-   }
-   release_mem_region(new_drvdata->res_mem->start,
-  resource_size(new_drvdata->res_mem));
-   new_drvdata->res_mem = NULL;
+   if (irq_registered) {
+   free_irq(new_drvdata->res_irq->start, new_drvdata);
+   new_drvdata->res_irq = NULL;
}
dev_set_drvdata(_dev->dev, NULL);
}
@@ -470,14 +448,6 @@ static void cleanup_cc_resources(struct platform_device 
*plat_dev)
cc_clk_off(drvdata);
free_irq(drvdata->res_irq->start, drvdata);
drvdata->res_irq = NULL;
-
-   if

Re: [PATCH v5 3/6] iomap: introduce io{read|write}64_{lo_hi|hi_lo}

2017-07-30 Thread Andy Shevchenko 
On Thu, Jul 27, 2017 at 2:19 AM, Logan Gunthorpe  wrote:
> In order to provide non-atomic functions for io{read|write}64 that will
> use readq and writeq when appropriate. We define a number of variants
> of these functions in the generic iomap that will do non-atomic
> operations on pio but atomic operations on mmio.
>
> These functions are only defined if readq and writeq are defined. If
> they are not, then the wrappers that always use non-atomic operations
> from include/linux/io-64-nonatomic*.h will be used.

Don't you see here a slight problem?

In some cases we want to substitute atomic in favour of non-atomic
when both are defined.
So, please don't do this "smartly".

> +u64 ioread64_lo_hi(void __iomem *addr)
> +{
> +   IO_COND(addr, return pio_read64_lo_hi(port), return readq(addr));
> +   return 0xLL;
> +}

U missed u.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 7/7] ima: Support module-style appended signatures for appraisal

2017-07-30 Thread Mimi Zohar 
On Thu, 2017-07-06 at 19:17 -0300, Thiago Jung Bauermann wrote:
> This patch introduces the modsig keyword to the IMA policy syntax to
> specify that a given hook should expect the file to have the IMA signature
> appended to it. Here is how it can be used in a rule:
> 
> appraise func=KEXEC_KERNEL_CHECK appraise_type=modsig|imasig
> 
> With this rule, IMA will accept either an appended signature or a signature
> stored in the extended attribute. In that case, it will first check whether
> there is an appended signature, and if not it will read it from the
> extended attribute.
> 
> 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. This means that only FIRMWARE_CHECK, KEXEC_KERNEL_CHECK,
> KEXEC_INITRAMFS_CHECK and POLICY_CHECK can be supported.
> 
> This feature warrants a separate config option because enabling it brings
> in many other config options.
> 
> Signed-off-by: Thiago Jung Bauermann 
> ---
>  security/integrity/ima/Kconfig|  13 +++
>  security/integrity/ima/Makefile   |   1 +
>  security/integrity/ima/ima.h  |  60 ++--
>  security/integrity/ima/ima_appraise.c | 102 ++---
>  security/integrity/ima/ima_main.c |   7 +-
>  security/integrity/ima/ima_modsig.c   | 147 
> ++
>  security/integrity/ima/ima_policy.c   |  26 --
>  security/integrity/ima/ima_template_lib.c |  14 ++-
>  security/integrity/integrity.h|   4 +-
>  9 files changed, 343 insertions(+), 31 deletions(-)
> 
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 35ef69312811..55f734a6124b 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -163,6 +163,19 @@ config IMA_APPRAISE_BOOTPARAM
> This option enables the different "ima_appraise=" modes
> (eg. fix, log) from the boot command line.
> 
> +config IMA_APPRAISE_MODSIG
> + bool "Support module-style signatures for appraisal"
> + depends on IMA_APPRAISE
> + depends on INTEGRITY_ASYMMETRIC_KEYS
> + select PKCS7_MESSAGE_PARSER
> + select MODULE_SIG_FORMAT
> + default n
> + help
> +Adds support for signatures appended to files. The format of the
> +appended signature is the same used for signed kernel modules.
> +The modsig keyword can be used in the IMA policy to allow a hook
> +to accept such signatures.
> +
>  config IMA_TRUSTED_KEYRING
>   bool "Require all keys on the .ima keyring be signed (deprecated)"
>   depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
> diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
> index 29f198bde02b..c72026acecc3 100644
> --- a/security/integrity/ima/Makefile
> +++ b/security/integrity/ima/Makefile
> @@ -8,5 +8,6 @@ obj-$(CONFIG_IMA) += ima.o
>  ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
>ima_policy.o ima_template.o ima_template_lib.o
>  ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
> +ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o
>  ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
>  obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index d52b487ad259..1e1e7c41ca19 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -190,6 +190,8 @@ enum ima_hooks {
>   __ima_hooks(__ima_hook_enumify)
>  };
> 
> +extern const char *const func_tokens[];
> +
>  /* LIM API function definitions */
>  int ima_get_action(struct inode *inode, int mask,
>  enum ima_hooks func, int *pcr);
> @@ -236,9 +238,10 @@ int ima_policy_show(struct seq_file *m, void *v);
>  #ifdef CONFIG_IMA_APPRAISE
>  int ima_appraise_measurement(enum ima_hooks func,
>struct integrity_iint_cache *iint,
> -  struct file *file, const unsigned char *filename,
> -  struct evm_ima_xattr_data *xattr_value,
> -  int xattr_len, int opened);
> +  struct file *file, const void *buf, loff_t size,
> +  const unsigned char *filename,
> +  struct evm_ima_xattr_data **xattr_value,
> +  int *xattr_len, int opened);
>  int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
>  void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
>  enum integrity_status

[PATCH] ccree: Removed a sparse warning in ssi_fips.h

2017-07-30 Thread rishabh hardas 
Sparse was giving out a warning 'warning: symbol 'cc_set_ree_fips_status' was 
not declared. Should it be static?'
which has been removed by making it static.

Signed-off-by: rishabh hardas 
---
 drivers/staging/ccree/ssi_fips.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ccree/ssi_fips.h b/drivers/staging/ccree/ssi_fips.h
index 369ddf9..846bd55 100644
--- a/drivers/staging/ccree/ssi_fips.h
+++ b/drivers/staging/ccree/ssi_fips.h
@@ -40,7 +40,7 @@ static inline int ssi_fips_init(struct ssi_drvdata *p_drvdata)
 }

 static inline void ssi_fips_fini(struct ssi_drvdata *drvdata) {}
-void cc_set_ree_fips_status(struct ssi_drvdata *drvdata, bool ok) {}
+static void cc_set_ree_fips_status(struct ssi_drvdata *drvdata, bool ok) {}
 void fips_handler(struct ssi_drvdata *drvdata) {}

 #endif /* CONFIG_CRYPTO_FIPS */
--
2.7.4

Re: [PATCH] Removed a sparse warning

2017-07-30 Thread Greg KH 
On Sun, Jul 30, 2017 at 03:05:18PM +0530, rishabh hardas wrote:
> Signed-off-by: rishabh hardas 
> ---
>  drivers/staging/ccree/ssi_fips.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I can't take patches without any changelog text at all.  Please read the
documentation for how to create a proper subject as well.

thanks,

greg k-h

Re: [RFC PATCH v12 3/4] Linux Random Number Generator

2017-07-30 Thread Pavel Machek 
Hi!

On Tue 2017-07-18 21:51:33, Theodore Ts'o wrote:
> On Tue, Jul 18, 2017 at 09:00:10PM -0400, Sandy Harris wrote:
> > The only really good solution I know of is to find a way to provide a
> > chunk of randomness early in the boot process. John Denker has a good
> > discussion of doing this by modifying the kernel image & Ted talks of
> > doing it via the boot loader. Neither looks remarkably easy. Other
> > approaches like making the kernel read a seed file or passing a
> > parameter on the kernel command line have been suggested but, if I
> > recall right, rejected.
> 
> It's actually not that _hard_ to modify the boot loader.  It's not
> finicky work like, say, adding support for metadata checksums or xattr
> deduplication to ext4.  It's actually mostly plumbing.  It's just that
> we haven't found a lot of people willing to do it as paid work, and
> the hobbyists haven't been interested.

Modifying the boot loader sources is not hard, right.

Deploying the modified boot loader is another story; these are
bootloaders -- they normally don't need updating, so they are often
not easy to update, or maybe updating them is risky.

Anyway, if you want to pay for some bootloader modifications... I'm
working for a company that can help :-). (Sometimes I use
pa...@denx.de address.)

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH] Removed a sparse warning

2017-07-30 Thread rishabh hardas 
Signed-off-by: rishabh hardas 
---
 drivers/staging/ccree/ssi_fips.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ccree/ssi_fips.h b/drivers/staging/ccree/ssi_fips.h
index 369ddf9..846bd55 100644
--- a/drivers/staging/ccree/ssi_fips.h
+++ b/drivers/staging/ccree/ssi_fips.h
@@ -40,7 +40,7 @@ static inline int ssi_fips_init(struct ssi_drvdata *p_drvdata)
 }

 static inline void ssi_fips_fini(struct ssi_drvdata *drvdata) {}
-void cc_set_ree_fips_status(struct ssi_drvdata *drvdata, bool ok) {}
+static void cc_set_ree_fips_status(struct ssi_drvdata *drvdata, bool ok) {}
 void fips_handler(struct ssi_drvdata *drvdata) {}

 #endif /* CONFIG_CRYPTO_FIPS */
--
2.7.4