Re: [PATCH] crypto: caam - properly set IV after {en,de}crypt

2017-09-06 Thread Gilad Ben-Yossef 
On Tue, Sep 5, 2017 at 6:33 PM, Horia Geantă  wrote:
> On 8/14/2017 10:59 AM, Gilad Ben-Yossef wrote:
>> Hi,
>>
>> On Thu, Jun 29, 2017 at 1:19 PM, Horia Geantă  wrote:
>>> On 6/28/2017 4:42 PM, Horia Geantă wrote:
 On 6/28/2017 4:27 PM, David Gstir wrote:
> Certain cipher modes like CTS expect the IV (req->info) of
> ablkcipher_request (or equivalently req->iv of skcipher_request) to
> contain the last ciphertext block when the {en,de}crypt operation is done.
> This is currently not the case for the CAAM driver which in turn breaks
> e.g. cts(cbc(aes)) when the CAAM driver is enabled.
>
> This patch fixes the CAAM driver to properly set the IV after the
> {en,de}crypt operation of ablkcipher finishes.
>
> This issue was revealed by the changes in the SW CTS mode in commit
> 0605c41cc53ca ("crypto: cts - Convert to skcipher")
>
> Cc:  # 4.8+
> Signed-off-by: David Gstir 
 Reviewed-by: Horia Geantă 

>>> Btw, instead of updating the IV in SW, CAAM engine could be programmed
>>> to do it - by saving the Context Register of the AES accelerator.
>>>
>>> Unfortunately this would require changes in quite a few places: shared
>>> descriptor, HW S/G generation logic, IV dma (un)mapping and maybe others.
>>>
>>> So it's better to have this fix now (which, considering size, is
>>> appropriate for -stable) and later, if needed, offload IV updating in HW.
>>>
>>
>> My apologies for reviving this thread from the dead, but doesn't the patch 
>> fail
>> for in-place decryption since we are copying from req->dst after
>> the operation is done, and therefore it no longer contains the ciphertext?
>>
> You are right, thanks! Will follow up with a fix.
> Though cts(cbc(aes)) in particular is working, see below.
>
>> I'm asking since I ran into a similar issue in the ccree driver and thought
>> to deploy a similar fix but could not convince myself why this works.
>>
> IIUC cts(cbc(aes)) in-place decryption (with cbc(aes) offloaded to CAAM
> engine) works since SW implementation of cts, when performing the
> ciphertext stealing phase in cts_cbc_decrypt() does not use req->iv, but
> a previously value, saved before staring decryption in crypto_cts_decrypt():
>
> if (cbc_blocks <= 1)
> memcpy(space, req->iv, bsize);
> else
> scatterwalk_map_and_copy(space, req->src, offset - 2 * bsize,
>  bsize, 0);
>

Is that not a performance bug in software CTS than? I mean all those
transformation
drivers doing that extra copy and possibly malloc and free to save the
data for the info
and than have the CTS implementation ignore that and do its own memory copy?

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

[RFC] tpm: Register RNG device only on tpm chip init

2017-09-06 Thread PrasannaKumar Muralidharan 
RNG device is registered as soon as tpm-rng module is loaded even if
there are no TPM chip available. Call hwrng_register once tpm chip has
registered.

Signed-off-by: PrasannaKumar Muralidharan 
---
 drivers/char/hw_random/tpm-rng.c | 50 
 drivers/char/tpm/tpm-chip.c  | 24 +++
 drivers/char/tpm/tpm.h   |  3 +++
 3 files changed, 27 insertions(+), 50 deletions(-)
 delete mode 100644 drivers/char/hw_random/tpm-rng.c

diff --git a/drivers/char/hw_random/tpm-rng.c b/drivers/char/hw_random/tpm-rng.c
deleted file mode 100644
index d6d4482..000
--- a/drivers/char/hw_random/tpm-rng.c
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- * Copyright (C) 2012 Kent Yoder IBM Corporation
- *
- * HWRNG interfaces to pull RNG data from a TPM
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
- */
-
-#include 
-#include 
-#include 
-
-#define MODULE_NAME "tpm-rng"
-
-static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
-{
-   return tpm_get_random(TPM_ANY_NUM, data, max);
-}
-
-static struct hwrng tpm_rng = {
-   .name = MODULE_NAME,
-   .read = tpm_rng_read,
-};
-
-static int __init rng_init(void)
-{
-   return hwrng_register(_rng);
-}
-module_init(rng_init);
-
-static void __exit rng_exit(void)
-{
-   hwrng_unregister(_rng);
-}
-module_exit(rng_exit);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Kent Yoder ");
-MODULE_DESCRIPTION("RNG driver for TPM devices");
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 0eca20c..bf9c2ad 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "tpm.h"
 #include "tpm_eventlog.h"
 
@@ -387,6 +388,15 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
 
return 0;
 }
+
+static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
+{
+   int chip_num = 0;
+
+   kstrtoint(>name[8], 10, _num);
+   return tpm_get_random(chip_num, data, max);
+}
+
 /*
  * tpm_chip_register() - create a character device for the TPM chip
  * @chip: TPM chip to use.
@@ -401,6 +411,7 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
 int tpm_chip_register(struct tpm_chip *chip)
 {
int rc;
+   char *tpm_rng_name;
 
if (chip->ops->flags & TPM_OPS_AUTO_STARTUP) {
if (chip->flags & TPM_CHIP_FLAG_TPM2)
@@ -431,6 +442,14 @@ int tpm_chip_register(struct tpm_chip *chip)
return rc;
}
 
+   tpm_rng_name = kmalloc(64, GFP_KERNEL);
+
+   if (tpm_rng_name) {
+   sprintf(tpm_rng_name, "tpm-rng-%d", chip->dev_num);
+   tpm_rng.name = tpm_rng_name;
+   tpm_rng.read = tpm_rng_read;
+   hwrng_register(_rng);
+   }
return 0;
 }
 EXPORT_SYMBOL_GPL(tpm_chip_register);
@@ -450,6 +469,11 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
  */
 void tpm_chip_unregister(struct tpm_chip *chip)
 {
+   if (chip->tpm_rng.name[0] != 0) {
+   hwrng_unregister(_rng);
+   kfree(chip->tpm_rng.name);
+   chip->tpm_rng.name[0] = 0;
+   }
tpm_del_legacy_sysfs(chip);
tpm_bios_log_teardown(chip);
if (chip->flags & TPM_CHIP_FLAG_TPM2)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index b50e92f..ca042f7 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -210,6 +211,8 @@ struct tpm_chip {
int dev_num;/* /dev/tpm# */
unsigned long is_open;  /* only one allowed */
 
+   struct hwrng tpm_rng;
+
struct mutex tpm_mutex; /* tpm is processing */
 
unsigned long timeout_a; /* jiffies */
-- 
2.10.0

[PATCH] crypto: axis - hide an unused variable

2017-09-06 Thread Arnd Bergmann 
Without CONFIG_DEBUG_FS, we get a harmless warning:

drivers/crypto/axis/artpec6_crypto.c:352:23: error: 'dbgfs_root' defined but 
not used [-Werror=unused-variable]

This moves it into the #ifdef that hides the only user.

Fixes: a21eb94fc4d3 ("crypto: axis - add ARTPEC-6/7 crypto accelerator driver")
Acked-by: Lars Persson 
Signed-off-by: Arnd Bergmann 
---
Originally sent on August 25, but hasn't made it into linux-next
yet. Resending it in case it got lost, sorry for any duplicates
if this was still pending.
---
 drivers/crypto/axis/artpec6_crypto.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/axis/artpec6_crypto.c 
b/drivers/crypto/axis/artpec6_crypto.c
index d9fbbf01062b..0f9754e07719 100644
--- a/drivers/crypto/axis/artpec6_crypto.c
+++ b/drivers/crypto/axis/artpec6_crypto.c
@@ -349,8 +349,6 @@ struct artpec6_crypto_aead_req_ctx {
 /* The crypto framework makes it hard to avoid this global. */
 static struct device *artpec6_crypto_dev;
 
-static struct dentry *dbgfs_root;
-
 #ifdef CONFIG_FAULT_INJECTION
 static DECLARE_FAULT_ATTR(artpec6_crypto_fail_status_read);
 static DECLARE_FAULT_ATTR(artpec6_crypto_fail_dma_array_full);
@@ -2984,6 +2982,8 @@ struct dbgfs_u32 {
char *desc;
 };
 
+static struct dentry *dbgfs_root;
+
 static void artpec6_crypto_init_debugfs(void)
 {
dbgfs_root = debugfs_create_dir("artpec6_crypto", NULL);
-- 
2.9.0

Re: [PATCH 8/8] staging: ccree: remove BUG macro usage

2017-09-06 Thread Dan Carpenter 
On Sun, Sep 03, 2017 at 11:56:50AM +0300, Gilad Ben-Yossef wrote:
> @@ -1154,7 +1150,8 @@ static inline int ssi_buffer_mgr_aead_chain_data(
>   //if have reached the end of the sgl, then this is unexpected
>   if (!areq_ctx->src_sgl) {
>   SSI_LOG_ERR("reached end of sg list. unexpected\n");
> - BUG();
> + return -EINVAL;
> + goto chain_data_exit;

You've got a direct return followed by a goto.

It's a do-nothing goto that just returns rc.  I hate those.  I've tried
to review locking bugs to see if single returns prevent future
programmers from introducing new error paths which don't unlock.  They
don't really help...

regards,
dan carpenter

Re: [PATCH] crypto: authenc - cryptlen must be at least AAD len

2017-09-06 Thread Stephan Müller 
Am Mittwoch, 6. September 2017, 21:22:44 CEST schrieb Stephan Müller:

Hi Herbert,

> With AF_ALG, AAD len and cryptlen can be set freely by unprivileged
> user space. The cipher implementation must therefore validate the input
> data for sanity. For AEAD ciphers, this implies that cryptlen must be
> at least as large as AAD size.
> 
> This fixes a kernel crash that can be triggered via AF_ALG detected by
> the fuzzing test implemented with libkcapi.

What is your opinion: should this check be rather added to crypto_aead_encrypt 
(similar to a sanity check found in crypto_aead_decrypt)?

Ciao
Stephan

Re: [RFC Part2 PATCH v3 02/26] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-06 Thread Brijesh Singh 

Hi Boris,


On 09/06/2017 12:00 PM, Borislav Petkov wrote:

...


--
|diff --git a/drivers/crypto/ccp/sp-dev.c b/drivers/crypto/ccp/sp-dev.c
|index a017233..d263ba4 100644
|--- a/drivers/crypto/ccp/sp-dev.c
|+++ b/drivers/crypto/ccp/sp-dev.c
--

What tree is that against? In any case, it doesn't apply here.


This RFC is based on tip/master commit : 22db3de (Merge branch 'x86/mm').




This bit of my struggle -- tip/master is not in sync with cryptodev-2.6 [1].
In order to expand the CCP driver we need the following commits from the
cryptodev-2.6

57de3aefb73f crypto: ccp - remove ccp_present() check from device initialize
d0ebbc0c407a crypto: ccp - rename ccp driver initialize files as sp device
f4d18d656f88 crypto: ccp - Abstract interrupt registeration
720419f01832 crypto: ccp - Introduce the AMD Secure Processor device
970e8303cb8d crypto: ccp - Use devres interface to allocate PCI/iomap and 
cleanup

I cherry-picked these patches into tip/master before starting the SEV work.

Since these patches were already reviewed and accepted hence I did not include 
it
in my RFC series. I am not sure what is best way to handle it. Should I include
these patches in the series ? or just mention them in cover letter ? I am 
looking
for suggestions on how to best communicate it. thanks

[1] https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/

My staging tree on github contain these precursor patches.



$ git show 22db3de
fatal: ambiguous argument '22db3de': unknown revision or path not in the 
working tree.

Do you have updated version of the series which you can send out?


@@ -67,6 +74,10 @@ struct sp_device {
/* DMA caching attribute support */
unsigned int axcache;
  
+	/* get and set master device */

+   struct sp_device*(*get_psp_master_device)(void);
+   void(*set_psp_master_device)(struct sp_device *);


WARNING: missing space after return type
#502: FILE: drivers/crypto/ccp/sp-dev.h:79:
+   void(*set_psp_master_device)(struct sp_device *);

Don't forget to run all patches through checkpatch. Some of the warnings
make sense.

Thx.

Re: [Outreachy kernel] [PATCH v2] Staging: ccree: ssi_cipher.c: Make comment more comprehensible.

2017-09-06 Thread Julia Lawall 


On Thu, 7 Sep 2017, Srishti Sharma wrote:

> Edited comment to make it more comprehensible.
>
> Signed-off-by: Srishti Sharma 
> ---
> Changes in v2:
>  - Make comment more comprehensible , instead of just correcting typos.
>  drivers/staging/ccree/ssi_cipher.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/ccree/ssi_cipher.c 
> b/drivers/staging/ccree/ssi_cipher.c
> index 99232b2..fec2faa 100644
> --- a/drivers/staging/ccree/ssi_cipher.c
> +++ b/drivers/staging/ccree/ssi_cipher.c
> @@ -702,7 +702,9 @@ static int ssi_blkcipher_complete(struct device *dev,
>
>   ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);
>
> - /*Set the inflight counter value to local variable*/
> + /*
> +  * Save inflight_counter in a local variable.
> +  */

This is the format for multiline comments.  Your comments is only one
line.  You just need to add spaces around the text, as compared to the
original version.

julia

>   inflight_counter =  ctx_p->drvdata->inflight_counter;
>   /*Decrease the inflight counter*/
>   if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter > 0)
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1504731390-9536-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

[PATCH v3] Staging: ccree: ssi_cipher.c: Remove unused variable.

2017-09-06 Thread Srishti Sharma 
Remove local variable inflight_counter ,as it is never used.

Signed-off-by: Srishti Sharma 
---
Changes in v3:
 - There was no longer a need to make the comment more comprehensible as
   I have deleted the variable associated with it because it is unused .
 drivers/staging/ccree/ssi_cipher.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index fec2faa..609ebe4 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -702,10 +702,7 @@ static int ssi_blkcipher_complete(struct device *dev,

ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);

-   /*
-* Save inflight_counter in a local variable.
-*/
-   inflight_counter =  ctx_p->drvdata->inflight_counter;
+
/*Decrease the inflight counter*/
if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter > 0)
ctx_p->drvdata->inflight_counter--;
--
2.7.4

Re: [Outreachy kernel] Re: [PATCH] Staging: ccree: ssi_cipher.c: Correct spelling mistake.

2017-09-06 Thread Julia Lawall 


On Wed, 6 Sep 2017, Srishti Sharma wrote:

>
>
> On Thursday, September 7, 2017 at 12:54:49 AM UTC+5:30, Srishti Sharma
> wrote:
>   Correct spelling of counter in comment .
>
>   Signed-off-by: Srishti Sharma 
>   ---
>    drivers/staging/ccree/ssi_cipher.c | 2 +-
>    1 file changed, 1 insertion(+), 1 deletion(-)
>
>   diff --git a/drivers/staging/ccree/ssi_cipher.c
>   b/drivers/staging/ccree/ssi_cipher.c
>   index 8d31a93..99232b2 100644
>   --- a/drivers/staging/ccree/ssi_cipher.c
>   +++ b/drivers/staging/ccree/ssi_cipher.c
>   @@ -702,7 +702,7 @@ static int ssi_blkcipher_complete(struct
>   device *dev,
>
>    ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx,
>   ivsize, src, dst);
>
>   -/*Set the inflight couter value to local variable*/
>   +/*Set the inflight counter value to local variable*/
>    inflight_counter =  ctx_p->drvdata->inflight_counter;
>    /*Decrease the inflight counter*/
>    if (ctx_p->flow_mode == BYPASS &&
>   ctx_p->drvdata->inflight_counter > 0)
>   --
>   2.7.4
>
>
>   Hey,
>
>   Can I say , /* store the value of inflight_counter variable from  driver
> private data context to a local variable */  , to make it more
> comprehensible ? 

I think it could be a bit of overkill.  The "driver private data context"
part can be seen from a quick glance at the code.  Also, it could be good
to bring out the purpose rather than just what the code does.  So "save
inflight_counter in a a local variable" could be better, because it
focuses on the idea of saving some information for later use.  Good job on
figuring out the formatting problem.

julia

>
> Regards,
> Srishti 
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web 
> visithttps://groups.google.com/d/msgid/outreachy-kernel/2b8bde9c-6e84-48c5-ab93-
> 76127f314429%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

[PATCH] crypto: authenc - cryptlen must be at least AAD len

2017-09-06 Thread Stephan Müller 
With AF_ALG, AAD len and cryptlen can be set freely by unprivileged
user space. The cipher implementation must therefore validate the input
data for sanity. For AEAD ciphers, this implies that cryptlen must be
at least as large as AAD size.

This fixes a kernel crash that can be triggered via AF_ALG detected by
the fuzzing test implemented with libkcapi.

CC: 
CC: Herbert Xu 
Signed-off-by: Stephan Mueller 
---
 crypto/authenc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/crypto/authenc.c b/crypto/authenc.c
index 875470b0e026..21e202fc32c1 100644
--- a/crypto/authenc.c
+++ b/crypto/authenc.c
@@ -209,6 +209,9 @@ static int crypto_authenc_encrypt(struct aead_request *req)
struct scatterlist *src, *dst;
int err;
 
+   if (req->assoclen > cryptlen)
+   return -EINVAL;
+
src = scatterwalk_ffwd(areq_ctx->src, req->src, req->assoclen);
dst = src;
 
-- 
2.13.5

[PATCH] Staging: ccree: ssi_cipher.c: Correct spelling mistake.

2017-09-06 Thread Srishti Sharma 
Correct spelling of counter in comment .

Signed-off-by: Srishti Sharma 
---
 drivers/staging/ccree/ssi_cipher.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index 8d31a93..99232b2 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -702,7 +702,7 @@ static int ssi_blkcipher_complete(struct device *dev,

ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);

-   /*Set the inflight couter value to local variable*/
+   /*Set the inflight counter value to local variable*/
inflight_counter =  ctx_p->drvdata->inflight_counter;
/*Decrease the inflight counter*/
if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter > 0)
--
2.7.4

Re: [Outreachy kernel] [PATCH v2] Staging: ccree: ssi_cipher.c: Make comment more comprehensible.

2017-09-06 Thread Srishti Sharma 
On Thu, Sep 7, 2017 at 2:28 AM, Julia Lawall  wrote:
>
>
> On Thu, 7 Sep 2017, Srishti Sharma wrote:
>
>> Edited comment to make it more comprehensible.
>>
>> Signed-off-by: Srishti Sharma 
>> ---
>> Changes in v2:
>>  - Make comment more comprehensible , instead of just correcting typos.
>>  drivers/staging/ccree/ssi_cipher.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/ccree/ssi_cipher.c 
>> b/drivers/staging/ccree/ssi_cipher.c
>> index 99232b2..fec2faa 100644
>> --- a/drivers/staging/ccree/ssi_cipher.c
>> +++ b/drivers/staging/ccree/ssi_cipher.c
>> @@ -702,7 +702,9 @@ static int ssi_blkcipher_complete(struct device *dev,
>>
>>   ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);
>>
>> - /*Set the inflight counter value to local variable*/
>> + /*
>> +  * Save inflight_counter in a local variable.
>> +  */
>
> This is the format for multiline comments.  Your comments is only one
> line.  You just need to add spaces around the text, as compared to the
> original version.
>
> julia
>
>>   inflight_counter =  ctx_p->drvdata->inflight_counter;
>>   /*Decrease the inflight counter*/
>>   if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter > 0)
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/outreachy-kernel/1504731390-9536-1-git-send-email-srishtishar%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>

Okay, Thanks I'll re-send it .

Regards,
Srishti

Re: [PATCH 00/12] x86/crypto: Fix RBP usage in several crypto .S files

2017-09-06 Thread Josh Poimboeuf 
On Fri, Sep 01, 2017 at 05:09:19PM -0700, Eric Biggers wrote:
> Hi Josh,
> 
> On Tue, Aug 29, 2017 at 01:05:33PM -0500, Josh Poimboeuf wrote:
> > Many of the x86 crypto functions use RBP as a temporary register.  This
> > breaks frame pointer convention, and breaks stack traces when unwinding
> > from an interrupt in the crypto code.
> > 
> > Convert most* of them to leave RBP alone.
> > 
> > These pass the crypto boot tests for me.  Any further testing would be
> > appreciated!
> > 
> > [*] There are still a few crypto files left that need fixing, but the
> > fixes weren't trivial and nobody reported unwinder warnings about
> > them yet, so I'm skipping them for now.
> > 
> > Josh Poimboeuf (12):
> >   x86/crypto: Fix RBP usage in blowfish-x86_64-asm_64.S
> >   x86/crypto: Fix RBP usage in camellia-x86_64-asm_64.S
> >   x86/crypto: Fix RBP usage in cast5-avx-x86_64-asm_64.S
> >   x86/crypto: Fix RBP usage in cast6-avx-x86_64-asm_64.S
> >   x86/crypto: Fix RBP usage in des3_ede-asm_64.S
> >   x86/crypto: Fix RBP usage in sha1_avx2_x86_64_asm.S
> >   x86/crypto: Fix RBP usage in sha1_ssse3_asm.S
> >   x86/crypto: Fix RBP usage in sha256-avx-asm.S
> >   x86/crypto: Fix RBP usage in sha256-avx2-asm.S
> >   x86/crypto: Fix RBP usage in sha256-ssse3-asm.S
> >   x86/crypto: Fix RBP usage in sha512-avx2-asm.S
> >   x86/crypto: Fix RBP usage in twofish-avx-x86_64-asm_64.S
> > 
> 
> Thanks for fixing these!  I don't have time to review these in detail, but I 
> ran
> the crypto self-tests on the affected algorithms, and they all pass.  I also
> benchmarked them before and after; the only noticable performance difference 
> was
> that sha256-avx2 and sha512-avx2 became a few percent slower.  I don't suppose
> there is a way around that?  Otherwise it's probably not a big deal.

Thanks for testing.  I might have a way to make sha256-avx2 faster, but
not sha512-avx2.  I'll let you know when I have a patch.

-- 
Josh

Re: [Outreachy kernel] [PATCH v3] Staging: ccree: ssi_cipher.c: Remove unused variable.

2017-09-06 Thread Srishti Sharma 
On Thu, Sep 7, 2017 at 2:47 AM, Julia Lawall  wrote:
>
>
> On Thu, 7 Sep 2017, Srishti Sharma wrote:
>
>> Remove local variable inflight_counter ,as it is never used.
>
> "counter ,as" -> "counter, as"
>
>> Signed-off-by: Srishti Sharma 
>> ---
>> Changes in v3:
>>  - There was no longer a need to make the comment more comprehensible as
>>I have deleted the variable associated with it because it is unused .
>>  drivers/staging/ccree/ssi_cipher.c | 5 +
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/ccree/ssi_cipher.c 
>> b/drivers/staging/ccree/ssi_cipher.c
>> index fec2faa..609ebe4 100644
>> --- a/drivers/staging/ccree/ssi_cipher.c
>> +++ b/drivers/staging/ccree/ssi_cipher.c
>> @@ -702,10 +702,7 @@ static int ssi_blkcipher_complete(struct device *dev,
>>
>>   ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);
>>
>> - /*
>> -  * Save inflight_counter in a local variable.
>> -  */
>> - inflight_counter =  ctx_p->drvdata->inflight_counter;
>> +
>
> This is a patch on your previous patch.  It should be a patch on the
> original code.  Also, don't add a new blank line.  Just remove the line
> completely.

  so , when I resend it should I version it as v3 or send as a new
patch as it is now fixing a different problem ?

 Regards,
 Srishti
>
> julia
>
>>   /*Decrease the inflight counter*/
>>   if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter > 0)
>>   ctx_p->drvdata->inflight_counter--;
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/outreachy-kernel/1504732426-9765-1-git-send-email-srishtishar%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>

Re: [Outreachy kernel] [PATCH v3] Staging: ccree: ssi_cipher.c: Remove unused variable.

2017-09-06 Thread Srishti Sharma 
On Thu, Sep 7, 2017 at 2:56 AM, Julia Lawall  wrote:
>
>
> On Thu, 7 Sep 2017, Srishti Sharma wrote:
>
>> On Thu, Sep 7, 2017 at 2:47 AM, Julia Lawall  wrote:
>> >
>> >
>> > On Thu, 7 Sep 2017, Srishti Sharma wrote:
>> >
>> >> Remove local variable inflight_counter ,as it is never used.
>> >
>> > "counter ,as" -> "counter, as"
>> >
>> >> Signed-off-by: Srishti Sharma 
>> >> ---
>> >> Changes in v3:
>> >>  - There was no longer a need to make the comment more comprehensible as
>> >>I have deleted the variable associated with it because it is unused .
>> >>  drivers/staging/ccree/ssi_cipher.c | 5 +
>> >>  1 file changed, 1 insertion(+), 4 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/ccree/ssi_cipher.c 
>> >> b/drivers/staging/ccree/ssi_cipher.c
>> >> index fec2faa..609ebe4 100644
>> >> --- a/drivers/staging/ccree/ssi_cipher.c
>> >> +++ b/drivers/staging/ccree/ssi_cipher.c
>> >> @@ -702,10 +702,7 @@ static int ssi_blkcipher_complete(struct device *dev,
>> >>
>> >>   ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, 
>> >> dst);
>> >>
>> >> - /*
>> >> -  * Save inflight_counter in a local variable.
>> >> -  */
>> >> - inflight_counter =  ctx_p->drvdata->inflight_counter;
>> >> +
>> >
>> > This is a patch on your previous patch.  It should be a patch on the
>> > original code.  Also, don't add a new blank line.  Just remove the line
>> > completely.
>>
>>   so , when I resend it should I version it as v3 or send as a new
>> patch as it is now fixing a different problem ?
>
> v3 (or maybe now v4?) would be fine.  It's still the same code that is
> under consideration.  Then it will be clear that he previosu patches are
> not needed.

 Okay , Thanks !
 Regards,
 Srishti
>
> julia
>
>>  Regards,
>>  Srishti
>> >
>> > julia
>> >
>> >>   /*Decrease the inflight counter*/
>> >>   if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter 
>> >> > 0)
>> >>   ctx_p->drvdata->inflight_counter--;
>> >> --
>> >> 2.7.4
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google Groups 
>> >> "outreachy-kernel" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send an 
>> >> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> >> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> >> To view this discussion on the web visit 
>> >> https://groups.google.com/d/msgid/outreachy-kernel/1504732426-9765-1-git-send-email-srishtishar%40gmail.com.
>> >> For more options, visit https://groups.google.com/d/optout.
>> >>
>>

Re: [Outreachy kernel] [PATCH v3] Staging: ccree: ssi_cipher.c: Remove unused variable.

2017-09-06 Thread Julia Lawall 


On Thu, 7 Sep 2017, Srishti Sharma wrote:

> On Thu, Sep 7, 2017 at 2:47 AM, Julia Lawall  wrote:
> >
> >
> > On Thu, 7 Sep 2017, Srishti Sharma wrote:
> >
> >> Remove local variable inflight_counter ,as it is never used.
> >
> > "counter ,as" -> "counter, as"
> >
> >> Signed-off-by: Srishti Sharma 
> >> ---
> >> Changes in v3:
> >>  - There was no longer a need to make the comment more comprehensible as
> >>I have deleted the variable associated with it because it is unused .
> >>  drivers/staging/ccree/ssi_cipher.c | 5 +
> >>  1 file changed, 1 insertion(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/staging/ccree/ssi_cipher.c 
> >> b/drivers/staging/ccree/ssi_cipher.c
> >> index fec2faa..609ebe4 100644
> >> --- a/drivers/staging/ccree/ssi_cipher.c
> >> +++ b/drivers/staging/ccree/ssi_cipher.c
> >> @@ -702,10 +702,7 @@ static int ssi_blkcipher_complete(struct device *dev,
> >>
> >>   ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, 
> >> dst);
> >>
> >> - /*
> >> -  * Save inflight_counter in a local variable.
> >> -  */
> >> - inflight_counter =  ctx_p->drvdata->inflight_counter;
> >> +
> >
> > This is a patch on your previous patch.  It should be a patch on the
> > original code.  Also, don't add a new blank line.  Just remove the line
> > completely.
>
>   so , when I resend it should I version it as v3 or send as a new
> patch as it is now fixing a different problem ?

v3 (or maybe now v4?) would be fine.  It's still the same code that is
under consideration.  Then it will be clear that he previosu patches are
not needed.

julia

>  Regards,
>  Srishti
> >
> > julia
> >
> >>   /*Decrease the inflight counter*/
> >>   if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter > 
> >> 0)
> >>   ctx_p->drvdata->inflight_counter--;
> >> --
> >> 2.7.4
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups 
> >> "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an 
> >> email to outreachy-kernel+unsubscr...@googlegroups.com.
> >> To post to this group, send email to outreachy-ker...@googlegroups.com.
> >> To view this discussion on the web visit 
> >> https://groups.google.com/d/msgid/outreachy-kernel/1504732426-9765-1-git-send-email-srishtishar%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>

Re: [PATCH v3] Staging: ccree: ssi_cipher.c: Remove unused variable.

2017-09-06 Thread Srishti Sharma 
On Thu, Sep 7, 2017 at 3:02 AM, Dan Carpenter  wrote:
> Always compile your patches.
>
>   CC [M]  drivers/staging/ccree/ssi_cipher.o
> drivers/staging/ccree/ssi_cipher.c: In function ‘ssi_blkcipher_complete’:
> drivers/staging/ccree/ssi_cipher.c:700:6: warning: unused variable 
> ‘inflight_counter’ [-Wunused-variable]
>   u32 inflight_counter;
>   ^~~~
>
> You need to delete the declaration as well.
>
> Don't be in a rush to resend patches.  I normally write them then let
> them sit in my outbox overnight and send them in the morning.  The extra
> delay helps me to calm down a bit and focus better.  Even though I've
> sent thousands of patches, it sometimes still stresses me out.  It's
> like you're disagreeing with the original author and the reviewers are
> disagreeing with you and everyone's trying to be nice about it but
> patches are fundamentally points of disagreement and that's stress.
>
> regards,
> dan carpenter
>
 Thanks , I'll be more careful !

 Regards,
 Srishti

[PATCH v4] Staging: ccree: ssi_cipher.c: Remove unused variable.

2017-09-06 Thread Srishti Sharma 
Remove local variable inflight_counter, as it is never used.

Signed-off-by: Srishti Sharma 
---
Change in v4:
 -There is no longer a need to make the comment more comprehensible, as I
  have deleted the variable associated with it. The v3 of this patch was
  not based on the original patch, hence v4.
 drivers/staging/ccree/ssi_cipher.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index 8d31a93..1ff3c8a 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -697,13 +697,10 @@ static int ssi_blkcipher_complete(struct device *dev,
  void __iomem *cc_base)
 {
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);

-   /*Set the inflight couter value to local variable*/
-   inflight_counter =  ctx_p->drvdata->inflight_counter;
/*Decrease the inflight counter*/
if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter > 0)
ctx_p->drvdata->inflight_counter--;
--
2.7.4

Re: [Outreachy kernel] [PATCH v3] Staging: ccree: ssi_cipher.c: Remove unused variable.

2017-09-06 Thread Julia Lawall 


On Thu, 7 Sep 2017, Srishti Sharma wrote:

> Remove local variable inflight_counter ,as it is never used.

"counter ,as" -> "counter, as"

> Signed-off-by: Srishti Sharma 
> ---
> Changes in v3:
>  - There was no longer a need to make the comment more comprehensible as
>I have deleted the variable associated with it because it is unused .
>  drivers/staging/ccree/ssi_cipher.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/staging/ccree/ssi_cipher.c 
> b/drivers/staging/ccree/ssi_cipher.c
> index fec2faa..609ebe4 100644
> --- a/drivers/staging/ccree/ssi_cipher.c
> +++ b/drivers/staging/ccree/ssi_cipher.c
> @@ -702,10 +702,7 @@ static int ssi_blkcipher_complete(struct device *dev,
>
>   ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);
>
> - /*
> -  * Save inflight_counter in a local variable.
> -  */
> - inflight_counter =  ctx_p->drvdata->inflight_counter;
> +

This is a patch on your previous patch.  It should be a patch on the
original code.  Also, don't add a new blank line.  Just remove the line
completely.

julia

>   /*Decrease the inflight counter*/
>   if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter > 0)
>   ctx_p->drvdata->inflight_counter--;
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1504732426-9765-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [PATCH v3] Staging: ccree: ssi_cipher.c: Remove unused variable.

2017-09-06 Thread Dan Carpenter 
Always compile your patches.

  CC [M]  drivers/staging/ccree/ssi_cipher.o
drivers/staging/ccree/ssi_cipher.c: In function ‘ssi_blkcipher_complete’:
drivers/staging/ccree/ssi_cipher.c:700:6: warning: unused variable 
‘inflight_counter’ [-Wunused-variable]
  u32 inflight_counter;
  ^~~~

You need to delete the declaration as well.

Don't be in a rush to resend patches.  I normally write them then let
them sit in my outbox overnight and send them in the morning.  The extra
delay helps me to calm down a bit and focus better.  Even though I've
sent thousands of patches, it sometimes still stresses me out.  It's
like you're disagreeing with the original author and the reviewers are
disagreeing with you and everyone's trying to be nice about it but
patches are fundamentally points of disagreement and that's stress.

regards,
dan carpenter

Re: [PATCH] crypto: authenc - cryptlen must be at least AAD len

2017-09-06 Thread Herbert Xu 
On Wed, Sep 06, 2017 at 10:10:08PM +0200, Stephan Müller wrote:
> Am Mittwoch, 6. September 2017, 21:22:44 CEST schrieb Stephan Müller:
> 
> Hi Herbert,
> 
> > With AF_ALG, AAD len and cryptlen can be set freely by unprivileged
> > user space. The cipher implementation must therefore validate the input
> > data for sanity. For AEAD ciphers, this implies that cryptlen must be
> > at least as large as AAD size.
> > 
> > This fixes a kernel crash that can be triggered via AF_ALG detected by
> > the fuzzing test implemented with libkcapi.
> 
> What is your opinion: should this check be rather added to 
> crypto_aead_encrypt 
> (similar to a sanity check found in crypto_aead_decrypt)?

Doesn't this apply to decryption as well? Perhaps we can simply
truncate assoclen in aead_request_set_ad.

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

Re: [PATCH 06/12] x86/crypto: Fix RBP usage in sha1_avx2_x86_64_asm.S

2017-09-06 Thread Tim Chen 
On 08/29/2017 11:05 AM, Josh Poimboeuf wrote:
> Using RBP as a temporary register breaks frame pointer convention and
> breaks stack traces when unwinding from an interrupt in the crypto code.
> 
> Use R11 instead of RBP.  Since R11 isn't a callee-saved register, it
> doesn't need to be saved and restored on the stack.

These changes seem okay.

Thanks.

Tim

> 
> Reported-by: Eric Biggers 
> Reported-by: Peter Zijlstra 
> Signed-off-by: Josh Poimboeuf 
> ---
>  arch/x86/crypto/sha1_avx2_x86_64_asm.S | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/arch/x86/crypto/sha1_avx2_x86_64_asm.S 
> b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
> index 1eab79c9ac48..9f712a7dfd79 100644
> --- a/arch/x86/crypto/sha1_avx2_x86_64_asm.S
> +++ b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
> @@ -89,7 +89,7 @@
>  #define  REG_RE  %rdx
>  #define  REG_RTA %r12
>  #define  REG_RTB %rbx
> -#define  REG_T1  %ebp
> +#define  REG_T1  %r11d
>  #define  xmm_mov vmovups
>  #define  avx2_zeroupper  vzeroupper
>  #define  RND_F1  1
> @@ -637,7 +637,6 @@ _loop3:
>   ENTRY(\name)
>  
>   push%rbx
> - push%rbp
>   push%r12
>   push%r13
>   push%r14
> @@ -673,7 +672,6 @@ _loop3:
>   pop %r14
>   pop %r13
>   pop %r12
> - pop %rbp
>   pop %rbx
>  
>   ret
>

Re: [PATCH] crypto: authenc - cryptlen must be at least AAD len

2017-09-06 Thread Stephan Müller 
Am Donnerstag, 7. September 2017, 05:54:05 CEST schrieb Herbert Xu:

Hi Herbert,
> > 
> > What is your opinion: should this check be rather added to
> > crypto_aead_encrypt (similar to a sanity check found in
> > crypto_aead_decrypt)?
> 
> Doesn't this apply to decryption as well?

There is already such check:

static inline int crypto_aead_decrypt(struct aead_request *req)
{
struct crypto_aead *aead = crypto_aead_reqtfm(req);

if (req->cryptlen < crypto_aead_authsize(aead))
return -EINVAL;
...

> Perhaps we can simply
> truncate assoclen in aead_request_set_ad.

I am not sure that would work because at the time we set the AAD len, we may 
not yet have cryptlen. I.e. aead_request_set_ad may be called before 
aead_request_set_crypt.


Ciao
Stephan

Re: [RFC Part2 PATCH v3 02/26] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-06 Thread Borislav Petkov 
On Mon, Jul 24, 2017 at 03:02:39PM -0500, Brijesh Singh wrote:
> Platform Security Processor (PSP) is part of AMD Secure Processor (AMD-SP),
> PSP is a dedicated processor that provides the support for key management
> commands in a Secure Encrypted Virtualiztion (SEV) mode, along with
> software-based Tursted Executation Environment (TEE) to enable the
> third-party tursted applications.
> 
> Cc: Herbert Xu 
> Cc: David S. Miller 
> Cc: Gary Hook 
> Cc: linux-crypto@vger.kernel.org
> Signed-off-by: Brijesh Singh 
> ---
>  drivers/crypto/ccp/Kconfig   |   9 ++
>  drivers/crypto/ccp/Makefile  |   1 +
>  drivers/crypto/ccp/psp-dev.c | 226 
> +++
>  drivers/crypto/ccp/psp-dev.h |  82 
>  drivers/crypto/ccp/sp-dev.c  |  43 
>  drivers/crypto/ccp/sp-dev.h  |  41 +++-
>  drivers/crypto/ccp/sp-pci.c  |  46 +
>  7 files changed, 447 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/crypto/ccp/psp-dev.c
>  create mode 100644 drivers/crypto/ccp/psp-dev.h

...

> diff --git a/drivers/crypto/ccp/sp-dev.c b/drivers/crypto/ccp/sp-dev.c
> index a017233..d263ba4 100644
> --- a/drivers/crypto/ccp/sp-dev.c
> +++ b/drivers/crypto/ccp/sp-dev.c

Hunk #1 succeeded at 24 (offset -7 lines).
checking file drivers/crypto/ccp/Makefile
Hunk #1 FAILED at 7.
1 out of 1 hunk FAILED
checking file drivers/crypto/ccp/psp-dev.c
checking file drivers/crypto/ccp/psp-dev.h
can't find file to patch at input line 414
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--
|diff --git a/drivers/crypto/ccp/sp-dev.c b/drivers/crypto/ccp/sp-dev.c
|index a017233..d263ba4 100644
|--- a/drivers/crypto/ccp/sp-dev.c
|+++ b/drivers/crypto/ccp/sp-dev.c
--

What tree is that against? In any case, it doesn't apply here.

> This RFC is based on tip/master commit : 22db3de (Merge branch 'x86/mm').

$ git show 22db3de
fatal: ambiguous argument '22db3de': unknown revision or path not in the 
working tree.

Do you have updated version of the series which you can send out?

> @@ -67,6 +74,10 @@ struct sp_device {
>   /* DMA caching attribute support */
>   unsigned int axcache;
>  
> + /* get and set master device */
> + struct sp_device*(*get_psp_master_device)(void);
> + void(*set_psp_master_device)(struct sp_device *);

WARNING: missing space after return type
#502: FILE: drivers/crypto/ccp/sp-dev.h:79:
+   void(*set_psp_master_device)(struct sp_device *);

Don't forget to run all patches through checkpatch. Some of the warnings
make sense.

Thx.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
--