Re: [PATCH v2] dt/bindings: exynos-rng: Move dt binding documentation to bindings/crypto

2017-08-23 Thread PrasannaKumar Muralidharan 
On 23 August 2017 at 22:24, Krzysztof Kozlowski  wrote:
> On Wed, Aug 23, 2017 at 10:14:29PM +0530, PrasannaKumar Muralidharan wrote:
>> Hi Krzysztof,
>>
>> On 23 August 2017 at 21:42, Krzysztof Kozlowski  wrote:
>> > On Wed, Aug 23, 2017 at 08:34:43PM +0530, PrasannaKumar Muralidharan wrote:
>> >> Samsung exynos PRNG driver is using crypto framework instead of
>> >> hw_random framework. So move the devicetree binding to crypto folder.
>> >>
>> >> Signed-off-by: PrasannaKumar Muralidharan 
>> >> ---
>> >> Changes in v2:
>> >> * Modify MAINTAINERS file to reflect file rename
>> >>
>> >>  .../devicetree/bindings/{rng => crypto}/samsung,exynos-rng4.txt 
>> >> | 0
>> >>  MAINTAINERS 
>> >> | 2 +-
>> >>  2 files changed, 1 insertion(+), 1 deletion(-)
>> >>  rename Documentation/devicetree/bindings/{rng => 
>> >> crypto}/samsung,exynos-rng4.txt (100%)
>> >>
>> >
>> > Patch is okay but CC list is still incomplete. I do not know how you
>> > could get Mauro for this patch... just use get_maintainers.pl.
>>
>> I used get_maintainer.pl. Rechecked again, it says Mauro and couple of
>> others who may not be interested in this. It did not give your or
>> crypto mailing list. Its strange.
>
> Hm, you're right. Aparently get_maintainer.pl does handle moved files
> thus he printed only entries for MAINTAINERS.
>
> You will get full entry with:
> scripts/get_maintainer.pl -f 
> Documentation/devicetree/bindings/crypto/samsung,exynos-rng4.txt

Will keep this in mind.

> Krzysztof Kozlowski  (maintainer:SAMSUNG EXYNOS PSEUDO 
> RANDOM NUMBER GENERATOR (...)
> Herbert Xu  (maintainer:CRYPTO API)
> "David S. Miller"  (maintainer:CRYPTO API)
> Rob Herring  (maintainer:OPEN FIRMWARE AND FLATTENED 
> DEVICE TREE BINDINGS)
> Mark Rutland  (maintainer:OPEN FIRMWARE AND FLATTENED 
> DEVICE TREE BINDINGS)
> Kukjin Kim  (maintainer:ARM/SAMSUNG EXYNOS ARM 
> ARCHITECTURES)
> linux-crypto@vger.kernel.org (open list:SAMSUNG EXYNOS PSEUDO RANDOM NUMBER 
> GENERATOR (...)
> linux-samsung-...@vger.kernel.org (open list:SAMSUNG EXYNOS PSEUDO RANDOM 
> NUMBER GENERATOR (...)
> devicet...@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE 
> BINDINGS)
> linux-arm-ker...@lists.infradead.org (moderated list:ARM/SAMSUNG EXYNOS ARM 
> ARCHITECTURES)
> linux-ker...@vger.kernel.org (open list)
>
> Best regards,
> Krzysztof
>

Thanks,
PrasannaKumar

Re: [PATCH v2] dt/bindings: exynos-rng: Move dt binding documentation to bindings/crypto

2017-08-23 Thread Krzysztof Kozlowski 
On Wed, Aug 23, 2017 at 10:14:29PM +0530, PrasannaKumar Muralidharan wrote:
> Hi Krzysztof,
> 
> On 23 August 2017 at 21:42, Krzysztof Kozlowski  wrote:
> > On Wed, Aug 23, 2017 at 08:34:43PM +0530, PrasannaKumar Muralidharan wrote:
> >> Samsung exynos PRNG driver is using crypto framework instead of
> >> hw_random framework. So move the devicetree binding to crypto folder.
> >>
> >> Signed-off-by: PrasannaKumar Muralidharan 
> >> ---
> >> Changes in v2:
> >> * Modify MAINTAINERS file to reflect file rename
> >>
> >>  .../devicetree/bindings/{rng => crypto}/samsung,exynos-rng4.txt | > >> 0
> >>  MAINTAINERS | 
> >> 2 +-
> >>  2 files changed, 1 insertion(+), 1 deletion(-)
> >>  rename Documentation/devicetree/bindings/{rng => 
> >> crypto}/samsung,exynos-rng4.txt (100%)
> >>
> >
> > Patch is okay but CC list is still incomplete. I do not know how you
> > could get Mauro for this patch... just use get_maintainers.pl.
> 
> I used get_maintainer.pl. Rechecked again, it says Mauro and couple of
> others who may not be interested in this. It did not give your or
> crypto mailing list. Its strange.

Hm, you're right. Aparently get_maintainer.pl does handle moved files
thus he printed only entries for MAINTAINERS.

You will get full entry with:
scripts/get_maintainer.pl -f 
Documentation/devicetree/bindings/crypto/samsung,exynos-rng4.txt

Krzysztof Kozlowski  (maintainer:SAMSUNG EXYNOS PSEUDO RANDOM 
NUMBER GENERATOR (...)
Herbert Xu  (maintainer:CRYPTO API)
"David S. Miller"  (maintainer:CRYPTO API)
Rob Herring  (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE 
TREE BINDINGS)
Mark Rutland  (maintainer:OPEN FIRMWARE AND FLATTENED 
DEVICE TREE BINDINGS)
Kukjin Kim  (maintainer:ARM/SAMSUNG EXYNOS ARM ARCHITECTURES)
linux-crypto@vger.kernel.org (open list:SAMSUNG EXYNOS PSEUDO RANDOM NUMBER 
GENERATOR (...)
linux-samsung-...@vger.kernel.org (open list:SAMSUNG EXYNOS PSEUDO RANDOM 
NUMBER GENERATOR (...)
devicet...@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE 
BINDINGS)
linux-arm-ker...@lists.infradead.org (moderated list:ARM/SAMSUNG EXYNOS ARM 
ARCHITECTURES)
linux-ker...@vger.kernel.org (open list)

Best regards,
Krzysztof

Re: [PATCH v2] dt/bindings: exynos-rng: Move dt binding documentation to bindings/crypto

2017-08-23 Thread PrasannaKumar Muralidharan 
Hi Krzysztof,

On 23 August 2017 at 21:42, Krzysztof Kozlowski  wrote:
> On Wed, Aug 23, 2017 at 08:34:43PM +0530, PrasannaKumar Muralidharan wrote:
>> Samsung exynos PRNG driver is using crypto framework instead of
>> hw_random framework. So move the devicetree binding to crypto folder.
>>
>> Signed-off-by: PrasannaKumar Muralidharan 
>> ---
>> Changes in v2:
>> * Modify MAINTAINERS file to reflect file rename
>>
>>  .../devicetree/bindings/{rng => crypto}/samsung,exynos-rng4.txt | 0
>>  MAINTAINERS | 2 
>> +-
>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>  rename Documentation/devicetree/bindings/{rng => 
>> crypto}/samsung,exynos-rng4.txt (100%)
>>
>
> Patch is okay but CC list is still incomplete. I do not know how you
> could get Mauro for this patch... just use get_maintainers.pl.

I used get_maintainer.pl. Rechecked again, it says Mauro and couple of
others who may not be interested in this. It did not give your or
crypto mailing list. Its strange.

> Reviewed-by: Krzysztof Kozlowski 

Thanks for the review.

> Best regards,
> Krzysztof
>

Re: [PATCH v2] dt/bindings: exynos-rng: Move dt binding documentation to bindings/crypto

2017-08-23 Thread Krzysztof Kozlowski 
On Wed, Aug 23, 2017 at 08:34:43PM +0530, PrasannaKumar Muralidharan wrote:
> Samsung exynos PRNG driver is using crypto framework instead of
> hw_random framework. So move the devicetree binding to crypto folder.
> 
> Signed-off-by: PrasannaKumar Muralidharan 
> ---
> Changes in v2:
> * Modify MAINTAINERS file to reflect file rename
> 
>  .../devicetree/bindings/{rng => crypto}/samsung,exynos-rng4.txt | 0
>  MAINTAINERS | 2 
> +-
>  2 files changed, 1 insertion(+), 1 deletion(-)
>  rename Documentation/devicetree/bindings/{rng => 
> crypto}/samsung,exynos-rng4.txt (100%)
> 

Patch is okay but CC list is still incomplete. I do not know how you
could get Mauro for this patch... just use get_maintainers.pl.

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

Re: [PATCH v2 3/4] crypto: jz4780-rng: Add RNG node to jz4780.dtsi

2017-08-23 Thread Ralf Baechle 
On Wed, Aug 23, 2017 at 08:27:06AM +0530, PrasannaKumar Muralidharan wrote:

> Add RNG node to jz4780 dtsi. This driver uses registers that are part of
> the register set used by Ingenic CGU driver. Make RNG node as child of
> CGU node.
> 
> Signed-off-by: PrasannaKumar Muralidharan 
> ---
> Changes in v2:
> * Add "syscon" in CGU node's compatible section
> * Make RNG child node of CGU.
> 
>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 6 +-

This barely touched arch/mips so probably should be funnelled along with
the rest of the series:

Acked-by: Ralf Baechle 

Or I can take everything.

  Ralf

Re: [PATCH v2 4/4] crypto: jz4780-rng: Enable PRNG support in CI20 defconfig

2017-08-23 Thread PrasannaKumar Muralidharan 
Hi Harvey,

On 23 August 2017 at 20:21, Harvey Hunt  wrote:
> Hi PrasannaKumar,
>
>
> On 23/08/17 15:50, PrasannaKumar Muralidharan wrote:
>>
>> Hi Harvey,
>>
>> On 23 August 2017 at 14:39, Harvey Hunt  wrote:
>>>
>>> Hi PrasannaKumar,
>>>
>>> On 23/08/17 03:57, PrasannaKumar Muralidharan wrote:


 Enable PRNG driver support in MIPS Creator CI20 default config.

 Signed-off-by: PrasannaKumar Muralidharan 
 ---
 No changes in v2

arch/mips/configs/ci20_defconfig | 5 +
1 file changed, 5 insertions(+)

 diff --git a/arch/mips/configs/ci20_defconfig
 b/arch/mips/configs/ci20_defconfig
 index b42cfa7..9f48f2c 100644
 --- a/arch/mips/configs/ci20_defconfig
 +++ b/arch/mips/configs/ci20_defconfig
 @@ -88,6 +88,11 @@ CONFIG_SERIAL_8250_RUNTIME_UARTS=5
CONFIG_SERIAL_8250_INGENIC=y
CONFIG_SERIAL_OF_PLATFORM=y
# CONFIG_HW_RANDOM is not set
 +CONFIG_CRYPTO_USER=y
 +CONFIG_CRYPTO_USER_API=y
 +CONFIG_CRYPTO_USER_API_RNG=y
 +CONFIG_CRYPTO_HW=y
 +CONFIG_CRYPTO_DEV_JZ4780_RNG=y
CONFIG_I2C=y
CONFIG_I2C_JZ4780=y
CONFIG_GPIO_SYSFS=y

>>>
>>> You need to regenerate your defconfig as it is missing CONFIG_MFD_SYSCON.
>>>
>>> Thanks,
>>>
>>> Harvey
>>
>>
>> CONFIG_MFD_SYSCON gets selected when CONFIG_CRYPTO_DEV_JZ4780_RNG is
>> selected. Please see the Kconfig changes. Given that should I add it
>> in ci20_defconfig? If it is required I will add and send a new
>> version.
>
>
> Oops, I hadn't noticed that - just skimmed the patches before my morning
> coffee. :-)
>
> It's fine as is, excuse the noise.

No issues.

>
>>
>> Thanks,
>> PrasannaKumar
>>
>
> Thanks,
>
> Harvey
>

Regards,
PrasannaKumar

[PATCH v2] dt/bindings: exynos-rng: Move dt binding documentation to bindings/crypto

2017-08-23 Thread PrasannaKumar Muralidharan 
Samsung exynos PRNG driver is using crypto framework instead of
hw_random framework. So move the devicetree binding to crypto folder.

Signed-off-by: PrasannaKumar Muralidharan 
---
Changes in v2:
* Modify MAINTAINERS file to reflect file rename

 .../devicetree/bindings/{rng => crypto}/samsung,exynos-rng4.txt | 0
 MAINTAINERS | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename Documentation/devicetree/bindings/{rng => 
crypto}/samsung,exynos-rng4.txt (100%)

diff --git a/Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt 
b/Documentation/devicetree/bindings/crypto/samsung,exynos-rng4.txt
similarity index 100%
rename from Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
rename to Documentation/devicetree/bindings/crypto/samsung,exynos-rng4.txt
diff --git a/MAINTAINERS b/MAINTAINERS
index 4d284c7..3e0b822 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11500,7 +11500,7 @@ L:  linux-crypto@vger.kernel.org
 L: linux-samsung-...@vger.kernel.org
 S: Maintained
 F: drivers/crypto/exynos-rng.c
-F: Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
+F: Documentation/devicetree/bindings/crypto/samsung,exynos-rng4.txt
 
 SAMSUNG FRAMEBUFFER DRIVER
 M: Jingoo Han 
-- 
2.10.0

Re: [PATCH v2 4/4] crypto: jz4780-rng: Enable PRNG support in CI20 defconfig

2017-08-23 Thread Harvey Hunt 

Hi PrasannaKumar,

On 23/08/17 15:50, PrasannaKumar Muralidharan wrote:

Hi Harvey,

On 23 August 2017 at 14:39, Harvey Hunt  wrote:

Hi PrasannaKumar,

On 23/08/17 03:57, PrasannaKumar Muralidharan wrote:


Enable PRNG driver support in MIPS Creator CI20 default config.

Signed-off-by: PrasannaKumar Muralidharan 
---
No changes in v2

   arch/mips/configs/ci20_defconfig | 5 +
   1 file changed, 5 insertions(+)

diff --git a/arch/mips/configs/ci20_defconfig
b/arch/mips/configs/ci20_defconfig
index b42cfa7..9f48f2c 100644
--- a/arch/mips/configs/ci20_defconfig
+++ b/arch/mips/configs/ci20_defconfig
@@ -88,6 +88,11 @@ CONFIG_SERIAL_8250_RUNTIME_UARTS=5
   CONFIG_SERIAL_8250_INGENIC=y
   CONFIG_SERIAL_OF_PLATFORM=y
   # CONFIG_HW_RANDOM is not set
+CONFIG_CRYPTO_USER=y
+CONFIG_CRYPTO_USER_API=y
+CONFIG_CRYPTO_USER_API_RNG=y
+CONFIG_CRYPTO_HW=y
+CONFIG_CRYPTO_DEV_JZ4780_RNG=y
   CONFIG_I2C=y
   CONFIG_I2C_JZ4780=y
   CONFIG_GPIO_SYSFS=y



You need to regenerate your defconfig as it is missing CONFIG_MFD_SYSCON.

Thanks,

Harvey


CONFIG_MFD_SYSCON gets selected when CONFIG_CRYPTO_DEV_JZ4780_RNG is
selected. Please see the Kconfig changes. Given that should I add it
in ci20_defconfig? If it is required I will add and send a new
version.


Oops, I hadn't noticed that - just skimmed the patches before my morning 
coffee. :-)


It's fine as is, excuse the noise.



Thanks,
PrasannaKumar



Thanks,

Harvey

Re: [PATCH v2 4/4] crypto: jz4780-rng: Enable PRNG support in CI20 defconfig

2017-08-23 Thread PrasannaKumar Muralidharan 
Hi Harvey,

On 23 August 2017 at 14:39, Harvey Hunt  wrote:
> Hi PrasannaKumar,
>
> On 23/08/17 03:57, PrasannaKumar Muralidharan wrote:
>>
>> Enable PRNG driver support in MIPS Creator CI20 default config.
>>
>> Signed-off-by: PrasannaKumar Muralidharan 
>> ---
>> No changes in v2
>>
>>   arch/mips/configs/ci20_defconfig | 5 +
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/mips/configs/ci20_defconfig
>> b/arch/mips/configs/ci20_defconfig
>> index b42cfa7..9f48f2c 100644
>> --- a/arch/mips/configs/ci20_defconfig
>> +++ b/arch/mips/configs/ci20_defconfig
>> @@ -88,6 +88,11 @@ CONFIG_SERIAL_8250_RUNTIME_UARTS=5
>>   CONFIG_SERIAL_8250_INGENIC=y
>>   CONFIG_SERIAL_OF_PLATFORM=y
>>   # CONFIG_HW_RANDOM is not set
>> +CONFIG_CRYPTO_USER=y
>> +CONFIG_CRYPTO_USER_API=y
>> +CONFIG_CRYPTO_USER_API_RNG=y
>> +CONFIG_CRYPTO_HW=y
>> +CONFIG_CRYPTO_DEV_JZ4780_RNG=y
>>   CONFIG_I2C=y
>>   CONFIG_I2C_JZ4780=y
>>   CONFIG_GPIO_SYSFS=y
>>
>
> You need to regenerate your defconfig as it is missing CONFIG_MFD_SYSCON.
>
> Thanks,
>
> Harvey

CONFIG_MFD_SYSCON gets selected when CONFIG_CRYPTO_DEV_JZ4780_RNG is
selected. Please see the Kconfig changes. Given that should I add it
in ci20_defconfig? If it is required I will add and send a new
version.

Thanks,
PrasannaKumar

Re: [PATCH 0/6] Add support for ECDSA algorithm

2017-08-23 Thread Tudor Ambarus 

Hi, Sandy,

On 08/22/2017 08:22 PM, Sandy Harris wrote:

On Tue, Aug 22, 2017 at 12:14 PM, Tudor Ambarus
 wrote:

Hi, Herbert,

On 02/02/2017 03:57 PM, Herbert Xu wrote:


Yes but RSA had an in-kernel user in the form of module signature
verification.  We don't add algorithms to the kernel without
actual users.  So this patch-set needs to come with an actual
in-kernel user of ECDSA.



ECDSA can be used by the kernel module signing facility too. Is there
any interest in using ECDSA by the kernel module signing facility?


I'd say keep it simple wherever possible; adding an algorithm should
need "is required by" not just "can be used by".

Even then, there is room for questions. In particular, whether such a
fragile algorithm should be trusted at all, let alone for signatures
on infrastructure modules that the whole OS will trust.
https://en.wikipedia.org/wiki/Elliptic_Curve_Digital_Signature_Algorithm#Security



ECDSA is a better alternative to RSA for digital signatures assuming
that you don't have implementation bugs.

ECDSA requires a much smaller key length in order to provide the same
security strength as RSA (see [1]):

security strength | RSA key length (bits) | ECDSA key lengths (bits)
   112   2048224-255
   128   3072256-383

When comparing to RSA, ECDSA promises better computational efficiency,
signature size and bandwith (see [2]).

Cheers,
ta

[1] NIST.SP.800-57pt1r4, section 5.6.1,  table 2
[2] rfc4754, rfc6979

Re: [BUGFIX PATCH v2] staging: ccree: save ciphertext for CTS IV

2017-08-23 Thread Stephan Mueller 
Am Mittwoch, 23. August 2017, 12:47:36 CEST schrieb Gilad Ben-Yossef:

Hi Gilad,
> 
> Thank you for your persistence. It is appreciated :-)
> 
> If I understood correctly what you are referring to than the buffer is not
> allocated in this code path (unless I've missed something):

Ah, that is what I was missing :-)

Thanks for clarification. No further objections ;-)

Ciao
Stephan

Re: [BUGFIX PATCH v2] staging: ccree: save ciphertext for CTS IV

2017-08-23 Thread Gilad Ben-Yossef 
On Wed, Aug 23, 2017 at 1:03 PM, Stephan Mueller  wrote:
> Am Mittwoch, 23. August 2017, 11:12:05 CEST schrieb Gilad Ben-Yossef:
>
> Hi Gilad,
>
>> The crypto API requires saving the last blocks of ciphertext
>> in req->info for use as IV for CTS mode. The ccree driver
>> was not doing this. This patch fixes that.
>>
>> The bug was manifested with cts(cbc(aes)) mode in tcrypt tests.
>>
>> Fixes: 302ef8ebb4b2 ("Add CryptoCell skcipher support")
>> Signed-off-by: Gilad Ben-Yossef 
>> ---
>>
>> Changes from v1:
>> - Free memory on error path, as pointed out by Stephan Mueller.
>>
>>  drivers/staging/ccree/ssi_cipher.c | 40
>> ++ 1 file changed, 36 insertions(+), 4
>> deletions(-)
>>
>> diff --git a/drivers/staging/ccree/ssi_cipher.c
>> b/drivers/staging/ccree/ssi_cipher.c index af9afea..8d31a93 100644
>> --- a/drivers/staging/ccree/ssi_cipher.c
>> +++ b/drivers/staging/ccree/ssi_cipher.c
>> @@ -24,6 +24,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #include "ssi_config.h"
>>  #include "ssi_driver.h"
>> @@ -697,6 +698,7 @@ static int ssi_blkcipher_complete(struct device *dev,
>>  {
>>   int completion_error = 0;
>>   u32 inflight_counter;
>> + struct ablkcipher_request *req = (struct ablkcipher_request *)areq;
>>
>>   ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);
>>
>> @@ -707,6 +709,22 @@ static int ssi_blkcipher_complete(struct device *dev,
>>   ctx_p->drvdata->inflight_counter--;
>>
>>   if (areq) {
>> + /*
>> +  * The crypto API expects us to set the req->info to the last
>> +  * ciphertext block. For encrypt, simply copy from the result.
>> +  * For decrypt, we must copy from a saved buffer since this
>> +  * could be an in-place decryption operation and the src is
>> +  * lost by this point.
>> +  */
>> + if (req_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT)  
>> {
>> + memcpy(req->info, req_ctx->backup_info, ivsize);
>> + kfree(req_ctx->backup_info);
>> + } else {
>> + scatterwalk_map_and_copy(req->info, req->dst,
>> +  (req->nbytes - ivsize),
>> +  ivsize, 0);
>
> Sorry to be persistent, but what about this code path? Here you do not free
> it, yet it is allocated.

Thank you for your persistence. It is appreciated :-)

If I understood correctly what you are referring to than the buffer is not
allocated in this code path (unless I've missed something):

The buffer is allocated in ssi_ablkcipher_decrypt() which sets
req_ctx->gen_ctx.op_type
to DRV_CRYPTO_DIRECTION_DECRYPT.

The buffer is not allocated in the parallel function
(ssi_ablkcipher_encrypt) which matches
the else clause of this if statement.



>> + }
>> +
>>   ablkcipher_request_complete(areq, completion_error);
>>   return 0;
>>   }
>> @@ -739,11 +757,13 @@ static int ssi_blkcipher_process(
>>   if (unlikely(validate_data_size(ctx_p, nbytes))) {
>>   SSI_LOG_ERR("Unsupported data size %d.\n", nbytes);
>>   crypto_tfm_set_flags(tfm, CRYPTO_TFM_RES_BAD_BLOCK_LEN);
>> - return -EINVAL;
>> + rc = -EINVAL;
>> + goto exit_process;
>>   }
>>   if (nbytes == 0) {
>>   /* No data to process is valid */
>> - return 0;
>> + rc = 0;
>> + goto exit_process;
>>   }
>>   /*For CTS in case of data size aligned to 16 use CBC mode*/
>>   if (((nbytes % AES_BLOCK_SIZE) == 0) && (ctx_p->cipher_mode ==
>> DRV_CIPHER_CBC_CTS)) { @@ -818,6 +838,9 @@ static int
>> ssi_blkcipher_process(
>>   if (cts_restore_flag != 0)
>>   ctx_p->cipher_mode = DRV_CIPHER_CBC_CTS;
>>
>> + if (rc != -EINPROGRESS)
>> + kfree(req_ctx->backup_info);
>> +
>>   return rc;
>>  }
>>
>> @@ -858,7 +881,6 @@ static int ssi_ablkcipher_encrypt(struct
>> ablkcipher_request *req) struct blkcipher_req_ctx *req_ctx =
>> ablkcipher_request_ctx(req); unsigned int ivsize =
>> crypto_ablkcipher_ivsize(ablk_tfm);
>>
>> - req_ctx->backup_info = req->info;
>>   req_ctx->is_giv = false;
>>
>>   return ssi_blkcipher_process(tfm, req_ctx, req->dst, req->src,
>> req->nbytes, req->info, ivsize, (void *)req, DRV_CRYPTO_DIRECTION_ENCRYPT);
>> @@ -871,8 +893,18 @@ static int ssi_ablkcipher_decrypt(struct
>> ablkcipher_request *req) struct blkcipher_req_ctx *req_ctx =
>> ablkcipher_request_ctx(req); unsigned int ivsize =
>> crypto_ablkcipher_ivsize(ablk_tfm);
>>
>> - req_ctx->backup_info = req->info;
>> + /*
>> +  * Allocate and save the last IV sized bytes of the source, which will
>> +  * be lost in case of in-place decryption and might be needed for CTS.
>> +

Re: [BUGFIX PATCH v2] staging: ccree: save ciphertext for CTS IV

2017-08-23 Thread Stephan Mueller 
Am Mittwoch, 23. August 2017, 11:12:05 CEST schrieb Gilad Ben-Yossef:

Hi Gilad,

> The crypto API requires saving the last blocks of ciphertext
> in req->info for use as IV for CTS mode. The ccree driver
> was not doing this. This patch fixes that.
> 
> The bug was manifested with cts(cbc(aes)) mode in tcrypt tests.
> 
> Fixes: 302ef8ebb4b2 ("Add CryptoCell skcipher support")
> Signed-off-by: Gilad Ben-Yossef 
> ---
> 
> Changes from v1:
> - Free memory on error path, as pointed out by Stephan Mueller.
> 
>  drivers/staging/ccree/ssi_cipher.c | 40
> ++ 1 file changed, 36 insertions(+), 4
> deletions(-)
> 
> diff --git a/drivers/staging/ccree/ssi_cipher.c
> b/drivers/staging/ccree/ssi_cipher.c index af9afea..8d31a93 100644
> --- a/drivers/staging/ccree/ssi_cipher.c
> +++ b/drivers/staging/ccree/ssi_cipher.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include "ssi_config.h"
>  #include "ssi_driver.h"
> @@ -697,6 +698,7 @@ static int ssi_blkcipher_complete(struct device *dev,
>  {
>   int completion_error = 0;
>   u32 inflight_counter;
> + struct ablkcipher_request *req = (struct ablkcipher_request *)areq;
> 
>   ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);
> 
> @@ -707,6 +709,22 @@ static int ssi_blkcipher_complete(struct device *dev,
>   ctx_p->drvdata->inflight_counter--;
> 
>   if (areq) {
> + /*
> +  * The crypto API expects us to set the req->info to the last
> +  * ciphertext block. For encrypt, simply copy from the result.
> +  * For decrypt, we must copy from a saved buffer since this
> +  * could be an in-place decryption operation and the src is
> +  * lost by this point.
> +  */
> + if (req_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT)  {
> + memcpy(req->info, req_ctx->backup_info, ivsize);
> + kfree(req_ctx->backup_info);
> + } else {
> + scatterwalk_map_and_copy(req->info, req->dst,
> +  (req->nbytes - ivsize),
> +  ivsize, 0);

Sorry to be persistent, but what about this code path? Here you do not free 
it, yet it is allocated.
> + }
> +
>   ablkcipher_request_complete(areq, completion_error);
>   return 0;
>   }
> @@ -739,11 +757,13 @@ static int ssi_blkcipher_process(
>   if (unlikely(validate_data_size(ctx_p, nbytes))) {
>   SSI_LOG_ERR("Unsupported data size %d.\n", nbytes);
>   crypto_tfm_set_flags(tfm, CRYPTO_TFM_RES_BAD_BLOCK_LEN);
> - return -EINVAL;
> + rc = -EINVAL;
> + goto exit_process;
>   }
>   if (nbytes == 0) {
>   /* No data to process is valid */
> - return 0;
> + rc = 0;
> + goto exit_process;
>   }
>   /*For CTS in case of data size aligned to 16 use CBC mode*/
>   if (((nbytes % AES_BLOCK_SIZE) == 0) && (ctx_p->cipher_mode ==
> DRV_CIPHER_CBC_CTS)) { @@ -818,6 +838,9 @@ static int
> ssi_blkcipher_process(
>   if (cts_restore_flag != 0)
>   ctx_p->cipher_mode = DRV_CIPHER_CBC_CTS;
> 
> + if (rc != -EINPROGRESS)
> + kfree(req_ctx->backup_info);
> +
>   return rc;
>  }
> 
> @@ -858,7 +881,6 @@ static int ssi_ablkcipher_encrypt(struct
> ablkcipher_request *req) struct blkcipher_req_ctx *req_ctx =
> ablkcipher_request_ctx(req); unsigned int ivsize =
> crypto_ablkcipher_ivsize(ablk_tfm);
> 
> - req_ctx->backup_info = req->info;
>   req_ctx->is_giv = false;
> 
>   return ssi_blkcipher_process(tfm, req_ctx, req->dst, req->src,
> req->nbytes, req->info, ivsize, (void *)req, DRV_CRYPTO_DIRECTION_ENCRYPT);
> @@ -871,8 +893,18 @@ static int ssi_ablkcipher_decrypt(struct
> ablkcipher_request *req) struct blkcipher_req_ctx *req_ctx =
> ablkcipher_request_ctx(req); unsigned int ivsize =
> crypto_ablkcipher_ivsize(ablk_tfm);
> 
> - req_ctx->backup_info = req->info;
> + /*
> +  * Allocate and save the last IV sized bytes of the source, which will
> +  * be lost in case of in-place decryption and might be needed for CTS.
> +  */
> + req_ctx->backup_info = kmalloc(ivsize, GFP_KERNEL);
> + if (!req_ctx->backup_info)
> + return -ENOMEM;
> +
> + scatterwalk_map_and_copy(req_ctx->backup_info, req->src,
> +  (req->nbytes - ivsize), ivsize, 0);
>   req_ctx->is_giv = false;
> +
>   return ssi_blkcipher_process(tfm, req_ctx, req->dst, req->src,
> req->nbytes, req->info, ivsize, (void *)req, DRV_CRYPTO_DIRECTION_DECRYPT);
> }



Ciao
Stephan

[BUGFIX PATCH v2] staging: ccree: save ciphertext for CTS IV

2017-08-23 Thread Gilad Ben-Yossef 
The crypto API requires saving the last blocks of ciphertext
in req->info for use as IV for CTS mode. The ccree driver
was not doing this. This patch fixes that.

The bug was manifested with cts(cbc(aes)) mode in tcrypt tests.

Fixes: 302ef8ebb4b2 ("Add CryptoCell skcipher support")
Signed-off-by: Gilad Ben-Yossef 
---

Changes from v1:
- Free memory on error path, as pointed out by Stephan Mueller.

 drivers/staging/ccree/ssi_cipher.c | 40 ++
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index af9afea..8d31a93 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "ssi_config.h"
 #include "ssi_driver.h"
@@ -697,6 +698,7 @@ static int ssi_blkcipher_complete(struct device *dev,
 {
int completion_error = 0;
u32 inflight_counter;
+   struct ablkcipher_request *req = (struct ablkcipher_request *)areq;
 
ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);
 
@@ -707,6 +709,22 @@ static int ssi_blkcipher_complete(struct device *dev,
ctx_p->drvdata->inflight_counter--;
 
if (areq) {
+   /*
+* The crypto API expects us to set the req->info to the last
+* ciphertext block. For encrypt, simply copy from the result.
+* For decrypt, we must copy from a saved buffer since this
+* could be an in-place decryption operation and the src is
+* lost by this point.
+*/
+   if (req_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT)  {
+   memcpy(req->info, req_ctx->backup_info, ivsize);
+   kfree(req_ctx->backup_info);
+   } else {
+   scatterwalk_map_and_copy(req->info, req->dst,
+(req->nbytes - ivsize),
+ivsize, 0);
+   }
+
ablkcipher_request_complete(areq, completion_error);
return 0;
}
@@ -739,11 +757,13 @@ static int ssi_blkcipher_process(
if (unlikely(validate_data_size(ctx_p, nbytes))) {
SSI_LOG_ERR("Unsupported data size %d.\n", nbytes);
crypto_tfm_set_flags(tfm, CRYPTO_TFM_RES_BAD_BLOCK_LEN);
-   return -EINVAL;
+   rc = -EINVAL;
+   goto exit_process;
}
if (nbytes == 0) {
/* No data to process is valid */
-   return 0;
+   rc = 0;
+   goto exit_process;
}
/*For CTS in case of data size aligned to 16 use CBC mode*/
if (((nbytes % AES_BLOCK_SIZE) == 0) && (ctx_p->cipher_mode == 
DRV_CIPHER_CBC_CTS)) {
@@ -818,6 +838,9 @@ static int ssi_blkcipher_process(
if (cts_restore_flag != 0)
ctx_p->cipher_mode = DRV_CIPHER_CBC_CTS;
 
+   if (rc != -EINPROGRESS)
+   kfree(req_ctx->backup_info);
+
return rc;
 }
 
@@ -858,7 +881,6 @@ static int ssi_ablkcipher_encrypt(struct ablkcipher_request 
*req)
struct blkcipher_req_ctx *req_ctx = ablkcipher_request_ctx(req);
unsigned int ivsize = crypto_ablkcipher_ivsize(ablk_tfm);
 
-   req_ctx->backup_info = req->info;
req_ctx->is_giv = false;
 
return ssi_blkcipher_process(tfm, req_ctx, req->dst, req->src, 
req->nbytes, req->info, ivsize, (void *)req, DRV_CRYPTO_DIRECTION_ENCRYPT);
@@ -871,8 +893,18 @@ static int ssi_ablkcipher_decrypt(struct 
ablkcipher_request *req)
struct blkcipher_req_ctx *req_ctx = ablkcipher_request_ctx(req);
unsigned int ivsize = crypto_ablkcipher_ivsize(ablk_tfm);
 
-   req_ctx->backup_info = req->info;
+   /*
+* Allocate and save the last IV sized bytes of the source, which will
+* be lost in case of in-place decryption and might be needed for CTS.
+*/
+   req_ctx->backup_info = kmalloc(ivsize, GFP_KERNEL);
+   if (!req_ctx->backup_info)
+   return -ENOMEM;
+
+   scatterwalk_map_and_copy(req_ctx->backup_info, req->src,
+(req->nbytes - ivsize), ivsize, 0);
req_ctx->is_giv = false;
+
return ssi_blkcipher_process(tfm, req_ctx, req->dst, req->src, 
req->nbytes, req->info, ivsize, (void *)req, DRV_CRYPTO_DIRECTION_DECRYPT);
 }
 
-- 
2.1.4

Re: [PATCH v2 4/4] crypto: jz4780-rng: Enable PRNG support in CI20 defconfig

2017-08-23 Thread Harvey Hunt 

Hi PrasannaKumar,

On 23/08/17 03:57, PrasannaKumar Muralidharan wrote:

Enable PRNG driver support in MIPS Creator CI20 default config.

Signed-off-by: PrasannaKumar Muralidharan 
---
No changes in v2

  arch/mips/configs/ci20_defconfig | 5 +
  1 file changed, 5 insertions(+)

diff --git a/arch/mips/configs/ci20_defconfig b/arch/mips/configs/ci20_defconfig
index b42cfa7..9f48f2c 100644
--- a/arch/mips/configs/ci20_defconfig
+++ b/arch/mips/configs/ci20_defconfig
@@ -88,6 +88,11 @@ CONFIG_SERIAL_8250_RUNTIME_UARTS=5
  CONFIG_SERIAL_8250_INGENIC=y
  CONFIG_SERIAL_OF_PLATFORM=y
  # CONFIG_HW_RANDOM is not set
+CONFIG_CRYPTO_USER=y
+CONFIG_CRYPTO_USER_API=y
+CONFIG_CRYPTO_USER_API_RNG=y
+CONFIG_CRYPTO_HW=y
+CONFIG_CRYPTO_DEV_JZ4780_RNG=y
  CONFIG_I2C=y
  CONFIG_I2C_JZ4780=y
  CONFIG_GPIO_SYSFS=y



You need to regenerate your defconfig as it is missing CONFIG_MFD_SYSCON.

Thanks,

Harvey

Re: [BUGFIX PATCH] staging: ccree: save ciphertext for CTS IV

2017-08-23 Thread Gilad Ben-Yossef 
On Wed, Aug 23, 2017 at 10:47 AM, Stephan Mueller  wrote:
> Am Mittwoch, 23. August 2017, 09:41:11 CEST schrieb Gilad Ben-Yossef:
>
> Hi Gilad,
>
>>
>>   if (areq) {
>> + /*
>> +  * The crypto API expects us to set the req->info to the last
>> +  * ciphertext block. For encrypt, simply copy from the result.
>> +  * For decrypt, we must copy from a saved buffer since this
>> +  * could be an in-place decryption operation and the src is
>> +  * lost by this point.
>> +  */
>> + if (req_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT)  
>> {
>> + memcpy(req->info, req_ctx->backup_info, ivsize);
>> + kfree(req_ctx->backup_info);
>> + } else {
>> + scatterwalk_map_and_copy(req->info, req->dst,
>> +  (req->nbytes - ivsize),
>> +  ivsize, 0);
>> + }
>> +
>>   ablkcipher_request_complete(areq, completion_error);
>>   return 0;
>>   }
>> @@ -858,7 +876,6 @@ static int ssi_ablkcipher_encrypt(struct
>> ablkcipher_request *req) struct blkcipher_req_ctx *req_ctx =
>> ablkcipher_request_ctx(req); unsigned int ivsize =
>> crypto_ablkcipher_ivsize(ablk_tfm);
>>
>> - req_ctx->backup_info = req->info;
>>   req_ctx->is_giv = false;
>>
>>   return ssi_blkcipher_process(tfm, req_ctx, req->dst, req->src,
>> req->nbytes, req->info, ivsize, (void *)req, DRV_CRYPTO_DIRECTION_ENCRYPT);
>> @@ -871,8 +888,18 @@ static int ssi_ablkcipher_decrypt(struct
>> ablkcipher_request *req) struct blkcipher_req_ctx *req_ctx =
>> ablkcipher_request_ctx(req); unsigned int ivsize =
>> crypto_ablkcipher_ivsize(ablk_tfm);
>>
>> - req_ctx->backup_info = req->info;
>> + /*
>> +  * Allocate and save the last IV sized bytes of the source, which will
>> +  * be lost in case of in-place decryption and might be needed for CTS.
>> +  */
>> + req_ctx->backup_info = kmalloc(ivsize, GFP_KERNEL);
>> + if (!req_ctx->backup_info)
>> + return -ENOMEM;
>
> Are you sure you do not add a memleak here? You seem to unconditionally
> allocate memory but conditionally free it.

You are right. I've missed freeing the memory on error code paths.
Thank you for catching that.
The code involved is badly need of a re-write but I wanted to get the
obvious bug fixed first.
I will send a revised patch

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: [BUGFIX PATCH] staging: ccree: save ciphertext for CTS IV

2017-08-23 Thread Stephan Mueller 
Am Mittwoch, 23. August 2017, 09:41:11 CEST schrieb Gilad Ben-Yossef:

Hi Gilad,

> 
>   if (areq) {
> + /*
> +  * The crypto API expects us to set the req->info to the last
> +  * ciphertext block. For encrypt, simply copy from the result.
> +  * For decrypt, we must copy from a saved buffer since this
> +  * could be an in-place decryption operation and the src is
> +  * lost by this point.
> +  */
> + if (req_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT)  {
> + memcpy(req->info, req_ctx->backup_info, ivsize);
> + kfree(req_ctx->backup_info);
> + } else {
> + scatterwalk_map_and_copy(req->info, req->dst,
> +  (req->nbytes - ivsize),
> +  ivsize, 0);
> + }
> +
>   ablkcipher_request_complete(areq, completion_error);
>   return 0;
>   }
> @@ -858,7 +876,6 @@ static int ssi_ablkcipher_encrypt(struct
> ablkcipher_request *req) struct blkcipher_req_ctx *req_ctx =
> ablkcipher_request_ctx(req); unsigned int ivsize =
> crypto_ablkcipher_ivsize(ablk_tfm);
> 
> - req_ctx->backup_info = req->info;
>   req_ctx->is_giv = false;
> 
>   return ssi_blkcipher_process(tfm, req_ctx, req->dst, req->src,
> req->nbytes, req->info, ivsize, (void *)req, DRV_CRYPTO_DIRECTION_ENCRYPT);
> @@ -871,8 +888,18 @@ static int ssi_ablkcipher_decrypt(struct
> ablkcipher_request *req) struct blkcipher_req_ctx *req_ctx =
> ablkcipher_request_ctx(req); unsigned int ivsize =
> crypto_ablkcipher_ivsize(ablk_tfm);
> 
> - req_ctx->backup_info = req->info;
> + /*
> +  * Allocate and save the last IV sized bytes of the source, which will
> +  * be lost in case of in-place decryption and might be needed for CTS.
> +  */
> + req_ctx->backup_info = kmalloc(ivsize, GFP_KERNEL);
> + if (!req_ctx->backup_info)
> + return -ENOMEM;

Are you sure you do not add a memleak here? You seem to unconditionally 
allocate memory but conditionally free it.

Ciao
Stephan

[BUGFIX PATCH] staging: ccree: save ciphertext for CTS IV

2017-08-23 Thread Gilad Ben-Yossef 
The crypto API requires saving the last blocks of ciphertext
in req->info for use as IV for CTS mode. The ccree driver
was not doing this. This patch fixes that.

The bug was manifested with cts(cbc(aes)) mode in tcrypt tests.

Fixes: 302ef8ebb4b2 ("Add CryptoCell skcipher support")
Signed-off-by: Gilad Ben-Yossef 
---
 drivers/staging/ccree/ssi_cipher.c | 31 +--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index af9afea..01011a2 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "ssi_config.h"
 #include "ssi_driver.h"
@@ -697,6 +698,7 @@ static int ssi_blkcipher_complete(struct device *dev,
 {
int completion_error = 0;
u32 inflight_counter;
+   struct ablkcipher_request *req = (struct ablkcipher_request *)areq;
 
ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);
 
@@ -707,6 +709,22 @@ static int ssi_blkcipher_complete(struct device *dev,
ctx_p->drvdata->inflight_counter--;
 
if (areq) {
+   /*
+* The crypto API expects us to set the req->info to the last
+* ciphertext block. For encrypt, simply copy from the result.
+* For decrypt, we must copy from a saved buffer since this
+* could be an in-place decryption operation and the src is
+* lost by this point.
+*/
+   if (req_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT)  {
+   memcpy(req->info, req_ctx->backup_info, ivsize);
+   kfree(req_ctx->backup_info);
+   } else {
+   scatterwalk_map_and_copy(req->info, req->dst,
+(req->nbytes - ivsize),
+ivsize, 0);
+   }
+
ablkcipher_request_complete(areq, completion_error);
return 0;
}
@@ -858,7 +876,6 @@ static int ssi_ablkcipher_encrypt(struct ablkcipher_request 
*req)
struct blkcipher_req_ctx *req_ctx = ablkcipher_request_ctx(req);
unsigned int ivsize = crypto_ablkcipher_ivsize(ablk_tfm);
 
-   req_ctx->backup_info = req->info;
req_ctx->is_giv = false;
 
return ssi_blkcipher_process(tfm, req_ctx, req->dst, req->src, 
req->nbytes, req->info, ivsize, (void *)req, DRV_CRYPTO_DIRECTION_ENCRYPT);
@@ -871,8 +888,18 @@ static int ssi_ablkcipher_decrypt(struct 
ablkcipher_request *req)
struct blkcipher_req_ctx *req_ctx = ablkcipher_request_ctx(req);
unsigned int ivsize = crypto_ablkcipher_ivsize(ablk_tfm);
 
-   req_ctx->backup_info = req->info;
+   /*
+* Allocate and save the last IV sized bytes of the source, which will
+* be lost in case of in-place decryption and might be needed for CTS.
+*/
+   req_ctx->backup_info = kmalloc(ivsize, GFP_KERNEL);
+   if (!req_ctx->backup_info)
+   return -ENOMEM;
+
+   scatterwalk_map_and_copy(req_ctx->backup_info, req->src,
+(req->nbytes - ivsize), ivsize, 0);
req_ctx->is_giv = false;
+
return ssi_blkcipher_process(tfm, req_ctx, req->dst, req->src, 
req->nbytes, req->info, ivsize, (void *)req, DRV_CRYPTO_DIRECTION_DECRYPT);
 }
 
-- 
2.1.4