Re: Backport e666d4e9ceec crypto: vmx - Use skcipher for ctr fallback

[Greg KH]

On Fri, Aug 24, 2018 at 09:51:41AM +0200, Petr Vorel wrote:
> Hi Herbert,
> 
> > On Thu, Aug 23, 2018 at 05:31:01PM +0200, Petr Vorel wrote:
> > > Hi,
> 
> > > I wonder, it it makes sense to backport commit
> > > e666d4e9ceec crypto: vmx - Use skcipher for ctr fallback
> > > to v 4.14 stable kernel.
> > > I'm using it in 4.12+.
> 
> > > These commits (somehow similar) has been backported to 4.10.2:
> > > 5839f555fa57 crypto: vmx - Use skcipher for xts fallback
> > > c96d0a1c47ab crypto: vmx - Use skcipher for cbc fallback
> 
> > I think so.  Here is the patch which should be applied for all
> > kernels since 4.10 up to and including 4.14:
> 
> Thanks a lot for confirmation and preparing the patch!

The patch didn't apply, but I fixed it :)

Now queued up.

greg k-h

Re: [PATCH] staging: ccree: ssi_hash: remove unnecessary parentheses

[Greg KH]

On Thu, Dec 28, 2017 at 12:24:17PM +0200, Elad Wexler wrote:
> Fixed a coding style issue
> 
> Signed-off-by: Elad Wexler 
> ---
>  drivers/staging/ccree/ssi_hash.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Why did you send this twice?

> diff --git a/drivers/staging/ccree/ssi_hash.c 
> b/drivers/staging/ccree/ssi_hash.c
> index 2035835..8d56fa7 100644
> --- a/drivers/staging/ccree/ssi_hash.c
> +++ b/drivers/staging/ccree/ssi_hash.c
> @@ -2280,8 +2280,8 @@ int ssi_hash_alloc(struct ssi_drvdata *drvdata)
> _handle->hash_list);
>   }
>  
> - if ((hw_mode == DRV_CIPHER_XCBC_MAC) ||
> - (hw_mode == DRV_CIPHER_CMAC))
> + if (hw_mode == DRV_CIPHER_XCBC_MAC ||
> + hw_mode == DRV_CIPHER_CMAC)

No, now I have to go look up if == or || comes in which order.  Ick,
just leave it as-is please.

thanks,

greg k-h

Re: [PATCH] staging: ccree: fix __dump_byte_array() declaration mismatch

[Greg KH]

On Wed, Dec 20, 2017 at 06:16:08PM +, Corentin Labbe wrote:
> This patch corrects the type of the size argument in __dump_byte_array()
> from unsigned long to size_t as done only in 
> drivers/staging/ccree/ssi_driver.c
> 
> This fix also a build error:
> drivers/staging/ccree/ssi_driver.c:82:6: error: conflicting types for 
> '__dump_byte_array'
> 
> Fixes: 3f268f5d6669 ("staging: ccree: turn compile time debug log to params")
> Signed-off-by: Corentin Labbe 
> ---
>  drivers/staging/ccree/ssi_driver.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

There's another warning here that Arnd's patches fixes, yet yours
didn't, so I'm going to take his patch, sorry.

greg k-h

Re: [PATCH] crypto: af_alg - add keylen checking to avoid NULL ptr passing down

[Greg KH]

On Mon, Dec 18, 2017 at 11:09:23AM +, Li Kun wrote:
> alg_setkey do not check the keylen whether it is zero, so the key
> may be ZERO_SIZE_PTR when keylen is 0, which will pass the
> copy_from_user's checking and be passed to the lower functions as key.
> 
> If the lower functions only check the key if it is NULL, ZERO_SIZE_PTR
> will pass the checking, and will cause null ptr dereference, so it's
> better to intercept the invalid parameters in the upper functions.
> 
> This patch is also suitable to fix CVE-2017-15116 for stable trees.
> 
> Signed-off-by: Li Kun 
> ---
>  crypto/af_alg.c | 2 ++
>  1 file changed, 2 insertions(+)



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

Re: [PATCH] staging: ccree: ssi_aead: fixed all coding style warnings.

[Greg KH]

On Wed, Dec 06, 2017 at 11:08:09PM +0100, SUNIL KALLUR RAMEGOWDA wrote:
> My first kernel patch, fixed warnings.
> 
> Signed-off-by: SUNIL KALLUR RAMEGOWDA 
> ---
>  drivers/staging/ccree/ssi_aead.c | 179 
> +++
>  1 file changed, 123 insertions(+), 56 deletions(-)

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch did many different things all at once, making it difficult
  to review.  All Linux kernel patches need to only do one thing at a
  time.  If you need to do multiple things (such as clean up all coding
  style issues in a file/driver), do it in a sequence of patches, each
  one doing only one thing.  This will make it easier to review the
  patches to ensure that they are correct, and to help alleviate any
  merge issues that larger patches can cause.

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH 4.9-stable] Revert "crypto: caam - get rid of tasklet"

[Greg KH]

On Tue, Dec 05, 2017 at 05:37:44PM +0200, Horia Geantă wrote:
> commit 2b163b5bce04546da72617bfb6c8bf07a45c4b17 upstream.

Now queued up, thanks!

greg k-h

Re: [PATCH] treewide: remove duplicate includes

[Greg KH]

On Mon, Dec 04, 2017 at 03:19:39AM +0530, Pravin Shedge wrote:
> These duplicate includes have been found with scripts/checkincludes.pl but
> they have been removed manually to avoid removing false positives.
> 
> Unit Testing:
> 
> - build successful
> - LTP testsuite passes.
> - checkpatch.pl passes
> 
> Signed-off-by: Pravin Shedge 

Please break this up per-subsystem, like any other cleanup patch, and
send it out that way.  As it is, no one can take such a patch.

thanks,

greg k-h

Re: [PATCH 4/4] staging: most: Remove a attribute group from a kobject

[Greg KH]

On Sat, Nov 25, 2017 at 12:09:31PM +0530, Arvind Yadav wrote:
> All attribute group created during dim2_sysfs_probe() should be removed
> in dim2_sysfs_destroy().
> 
> Signed-off-by: Arvind Yadav 
> ---
>  drivers/staging/most/hdm-dim2/dim2_sysfs.c | 1 +

This file is not in the tree anymore.

Re: [PATCH -stable] arm: crypto: reduce priority of bit-sliced AES cipher

[Greg KH]

On Fri, Nov 17, 2017 at 11:50:27AM -0800, Eric Biggers wrote:
> Hi,
> 
> I'd like the following patch to be applied to stable for versions
> between 4.1 and 4.10 (inclusively).
> 
> This is a minimal fix for a bug where arm32 kernels can use a much
> slower implementation of AES than is actually available, potentially
> forcing vendors to disable encryption on their devices.
> 
> Min version is 4.1 because that was the first version to include the
> aes-ce algorithms.
> 
> Max version is 4.10 because in 4.11, this bug was fixed incidentally as
> part of a complete rewrite of the bit-sliced AES implementation.

Thanks for the patch, now queued up.

greg k-h

Re: [PATCH v2] staging: ccree: Convert to platform_{get,set}_drvdata()

[Greg KH]

On Thu, Sep 21, 2017 at 05:47:42PM +0530, suni...@techveda.org wrote:
> From: Suniel Mahesh 
> 
> Platform devices are expected to use wrapper functions,
> platform_{get,set}_drvdata() with platform_device as argument,
> for getting and setting the driver data. dev_{get,set}_drvdata()
> are using _dev->dev.
> For wrapper functions we can directly pass a struct platform_device.
> 
> dev_set_drvdata() is redundant and therefore removed. The driver core
> clears the driver data to NULL after device_release or on probe failure.
> 
> Signed-off-by: Suniel Mahesh 
> ---
> Changes for v2:
> - Rebased on top of staging-testing.

Can you rebase again, this still does not apply :(

Re: [PATCH] staging:ccree Fix dont use assignment in if condition

[Greg KH]

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?


http://daringfireball.net/2007/07/on_top

On Fri, Sep 22, 2017 at 03:52:45PM +0530, Janani Sankara Babu wrote:
> Hi Mr. Greg,
> 
>      The patches that I have sent are independent of each other. There is
> nothing  from one patch that will affect the other one and vice verse. I have
> double checked the same.

And how am I supposed to know that?  :)

>      Kindly let me know if I have to do the ordering of patches.

Please do, they are long-gone from my queue now.

thanks,

greg k-h

Re: [PATCH] staging:ccree Fix dont use assignment in if condition

[Greg KH]

On Thu, Sep 21, 2017 at 12:07:04PM +0530, Janani Sankara Babu wrote:
> This patch solves the following error shown by checkpatch script
> ERROR: do not use assignment in if condition
> 
> Signed-off-by: Janani Sankara Babu 
> ---
>  drivers/staging/ccree/ssi_hash.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

You sent me 5 patches for the same driver, yet no way for me to
determine what the order of the patches are.  Please resend as a
numbered patch series so I get it right.

Remember, make it obvious what a maintainer is supposed to do...

thanks,

greg k-h

Re: [PATCH] crypto: AF_ALG - remove SGL end indicator when chaining

[Greg KH]

On Wed, Sep 20, 2017 at 03:47:46PM +0200, Stephan Mueller wrote:
> Am Mittwoch, 20. September 2017, 10:32:09 CEST schrieb Herbert Xu:
> 
> Hi Herbert,
> 
> > 
> > Hmm, this patch does not apply against the current tree.  Is this
> > a stable-only patch?
> 
> This would be a stable-only patch. With the overhauling of the AF_ALG memory 
> handling, this is a no-issue any more.

If you want this as a stable-only patch, you need to resend it and
justify it a bunch as to why it isn't in Linus's tree as well.

thanks,

greg k-h

Re: [PATCH v2] Staging: ccree: Prefer using BIT macro.

[Greg KH]

On Thu, Sep 07, 2017 at 07:44:52PM +0530, Srishti Sharma wrote:
> Use BIT(x) instead of using (1< 
> Signed-off-by: Srishti Sharma 
> ---
> Changes in v2:
>  - Add tab spaces before BIT macro.
> 
>  drivers/staging/ccree/ssi_cipher.h | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

This change is already in my tree from someone else, sorry.

greg k-h

Re: [PATCH] Staging: ccree: Don't use volatile for monitor_lock

[Greg KH]

On Mon, Sep 11, 2017 at 09:29:31PM +0530, Srishti Sharma wrote:
> The use of volatile for the variable monitor_lock is unnecessary.
> 
> Signed-off-by: Srishti Sharma 
> ---
>  drivers/staging/ccree/ssi_request_mgr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/ccree/ssi_request_mgr.c 
> b/drivers/staging/ccree/ssi_request_mgr.c
> index e5c2f92..7d77941 100644
> --- a/drivers/staging/ccree/ssi_request_mgr.c
> +++ b/drivers/staging/ccree/ssi_request_mgr.c
> @@ -49,7 +49,7 @@ struct ssi_request_mgr_handle {
>   dma_addr_t dummy_comp_buff_dma;
>   struct cc_hw_desc monitor_desc;
> 
> - volatile unsigned long monitor_lock;
> + unsigned long monitor_lock;

While volatile is not right, odds are, this is still totally wrong as
well.  How about using a "real" lock instead?

thanks,

greg k-h

Re: [PATCH] Staging: ccree: Prefer using BIT macro.

[Greg KH]

On Thu, Sep 07, 2017 at 07:17:09PM +0530, Srishti Sharma wrote:
> Use BIT(x) instead of (1< 
> Signed-off-by: Srishti Sharma 
> ---
>  drivers/staging/ccree/ssi_cipher.h | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/ccree/ssi_cipher.h 
> b/drivers/staging/ccree/ssi_cipher.h
> index 296b375..6fbcf9d 100644
> --- a/drivers/staging/ccree/ssi_cipher.h
> +++ b/drivers/staging/ccree/ssi_cipher.h
> @@ -27,11 +27,11 @@
>  #include "ssi_buffer_mgr.h"
> 
>  /* Crypto cipher flags */
> -#define CC_CRYPTO_CIPHER_KEY_KFDE0(1 << 0)
> -#define CC_CRYPTO_CIPHER_KEY_KFDE1(1 << 1)
> -#define CC_CRYPTO_CIPHER_KEY_KFDE2(1 << 2)
> -#define CC_CRYPTO_CIPHER_KEY_KFDE3(1 << 3)
> -#define CC_CRYPTO_CIPHER_DU_SIZE_512B (1 << 4)
> +#define CC_CRYPTO_CIPHER_KEY_KFDE0BIT(0)
> +#define CC_CRYPTO_CIPHER_KEY_KFDE1BIT(1)
> +#define CC_CRYPTO_CIPHER_KEY_KFDE2BIT(2)
> +#define CC_CRYPTO_CIPHER_KEY_KFDE3BIT(3)
> +#define CC_CRYPTO_CIPHER_DU_SIZE_512B BIT(4)

Care to use a  here as well?  I know the original didn't, but the
cleaned up code should.

thanks,

greg k-h

Re: [PATCH] staging/ccree: Declare compiled out fuctions static inline

[Greg KH]

On Mon, Jul 31, 2017 at 02:47:23PM +0530, RishabhHardas wrote:
> From: RishabhHardas 
> 
> Sparse was giving out a warning for symbols 'cc_set_ree_fips_status' and 
> 'fips_handler'
> that they were not declared and need to be made static. This patch makes both 
> the symbols
> static inline, to remove the warnings.
> 
> Signed-off-by: RishabhHardas 

I doubt you sign your name on legal documents without a ' ' somewhere in
it :(

Please fix up and resend.

thanks,

greg k-h

Re: [PATCH] Removed a sparse warning

[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: [PATCH 1/3] staging: ccree: Replace kzalloc with devm_kzalloc

[Greg KH]

On Sat, Jul 15, 2017 at 01:21:54PM +0530, suni...@techveda.org wrote:
> 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 
> ---
> Note:
> - Patch was tested and built(ARCH=arm) on next-20170714.
>   No build issues reported, however it was not tested on
>   real hardware.
> ---
>  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 78709b92..f231ecf 100644
> --- a/drivers/staging/ccree/ssi_driver.c
> +++ b/drivers/staging/ccree/ssi_driver.c
> @@ -224,13 +224,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(struct ssi_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(struct ssi_drvdata),

sizeof(*new_drvdata), right?

thanks,

greg k-h

Re: [PATCH 00/10] Fix alignment issues in staging/ccree

[Greg KH]

On Sun, Jul 02, 2017 at 01:25:45AM +0200, Simon Sandström wrote:
> Fixes a total of 195 alignment issues in staging/ccree reported by
> checkpatch.pl. Adds a few "line over 80 characters" warnings as a
> result of the realignments, but I could try to get rid of them in the
> same patchset if needed.

Not all of these applied, some did, if you could rebase the remaining
against my staging-testing branch at the moment, and resend, that would
be great.

thanks,

greg k-h

Re: [PATCH] Stagingdriver cctree: Fix for checkpatch warning

[Greg KH]

On Tue, Jun 27, 2017 at 11:47:56AM +0530, bincy_k_phi...@yahoo.co.in wrote:
> From: Bincy K Philip 
> 
> Trivial fix for Line over 80 characters
> 
> Moved the comment to top of the definition
> ---
>  drivers/staging/ccree/cc_hw_queue_defs.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)


Always use scripts/checkpatch.pl on your patch so you don't get a grumpy
maintaner telling you to run scripts/checkpatch.pl on your patch :(

Re: [PATCH 1/4] Staging: ccree: cc_crypto_ctx.h: Added * on subsequent lines of a comment block.

[Greg KH]

On Sun, Jun 04, 2017 at 05:02:08AM +0530, srishti sharma wrote:
> Added * on subsequent lines of a comment block.
> 
> Signed-off-by: srishti sharma 
> ---
>  drivers/staging/ccree/cc_crypto_ctx.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This whole series also does not apply.  Note, this driver is under
active development, you are probably running into the problem that lots
of people are working on it at the same time.

sorry,

greg k-h

Re: [PATCH] Staging: ccree: ssi_aead.h: Fixed a comment coding style issue.

[Greg KH]

On Sat, Jun 03, 2017 at 06:04:59PM +0530, srishti sharma wrote:
> Fixed a comment coding style issue , block comments use * on subsequent lines.
> 
> Signed-off-by: srishti sharma 
> ---
>  drivers/staging/ccree/ssi_aead.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Patch does not apply to my staging-testing branch, sorry :(

Re: [PATCH V2 01/27] Drivers: ccree: ssi_sysfs.h - align block comments

[Greg KH]

On Tue, May 30, 2017 at 06:09:54PM +1200, Derek Robson wrote:
> Fixed block comment alignment, Style fix only
> Found using checkpatch
> 
> Signed-off-by: Derek Robson 
> ---
>  drivers/staging/ccree/ssi_sysfs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Does not apply to my tree :(

Re: [PATCH V2 00/27] Drivers: ccree - align block comments

[Greg KH]

On Tue, May 30, 2017 at 06:09:37PM +1200, Derek Robson wrote:
> Fixed block comments across whole ccree driver
> 
> Version #1 has some trailing white space issue in a few patches

Some of these applied, some did not :(

Re: [PATCH] Staging: ccree: ssi_aead.h: Fixed a pointer declaration error.

[Greg KH]

On Sat, Jun 03, 2017 at 04:15:17AM +0530, srishti sharma wrote:
> Fixed a pointer declaration error , the dereferencing operator was misplaced.

Odd use of a ,

Also, I don't understand what was "misplaced" here?  There does not seem
to be a "error" fixed here...

Re: [PATCH] Staging: ccree: cc_crypto_ctx.h: Fixed a comment coding style issue.

[Greg KH]

On Wed, May 31, 2017 at 06:24:55PM +0530, srishti sharma wrote:
> Fixed a comment coding style issue that generated a warning stating that 
> block comments should align the * on each line.
> 

Please properly line-wrap your changelog comments :(

You also sent 2 different patches, that did different things, yet they
had the same subject line.  That's not ok, and also, I have no idea
which one to apply first.

I'm dropping both, please fix and resend them properly as a patch
series.

thanks,

greg k-h

Re: [PATCH 12/27] Drivers: ccree: ssi_fips_ll.c - align block comments

[Greg KH]

On Wed, May 24, 2017 at 04:43:52PM +1200, Derek Robson wrote:
> Fixed block comment alignment, Style fix only
> Found using checkpatch
> 
> Signed-off-by: Derek Robson 
> ---
>  drivers/staging/ccree/ssi_fips_ll.c | 21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/ccree/ssi_fips_ll.c 
> b/drivers/staging/ccree/ssi_fips_ll.c
> index 7c7c922f0788..cb487d747ee8 100644
> --- a/drivers/staging/ccree/ssi_fips_ll.c
> +++ b/drivers/staging/ccree/ssi_fips_ll.c
> @@ -15,9 +15,9 @@
>   */
>  
>  /**
> -This file defines the driver FIPS Low Level implmentaion functions,
> -that executes the KAT.
> -***/
> + * This file defines the driver FIPS Low Level implmentaion functions,
> + * that executes the KAT.
> + ***/
>  #include 
>  
>  #include "ssi_driver.h"
> @@ -816,7 +816,8 @@ ssi_hmac_fips_run_test(struct ssi_drvdata *drvdata,
>  dma_addr_t digest_bytes_len_dma_addr)
>  {
>   /* The implemented flow is not the same as the one implemented in 
> ssi_hash.c (setkey + digest flows).
> -In this flow, there is no need to store and reload some of the 
> intermidiate results. */
> +  * In this flow, there is no need to store and reload some of the 
> intermidiate results.
> +  */
>  
>   /* max number of descriptors used for the flow */
>   #define FIPS_HMAC_MAX_SEQ_LEN 12
> @@ -948,9 +949,9 @@ ssi_hmac_fips_run_test(struct ssi_drvdata *drvdata,
>   idx++;
>  
>   /* at this point:
> -tmp_digest = H(o_key_pad)
> -k0 = H(i_key_pad || m)
> -*/
> +  * tmp_digest = H(o_key_pad)
> +  * k0 = H(i_key_pad || m)
> +  */
>  
>   /* Loading hash opad xor key state */
>   HW_DESC_INIT([idx]);
> @@ -1413,8 +1414,10 @@ ssi_gcm_fips_run_test(struct ssi_drvdata *drvdata,
>   idx++;
>  
>   /* Configure Hash Engine to work with GHASH.
> -Since it was not possible to extend HASH submodes to add GHASH,
> -The following command is necessary in order to select GHASH 
> (according to HW designers)*/
> +  * Since it was not possible to extend HASH submodes to add GHASH,
> +  * The following command is necessary in order to 

You added a coding style issue that was not previously here in this
patch, which isn't ok :(

I've stopped here with this series, please fix up the reset of this
series (hint, this wasn't the only place you messed this up) and resend.

thanks,

greg k-h

Re: [PATCH] padata: avoid race in reordering

[Greg KH]

On Tue, Apr 04, 2017 at 01:53:15PM +0200, Jason A. Donenfeld wrote:
> Herbert applied this to his tree. It's probably a good stable
> candidate, since it's a two line change to fix a race condition.
> 
> On Fri, Mar 24, 2017 at 3:16 PM, Herbert Xu  
> wrote:
> > Jason A. Donenfeld  wrote:
> >> Under extremely heavy uses of padata, crashes occur, and with list
> >> debugging turned on, this happens instead:
> >>
> >> [87487.298728] WARNING: CPU: 1 PID: 882 at lib/list_debug.c:33
> >> __list_add+0xae/0x130
> >> [87487.301868] list_add corruption. prev->next should be next
> >> (b17abfc043d0), but was 8dba70872c80. (prev=8dba70872b00).
> >> [87487.339011]  [] dump_stack+0x68/0xa3
> >> [87487.342198]  [] ? console_unlock+0x281/0x6d0
> >> [87487.345364]  [] __warn+0xff/0x140
> >> [87487.348513]  [] warn_slowpath_fmt+0x4a/0x50
> >> [87487.351659]  [] __list_add+0xae/0x130
> >> [87487.354772]  [] ? _raw_spin_lock+0x64/0x70
> >> [87487.357915]  [] padata_reorder+0x1e6/0x420
> >> [87487.361084]  [] padata_do_serial+0xa5/0x120
> >>
> >> padata_reorder calls list_add_tail with the list to which its adding
> >> locked, which seems correct:
> >>
> >> spin_lock(>serial.lock);
> >> list_add_tail(>list, >serial.list);
> >> spin_unlock(>serial.lock);
> >>
> >> This therefore leaves only place where such inconsistency could occur:
> >> if padata->list is added at the same time on two different threads.
> >> This pdata pointer comes from the function call to
> >> padata_get_next(pd), which has in it the following block:
> >>
> >> next_queue = per_cpu_ptr(pd->pqueue, cpu);
> >> padata = NULL;
> >> reorder = _queue->reorder;
> >> if (!list_empty(>list)) {
> >>   padata = list_entry(reorder->list.next,
> >>   struct padata_priv, list);
> >>   spin_lock(>lock);
> >>   list_del_init(>list);
> >>   atomic_dec(>reorder_objects);
> >>   spin_unlock(>lock);
> >>
> >>   pd->processed++;
> >>
> >>   goto out;
> >> }
> >> out:
> >> return padata;
> >>
> >> I strongly suspect that the problem here is that two threads can race
> >> on reorder list. Even though the deletion is locked, call to
> >> list_entry is not locked, which means it's feasible that two threads
> >> pick up the same padata object and subsequently call list_add_tail on
> >> them at the same time. The fix is thus be hoist that lock outside of
> >> that block.
> >>
> >> Signed-off-by: Jason A. Donenfeld 
> >
> > Patch applied.  Thanks.

Any clue as to what the git commit id is?

thanks,

greg k-h

Re: [PATCH] crypto: Add ECB dependency for XTS mode

[Greg KH]

On Fri, Mar 03, 2017 at 07:25:21AM +0100, Milan Broz wrote:
> Patch below should be backported to 4.10 stable
> (only 4.10, older kernels are ok).
> We have reports some systems fail to boot from LUKS now
> (missing ecb module in initramdisk) ...
> 
> Upstream commit is 12cb3a1c4184f891d965d1f39f8cfcc9ef617647

Now queued up, thanks.

greg k-h

Re: [PATCH 1/4] lib: Update LZ4 compressor module

[Greg KH]

On Sat, Jan 21, 2017 at 04:09:08PM +0100, Sven Schmidt wrote:
> This patch updates LZ4 kernel module to LZ4 v1.7.3 by Yann Collet.
> The kernel module is inspired by the previous work by Chanho Min.
> The updated LZ4 module will not break existing code since there were alias
> methods added to ensure backwards compatibility.
> 
> API changes:
> 
> New method LZ4_compress_fast which differs from the variant available
> in kernel by the new acceleration parameter,
> allowing to trade compression ratio for more compression speed
> and vice versa.
> 
> LZ4_decompress_fast is the respective decompression method, featuring a very
> fast decoder (multiple GB/s per core), able to reach RAM speed in multi-core
> systems. The decompressor allows to decompress data compressed with
> LZ4 fast as well as the LZ4 HC (high compression) algorithm.
> 
> Also the useful functions LZ4_decompress_safe_partial
> LZ4_compress_destsize were added. The latter reverses the logic by trying to
> compress as much data as possible from source to dest while the former aims
> to decompress partial blocks of data.
> 
> A bunch of streaming functions were also added
> which allow compressig/decompressing data in multiple steps
> (so called "streaming mode").
> 
> The methods lz4_compress and lz4_decompress_unknownoutputsize
> are now known as LZ4_compress_default respectivley LZ4_decompress_safe.
> The old methods are still available for providing backwards compatibility.
> 
> Signed-off-by: Sven Schmidt <4ssch...@informatik.uni-hamburg.de>

Please fix up so that it doesn't break the build as reported by the
kbuild tool...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Introduce AMD Secure Processor device

[Greg KH]

On Fri, Jan 20, 2017 at 09:40:49AM -0600, Brijesh Singh wrote:
> 
> On 01/20/2017 02:45 AM, Greg KH wrote:
> > On Thu, Jan 19, 2017 at 02:03:12PM -0600, Brijesh Singh wrote:
> > > Hi Greg,
> > > 
> > > On 01/19/2017 12:21 PM, Greg KH wrote:
> > > > On Thu, Jan 19, 2017 at 01:07:50PM -0500, Brijesh Singh wrote:
> > > > > The CCP device (drivers/crypto/ccp/ccp.ko) is part of AMD Secure 
> > > > > Processor,
> > > > > which is not dedicated solely to crypto. The AMD Secure Processor 
> > > > > includes
> > > > > CCP and PSP (Platform Secure Processor) devices.
> > > > > 
> > > > > This patch series moves the CCP device driver to the misc directory 
> > > > > and
> > > > > creates a framework that allows functional component of the AMD Secure
> > > > > Processor to be initialized and handled appropriately.
> > > > 
> > > > Why the misc directory?  I don't see the justification here...
> > > > 
> > > 
> > > Since this driver is not solely for crypto purposes and do not fit in any 
> > > of
> > > the standard categories hence I thought of moving it into misc directory. 
> > > I
> > > am open to other suggestions unless Herbert is ok with leaving it into
> > > crypto and allowing the addition of the Secure Processor support.
> > > 
> > > The patch series allows the CCP driver to support other Secure Processor
> > > functions, e.g Secure Encrypted Virtualization (SEV) key management. In
> > > past, I tried to add SEV support into existing CCP driver [1] but we 
> > > quickly
> > > learned that CCP driver should be moved outside the crypto directory
> > > otherwise will end up adding non crypto code into drivers/crypto 
> > > directory.
> > > Once this cleanup is accepted then I can work to add SEV support inside 
> > > the
> > > CCP driver.
> > > 
> > > [1] http://marc.info/?l=linux-kernel=147204118426151=2
> > 
> > Ok, what type of interface will this driver have with userspace and/or
> > other parts of the kernel?  Is there a misc char device burried in there
> > somewhere (I couldn't find it in the big diff sent out), or is this
> > driver just creating specific apis that other parts of the kernel will
> > call if available?
> > 
> 
> Eventually, the driver will export functions which will be used by KVM
> to encrypt the guest memory and more. Additionally, If SEV device is
> detected then driver will create a misc char device which can be used by
> userspace to import/export certificates etc.

Why create a new api for certificates, why not just use the existing
kernel key handling for it?

Having a random char device for something like this is going to be rough
to approve, I'll wait for the patches before I start objecting really
hard :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Introduce AMD Secure Processor device

[Greg KH]

On Thu, Jan 19, 2017 at 02:03:12PM -0600, Brijesh Singh wrote:
> Hi Greg,
> 
> On 01/19/2017 12:21 PM, Greg KH wrote:
> > On Thu, Jan 19, 2017 at 01:07:50PM -0500, Brijesh Singh wrote:
> > > The CCP device (drivers/crypto/ccp/ccp.ko) is part of AMD Secure 
> > > Processor,
> > > which is not dedicated solely to crypto. The AMD Secure Processor includes
> > > CCP and PSP (Platform Secure Processor) devices.
> > > 
> > > This patch series moves the CCP device driver to the misc directory and
> > > creates a framework that allows functional component of the AMD Secure
> > > Processor to be initialized and handled appropriately.
> > 
> > Why the misc directory?  I don't see the justification here...
> > 
> 
> Since this driver is not solely for crypto purposes and do not fit in any of
> the standard categories hence I thought of moving it into misc directory. I
> am open to other suggestions unless Herbert is ok with leaving it into
> crypto and allowing the addition of the Secure Processor support.
> 
> The patch series allows the CCP driver to support other Secure Processor
> functions, e.g Secure Encrypted Virtualization (SEV) key management. In
> past, I tried to add SEV support into existing CCP driver [1] but we quickly
> learned that CCP driver should be moved outside the crypto directory
> otherwise will end up adding non crypto code into drivers/crypto directory.
> Once this cleanup is accepted then I can work to add SEV support inside the
> CCP driver.
> 
> [1] http://marc.info/?l=linux-kernel=147204118426151=2

Ok, what type of interface will this driver have with userspace and/or
other parts of the kernel?  Is there a misc char device burried in there
somewhere (I couldn't find it in the big diff sent out), or is this
driver just creating specific apis that other parts of the kernel will
call if available?

I think we need to see more code here, broken up into sane and
reviewable pieces, before we could either say "No don't do that!" or
"sure, let me go merge these!" :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] misc: amd-sp: introduce the AMD Secure Processor device

[Greg KH]

On Thu, Jan 19, 2017 at 01:08:11PM -0500, Brijesh Singh wrote:
> The CCP device is part of the AMD Secure Processor. In order to expand the
> usage of the AMD Secure Processor, create a framework that allows functional
> components of the AMD Secure Processor to be initialized and handled
> appropriately.

What do you mean by "framework"?  What is this for?  Who is going to use
it?  Don't add framework that is not ever used, otherwise we will just
delete it.

> 
> Signed-off-by: Brijesh Singh 
> Signed-off-by: Tom Lendacky 
> ---
>  drivers/crypto/ccp/Kconfig|1 
>  drivers/misc/amd-sp/Kconfig   |   16 +-
>  drivers/misc/amd-sp/Makefile  |   11 +
>  drivers/misc/amd-sp/ccp-dev-v3.c  |   86 +-
>  drivers/misc/amd-sp/ccp-dev-v5.c  |   72 
>  drivers/misc/amd-sp/ccp-dev.c |  137 +---
>  drivers/misc/amd-sp/ccp-dev.h |   35 
>  drivers/misc/amd-sp/sp-dev.c  |  309 +++
>  drivers/misc/amd-sp/sp-dev.h  |  141 
>  drivers/misc/amd-sp/sp-pci.c  |  325 
> +
>  drivers/misc/amd-sp/sp-platform.c |  269 +++
>  11 files changed, 1225 insertions(+), 177 deletions(-)
>  create mode 100644 drivers/misc/amd-sp/sp-dev.c
>  create mode 100644 drivers/misc/amd-sp/sp-dev.h
>  create mode 100644 drivers/misc/amd-sp/sp-pci.c
>  create mode 100644 drivers/misc/amd-sp/sp-platform.c

This patch makes no sense, you need to break it up into "do one logical
thing", as that is NOT what is happening here.  It's impossible to
review as-is, sorry.

greg k-h

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Introduce AMD Secure Processor device

[Greg KH]

On Thu, Jan 19, 2017 at 01:07:50PM -0500, Brijesh Singh wrote:
> The CCP device (drivers/crypto/ccp/ccp.ko) is part of AMD Secure Processor,
> which is not dedicated solely to crypto. The AMD Secure Processor includes
> CCP and PSP (Platform Secure Processor) devices.
> 
> This patch series moves the CCP device driver to the misc directory and
> creates a framework that allows functional component of the AMD Secure
> Processor to be initialized and handled appropriately.

Why the misc directory?  I don't see the justification here...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] crypto: move CCP device driver to misc

[Greg KH]

On Thu, Jan 19, 2017 at 01:08:01PM -0500, Brijesh Singh wrote:
> The CCP device is part of the AMD Secure Processor, which is not dedicated
> solely to crypto. Move the CCP device driver to the misc directory in
> prepration for expanding the usage of the AMD Secure Processor. Leaving the
> CCP cryptographic layer (the ccp-crypto* files) in their current directory.
> 
> Signed-off-by: Brijesh Singh 
> Signed-off-by: Tom Lendacky 
> ---
>  drivers/crypto/Kconfig  |   11 
>  drivers/crypto/Makefile |2 
>  drivers/crypto/ccp/Kconfig  |   21 
>  drivers/crypto/ccp/Makefile |9 
>  drivers/crypto/ccp/ccp-dev-v3.c |  574 ---
>  drivers/crypto/ccp/ccp-dev-v5.c | 1021 ---
>  drivers/crypto/ccp/ccp-dev.c|  588 ---
>  drivers/crypto/ccp/ccp-dev.h|  647 
>  drivers/crypto/ccp/ccp-dmaengine.c  |  728 --
>  drivers/crypto/ccp/ccp-ops.c| 1876 
> ---
>  drivers/crypto/ccp/ccp-pci.c|  354 ---
>  drivers/crypto/ccp/ccp-platform.c   |  293 -
>  drivers/misc/Kconfig|1 
>  drivers/misc/Makefile   |1 
>  drivers/misc/amd-sp/Kconfig |   14 
>  drivers/misc/amd-sp/Makefile|8 
>  drivers/misc/amd-sp/ccp-dev-v3.c|  574 +++
>  drivers/misc/amd-sp/ccp-dev-v5.c| 1021 +++
>  drivers/misc/amd-sp/ccp-dev.c   |  588 +++
>  drivers/misc/amd-sp/ccp-dev.h   |  647 
>  drivers/misc/amd-sp/ccp-dmaengine.c |  728 ++
>  drivers/misc/amd-sp/ccp-ops.c   | 1876 
> +++
>  drivers/misc/amd-sp/ccp-pci.c   |  354 +++
>  drivers/misc/amd-sp/ccp-platform.c  |  293 +
>  include/linux/ccp.h |3 
>  25 files changed, 6111 insertions(+), 6121 deletions(-)
>  delete mode 100644 drivers/crypto/ccp/ccp-dev-v3.c
>  delete mode 100644 drivers/crypto/ccp/ccp-dev-v5.c
>  delete mode 100644 drivers/crypto/ccp/ccp-dev.c
>  delete mode 100644 drivers/crypto/ccp/ccp-dev.h
>  delete mode 100644 drivers/crypto/ccp/ccp-dmaengine.c
>  delete mode 100644 drivers/crypto/ccp/ccp-ops.c
>  delete mode 100644 drivers/crypto/ccp/ccp-pci.c
>  delete mode 100644 drivers/crypto/ccp/ccp-platform.c
>  create mode 100644 drivers/misc/amd-sp/Kconfig
>  create mode 100644 drivers/misc/amd-sp/Makefile
>  create mode 100644 drivers/misc/amd-sp/ccp-dev-v3.c
>  create mode 100644 drivers/misc/amd-sp/ccp-dev-v5.c
>  create mode 100644 drivers/misc/amd-sp/ccp-dev.c
>  create mode 100644 drivers/misc/amd-sp/ccp-dev.h
>  create mode 100644 drivers/misc/amd-sp/ccp-dmaengine.c
>  create mode 100644 drivers/misc/amd-sp/ccp-ops.c
>  create mode 100644 drivers/misc/amd-sp/ccp-pci.c
>  create mode 100644 drivers/misc/amd-sp/ccp-platform.c

Please create your patch with -M, to show this is a rename, or a change
with a rename.  Otherwise this is an impossible patch to review, would
you want to try to do it?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

[Greg KH]

On Tue, Jan 10, 2017 at 10:21:16AM +0100, Sven Schmidt wrote:
> On 01/08/2017 12:25 PM, Greg KH wrote:
> >On Sat, Jan 07, 2017 at 05:55:42PM +0100, Sven Schmidt wrote:
> >> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
> >> The kernel module is inspired by the previous work by Chanho Min.
> >> The updated LZ4 module will not break existing code since there were alias
> >> methods added to ensure backwards compatibility.
> >
> > Meta-comment.  Does this update include all of the security fixes that
> > we have made over the past few years to the lz4 code?  I don't want to
> > be adding back insecure functions that will cause us problems.
> >
> > Specifically look at the changes I made in 2014 in this directory for an
> > example of what I am talking about here.
> >
> 
> Hi Greg,
> 
> it doesn't. I didn't have that in mind until now.

Ick, those changes never got made "upstream"?  Not good, but makes sense
as we couldn't really find an "upstream" when we made them :(

As you took this code from somewhere, you might want to also push your
changes for these issues there as well, so that others don't run into
them in the future.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

[Greg KH]

On Tue, Jan 10, 2017 at 10:32:17AM +0100, Sven Schmidt wrote:
> On 01/08/2017 12:23 PM, Greg KH wrote:
> > And follow the proper kernel coding style rules, putting your patches
> > through scripts/checkpatch.pl should help you out here.
> 
> Sorry, I didn't know about that particular script. I already put
> Patches 2, 3 and 4 through checkpatch.pl; patch 1 is a little more to
> rework but I will, of course, do that.

You do know about Documentation/SubmittingPatches, right?  If not, take
a look at that as well :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

[Greg KH]

On Sat, Jan 07, 2017 at 05:55:42PM +0100, Sven Schmidt wrote:
> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
> The kernel module is inspired by the previous work by Chanho Min.
> The updated LZ4 module will not break existing code since there were alias
> methods added to ensure backwards compatibility.

Meta-comment.  Does this update include all of the security fixes that
we have made over the past few years to the lz4 code?  I don't want to
be adding back insecure functions that will cause us problems.

Specifically look at the changes I made in 2014 in this directory for an
example of what I am talking about here.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] lib/decompress_unlz4: Change module to work with new LZ4 module version

[Greg KH]

On Sat, Jan 07, 2017 at 05:55:43PM +0100, Sven Schmidt wrote:
> This patch updates the unlz4 wrapper to work with the new LZ4 kernel module 
> version.
> 
> Signed-off-by: Sven Schmidt <4ssch...@informatik.uni-hamburg.de>
> ---
>  lib/decompress_unlz4.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/decompress_unlz4.c b/lib/decompress_unlz4.c
> index 036fc88..1b0baf3 100644
> --- a/lib/decompress_unlz4.c
> +++ b/lib/decompress_unlz4.c
> @@ -72,7 +72,7 @@ STATIC inline int INIT unlz4(u8 *input, long in_len,
>   error("NULL input pointer and missing fill function");
>   goto exit_1;
>   } else {
> - inp = large_malloc(lz4_compressbound(uncomp_chunksize));
> + inp = large_malloc(LZ4_compressBound(uncomp_chunksize));

Having functions differ by different cases of the characters is ripe for
abuse and confusion.  Please never do that, especially as these "new"
functions you created don't follow the correct kernel coding style rules
:(

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

[Greg KH]

On Sat, Jan 07, 2017 at 05:55:42PM +0100, Sven Schmidt wrote:
> +/*!LZ4_compressbound() :
> +Provides the maximum size that LZ4 may output in a "worst case" scenario
> +(input data not compressible)
> +*/

Odd coding style, please use kerneldoc format if you are going to have
comments like this.

> +static inline int LZ4_compressBound(int isize) {
> +  return LZ4_COMPRESSBOUND(isize);

And follow the proper kernel coding style rules, putting your patches
through scripts/checkpatch.pl should help you out here.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kernel-hardening] [PATCH] siphash: add cryptographically secure hashtable function

[Greg KH]

On Sun, Dec 11, 2016 at 04:30:31PM +0100, Jason A. Donenfeld wrote:
> Hi Greg,
> 
> Thanks for the review. Responses to your suggestions are inline below:
> 
> On Sat, Dec 10, 2016 at 1:37 PM, Greg KH <gre...@linuxfoundation.org> wrote:
> > Please use u64 and u8 instead of the userspace uint64_t and uint8_t
> > types for kernel code.  Yes, the ship has probably sailed for trying to
> > strictly enforce it, but it's a good idea to do where ever possible.
> 
> I didn't know this was a rule. Since I had seen a hodgepodge
> throughout the kernel I just sort of assumed it was a free for all.
> I've fixed this up for v2, and I've also gone through all of my other
> [not yet submitted] code and made this change.
> 
> > Any specific license for this code?  It's good to at the least say what
> > it is.  Yes, we know it will default to GPLv2 only as part of the whole
> > kernel tree, but it's good to be explicit for when someone wants to copy
> > this code for their own projects...
> 
> Public domain, actually. I'll add notice of this to the header.

Hm, there really is no such license as "Public domain" that works in all
countries, sorry.  You will note it's not one of the "valid module
license list" we have in module.h because of that.

So, I don't know where you got the code from, but perhaps "Dual BSD/GPL"
is the correct one for you?

Note, I'm not a lawyer, so this isn't legal advice about the license of
code, but I do spend way too much time with lawyers dealing with license
issues...

> >> +#define U8TO64(p) le64_to_cpu(*(__le64 *)(p))
> >
> > Why the crazy casting behind a macro?
> 
> le64_to_cpup doesn't take the right type. But I agree the macro is not
> the nicest way to do this. Instead, I'll copy what
> crypto/chacha20_generic.c does and define locally le64_to_cpuvp which
> takes a void pointer:
> 
> static inline u64 le64_to_cpuvp(const void *p)
> {
> return le64_to_cpup(p);
> }

Ah much better.

> >> +__attribute__((optimize("unroll-loops")))
> >
> > Care to document why this attribute is needed?  Older versions of gcc
> > doesn't know how to handle it properly?  Faster with newer versions?
> > Black magic?  :)
> 
> It tells gcc to unroll the loops. Comparing the assembly, it looks
> like on x86_64, gcc does twice as many rounds per loop iteration when
> asked to unroll the loops. This allows the code to remain neat and
> straightforward, while instructing gcc to do the gnarly part.
> 
> Is this too much rice? Have I been Gentoo-ing for too long? Or are
> limited uses of unroll-loops acceptable?

Given that this would be the first use of it in the kernel, it might be
too much rice :)

Unless you can show real numbers that it actually matters, I wouldn't do
it, it's not worth the hassle.  That's the same rule for when you use
likely()/unlikely(), if you can not measure it, don't use it as you
almost always get it wrong, the compiler is smarter, and keeps getting
better over time.

> > s/uint64_t/u64/g please.
> 
> Done.
> 
> > EXPORT_SYMBOL_GPL()?  I have to ask, sorry :)
> 
> Since it's public domain, EXPORT_SYMBOL() is fine.
> 
> If you have some reason for preferring GPL2 over public domain, I'm
> happy to make the change. Maybe you want to preclude the new
> from proprietary modules? That's fine with me, if you desire it being
> that way.

Nope, I just have to ask :)

If it's dual licensed, a normal EXPORT_SYMBOL() is just fine, I have no
objection to that at all.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kernel-hardening] [PATCH] siphash: add cryptographically secure hashtable function

[Greg KH]

On Fri, Dec 09, 2016 at 07:36:59PM +0100, Jason A. Donenfeld wrote:
> SipHash is a 64-bit keyed hash function that is actually a
> cryptographically secure PRF, like HMAC. Except SipHash is super fast,
> and is meant to be used as a hashtable keyed lookup function.
> 
> SipHash isn't just some new trendy hash function. It's been around for a
> while, and there really isn't anything that comes remotely close to
> being useful in the way SipHash is. With that said, why do we need this?
> 
> There are a variety of attacks known as "hashtable poisoning" in which an
> attacker forms some data such that the hash of that data will be the
> same, and then preceeds to fill up all entries of a hashbucket. This is
> a realistic and well-known denial-of-service vector.
> 
> Linux developers already seem to be aware that this is an issue, and
> various places that use hash tables in, say, a network context, use a
> non-cryptographically secure function (usually jhash) and then try to
> twiddle with the key on a time basis (or in many cases just do nothing
> and hope that nobody notices). While this is an admirable attempt at
> solving the problem, it doesn't actually fix it. SipHash fixes it.
> 
> (It fixes it in such a sound way that you could even build a stream
> cipher out of SipHash that would resist the modern cryptanalysis.)
> 
> There are a modicum of places in the kernel that are vulnerable to
> hashtable poisoning attacks, either via userspace vectors or network
> vectors, and there's not a reliable mechanism inside the kernel at the
> moment to fix it. The first step toward fixing these issues is actually
> getting a secure primitive into the kernel for developers to use. Then
> we can, bit by bit, port things over to it as deemed appropriate.
> 
> Dozens of languages are already using this internally for their hash
> tables. Some of the BSDs already use this in their kernels. SipHash is
> a widely known high-speed solution to a widely known problem, and it's
> time we catch-up.
> 
> Signed-off-by: Jason A. Donenfeld 
> Cc: Jean-Philippe Aumasson 
> Cc: Daniel J. Bernstein 
> ---
>  include/linux/siphash.h |  18 ++
>  lib/Makefile|   3 +-
>  lib/siphash.c   | 163 
> 
>  3 files changed, 183 insertions(+), 1 deletion(-)
>  create mode 100644 include/linux/siphash.h
>  create mode 100644 lib/siphash.c

This looks really nice, but we don't usually add stuff into lib/ unless
there is an actual user of the code :)

Have you tried converting any of the existing hash users to use this
instead?  If you did that, and it shows a solution for the known
problems with our existing hashes (as you point out above), I doubt
there would be any objection for this patch at all.

Minor coding style nits below:

> @@ -0,0 +1,18 @@
> +/* Copyright (C) 2016 Jason A. Donenfeld 
> + *
> + * SipHash: a fast short-input PRF
> + * https://131002.net/siphash/
> + */
> +
> +#ifndef _LINUX_SIPHASH_H
> +#define _LINUX_SIPHASH_H
> +
> +#include 
> +
> +enum siphash24_lengths {
> + SIPHASH24_KEY_LEN = 16
> +};
> +
> +uint64_t siphash24(const uint8_t *data, size_t len, const uint8_t 
> key[SIPHASH24_KEY_LEN]);

Please use u64 and u8 instead of the userspace uint64_t and uint8_t
types for kernel code.  Yes, the ship has probably sailed for trying to
strictly enforce it, but it's a good idea to do where ever possible.

> +
> +#endif /* _LINUX_SIPHASH_H */
> diff --git a/lib/Makefile b/lib/Makefile
> index 50144a3aeebd..d224337b0d01 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -22,7 +22,8 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
>sha1.o chacha20.o md5.o irq_regs.o argv_split.o \
>flex_proportions.o ratelimit.o show_mem.o \
>is_single_threaded.o plist.o decompress.o kobject_uevent.o \
> -  earlycpio.o seq_buf.o nmi_backtrace.o nodemask.o win_minmax.o
> +  earlycpio.o seq_buf.o siphash.o \
> +  nmi_backtrace.o nodemask.o win_minmax.o
>  
>  lib-$(CONFIG_MMU) += ioremap.o
>  lib-$(CONFIG_SMP) += cpumask.o
> diff --git a/lib/siphash.c b/lib/siphash.c
> new file mode 100644
> index ..022d86f04b9b
> --- /dev/null
> +++ b/lib/siphash.c
> @@ -0,0 +1,163 @@
> +/* Copyright (C) 2015-2016 Jason A. Donenfeld 
> + * Copyright (C) 2012-2014 Jean-Philippe Aumasson 
> 
> + * Copyright (C) 2012-2014 Daniel J. Bernstein 
> + *
> + * SipHash: a fast short-input PRF
> + * https://131002.net/siphash/
> + */

Any specific license for this code?  It's good to at the least say what
it is.  Yes, we know it will default to GPLv2 only as part of the whole
kernel tree, but it's good to be explicit for when someone wants to copy
this code for their own projects...

> +
> +#include 
> +#include 
> +#include 
> +
> +#define ROTL(x,b) (uint64_t)(((x) << (b)) | ((x) >> (64 - (b

Don't we 

Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized

[Greg KH]

On Sun, Nov 13, 2016 at 09:47:41AM +0100, Greg KH wrote:
> On Sat, Nov 12, 2016 at 01:27:12PM +, Jonathan Cameron wrote:
> > On 11/11/16 19:49, Arnd Bergmann wrote:
> > > On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
> > >> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann <a...@arndb.de> wrote:
> > >>>
> > >>> Please merge these directly if you are happy with the result.
> > >>
> > >> I will take this.
> > > 
> > > Thanks a lot!
> > >  
> > >> I do see two warnings, but they both seem to be valid and recent,
> > >> though, so I have no issues with the spurious cases.
> > > 
> > > Ok, both of them should have my fixes coming your way already.
> > > 
> > >> Warning #1:
> > >>
> > >>   sound/soc/qcom/lpass-platform.c: In function 
> > >> ‘lpass_platform_pcmops_open’:
> > >>   sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
> > >> uninitialized in this function [-Wmaybe-uninitialized]
> > >> drvdata->substream[dma_ch] = substream;
> > >> ~~~^~~
> > >>
> > >> and 'dma_ch' usage there really is crazy and wrong. Broken by
> > >> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
> > > 
> > > Right, the patches crossed here, the bugfix patch that introduced
> > > this came into linux-next over the kernel summit, and the fix I
> > > sent on Tuesday made it into Mark Brown's tree on Wednesday but not
> > > before you pulled alsa tree. It should be fixed the next time you
> > > pull from the alsa tree, the commit is
> > > 
> > > 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
> > >  
> > >> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
> > >> know that storage_bytes (chip->read_size) has to be 2/4. Again,
> > >> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
> > >> Align 16 bit big endian value of raw reads"), so you didn't see it.
> > > 
> > > This is the one I mentioned in the commit message as one that
> > > is fixed in linux-next and that should make it in soon.
> > > 
> > >>   drivers/iio/temperature/maxim_thermocouple.c: In function
> > >> ‘maxim_thermocouple_read_raw’:
> > >>   drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
> > >> may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >> if (ret)
> > >>^
> > >>   drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
> > >> declared here
> > >> int ret;
> > >> ^~~
> > >>
> > >> and I guess that code can just initialize 'ret' to '-EINVAL' or
> > >> something to just make the theoretical "somehow we had a wrong
> > >> chip->read_size" case error out cleanly.
> > > 
> > > Right, that was my conclusion too. I sent the bugfix on Oct 25
> > > for linux-next but it didn't make it in until this Monday, after
> > > you pulled the patch that introduced it on Oct 29.
> > > 
> > > The commit in staging-testing is
> > > 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in 
> > > read()")
> > > 
> > > Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
> > > branch, so I suspect you were not planning to send this before the
> > > merge window. Could you make sure this ends up in v4.9 so we get
> > > a clean build when -Wmaybe-uninitialized gets enabled again?
> > I'll queue this up and send a pull to Greg tomorrow.
> > 
> > Was highly doubtful that a false warning suppression (be it an
> > understandable one) was worth sending mid cycle, hence it was
> > taking the slow route.
> 
> I can just cherry-pick this, no need to send a separate pull request.

Now done and sent to Linus, so all should be good here.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized

[Greg KH]

On Sat, Nov 12, 2016 at 01:27:12PM +, Jonathan Cameron wrote:
> On 11/11/16 19:49, Arnd Bergmann wrote:
> > On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
> >> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann  wrote:
> >>>
> >>> Please merge these directly if you are happy with the result.
> >>
> >> I will take this.
> > 
> > Thanks a lot!
> >  
> >> I do see two warnings, but they both seem to be valid and recent,
> >> though, so I have no issues with the spurious cases.
> > 
> > Ok, both of them should have my fixes coming your way already.
> > 
> >> Warning #1:
> >>
> >>   sound/soc/qcom/lpass-platform.c: In function 
> >> ‘lpass_platform_pcmops_open’:
> >>   sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
> >> uninitialized in this function [-Wmaybe-uninitialized]
> >> drvdata->substream[dma_ch] = substream;
> >> ~~~^~~
> >>
> >> and 'dma_ch' usage there really is crazy and wrong. Broken by
> >> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
> > 
> > Right, the patches crossed here, the bugfix patch that introduced
> > this came into linux-next over the kernel summit, and the fix I
> > sent on Tuesday made it into Mark Brown's tree on Wednesday but not
> > before you pulled alsa tree. It should be fixed the next time you
> > pull from the alsa tree, the commit is
> > 
> > 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
> >  
> >> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
> >> know that storage_bytes (chip->read_size) has to be 2/4. Again,
> >> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
> >> Align 16 bit big endian value of raw reads"), so you didn't see it.
> > 
> > This is the one I mentioned in the commit message as one that
> > is fixed in linux-next and that should make it in soon.
> > 
> >>   drivers/iio/temperature/maxim_thermocouple.c: In function
> >> ‘maxim_thermocouple_read_raw’:
> >>   drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
> >> may be used uninitialized in this function [-Wmaybe-uninitialized]
> >> if (ret)
> >>^
> >>   drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
> >> declared here
> >> int ret;
> >> ^~~
> >>
> >> and I guess that code can just initialize 'ret' to '-EINVAL' or
> >> something to just make the theoretical "somehow we had a wrong
> >> chip->read_size" case error out cleanly.
> > 
> > Right, that was my conclusion too. I sent the bugfix on Oct 25
> > for linux-next but it didn't make it in until this Monday, after
> > you pulled the patch that introduced it on Oct 29.
> > 
> > The commit in staging-testing is
> > 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in 
> > read()")
> > 
> > Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
> > branch, so I suspect you were not planning to send this before the
> > merge window. Could you make sure this ends up in v4.9 so we get
> > a clean build when -Wmaybe-uninitialized gets enabled again?
> I'll queue this up and send a pull to Greg tomorrow.
> 
> Was highly doubtful that a false warning suppression (be it an
> understandable one) was worth sending mid cycle, hence it was
> taking the slow route.

I can just cherry-pick this, no need to send a separate pull request.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] hwrng: chaoskey - Add support for Araneus Alea I USB RNG

[Greg KH]

On Wed, Jun 08, 2016 at 08:49:55AM +0800, Herbert Xu wrote:
> On Tue, Jun 07, 2016 at 03:03:11PM -0700, Greg KH wrote:
> >
> > Ok, but usually drivers/usb/misc/ patches go through my tree :)
> 
> Sorry.  But I do wonder whether this driver should be moved as
> it is just an hwrng device like every other driver under hw_random,
> except for the fact that it happens to be a USB physical device.

That's fine with me, but it uses the usb misc device major number,
unlike other hw_random drivers, so that might get messy.

It's your call...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] hwrng: chaoskey - Add support for Araneus Alea I USB RNG

[Greg KH]

On Tue, Jun 07, 2016 at 06:53:45PM +0800, Herbert Xu wrote:
> On Fri, Jun 03, 2016 at 12:13:06PM +0100, Bob Ham wrote:
> > Two patches to add support for the Araneus Alea I USB RNG to the
> > chaoskey driver.  The first simply includes the Alea I as a device,
> > the second fixes an issue with the timeout on the first read.
> > 
> > Bob Ham (2):
> >   hwrng: chaoskey - Add support for Araneus Alea I USB RNG
> >   hwrng: chaoskey - Fix URB warning due to timeout on Alea
> > 
> >  drivers/usb/misc/Kconfig| 11 ++-
> >  drivers/usb/misc/chaoskey.c | 21 +++--
> >  2 files changed, 25 insertions(+), 7 deletions(-)
> 
> Both applied.  Thanks.

Ok, but usually drivers/usb/misc/ patches go through my tree :)

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: gcm - Fix rfc4543 decryption crash

[Greg KH]

On Fri, Mar 18, 2016 at 10:42:40PM +0800, Herbert Xu wrote:
> This bug has already bee fixed upstream since 4.2.  However, it
> was fixed during the AEAD conversion so no fix was backported to
> the older kernels.

What was the commit id of that fix?

> 
> When we do an RFC 4543 decryption, we will end up writing the
> ICV beyond the end of the dst buffer.  This should lead to a
> crash but for some reason it was never noticed.
> 
> This patch fixes it by only writing back the ICV for encryption.
> 
> Fixes: d733ac90f9fe ("crypto: gcm - fix rfc4543 to handle async...")
> Reported-by: Patrick Meyer 
> Signed-off-by: Herbert Xu 

What stable kernel(s) do you want this in?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: crypto regression in 4.1.18: Check that kernel supports aes-cbc-essiv:sha256 cipher

[Greg KH]

On Tue, Mar 01, 2016 at 01:32:05AM +, Eric Wheeler wrote:
> Hello all,
> 
> We updated from 4.1.17 to 4.1.18 (same .config) and now get the following 
> error when trying to open a LUKS volume.  We've reverted to 4.1.17 and it 
> still works, so except that I'm not sure which commit caused the problem, 
> it is likely one of the recent commits:
> 
> When we `cryptsetup luksOpen` the volume we get this:
> 
>   Check that kernel supports aes-cbc-essiv:sha256 cipher (check syslog for 
> more info).  
> 
> Note that I do not see any dm-crypto commits so I don't think its a 
> dm-crypto issue. 
> 
> I'm happy to test, but can someone who knows the crypto stack suggest 
> which of these patches I should focus on for testing? These are the 
> commits in 4.1.18 that could be relevant:

There are some pending crypto backports to resolve this that hopefully
will show up in the next release, whenever Sasha gets to it.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Re: Broken userspace crypto in linux-4.1.18

[Greg KH]

On Wed, Feb 24, 2016 at 09:54:48AM +0100, Milan Broz wrote:
> On 02/24/2016 09:32 AM, Jiri Slaby wrote:
> >> +  af_alg_release_parent(sk);
> > 
> > and this occurs to me like a double release?
> 
> yes, my copy mistake.

Which is why I want the real patches backported please.  Whenever we do
a "just this smaller patch" for a stable kernel, it is ALWAYS wrong.

Please backport the patches in a correct way so that we can apply
them...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Re: Broken userspace crypto in linux-4.1.18

[Greg KH]

On Wed, Feb 24, 2016 at 01:10:55AM +0100, Thomas D. wrote:
> Hi,
> 
> I have applied Milan's patch on top of 4.1.18. I can reboot and open all
> of my LUKS-encrypted disks. "cryptsetup benchmark" also works.
> 
> However, don't we need all the recent changes from
> "crypto/algif_skcipher.c", too?

Can someone just backport the full patches in a proper format that I can
apply them in for the 3.14 and 3.10 kernels?  I told people that they
failed to apply, or at least I thought I did...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] staging: lustre: Use ahash

[Greg KH]

On Mon, Feb 01, 2016 at 09:36:53PM +0800, Herbert Xu wrote:
> This patch replaces uses of the long obsolete hash interface with
> ahash.
> 
> Signed-off-by: Herbert Xu 
> ---
> 
>  drivers/staging/lustre/lustre/libcfs/linux/linux-crypto.c |   92 
> +++---
>  1 file changed, 52 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-crypto.c 
> b/drivers/staging/lustre/lustre/libcfs/linux/linux-crypto.c
> index 079d50e..94c01aa 100644
> --- a/drivers/staging/lustre/lustre/libcfs/linux/linux-crypto.c
> +++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-crypto.c
> @@ -27,7 +27,7 @@
>   * Copyright (c) 2012, Intel Corporation.
>   */
>  
> -#include 
> +#include 
>  #include 
>  #include "../../../include/linux/libcfs/libcfs.h"
>  #include "linux-crypto.h"
> @@ -38,9 +38,11 @@ static int cfs_crypto_hash_speeds[CFS_HASH_ALG_MAX];
>  
>  static int cfs_crypto_hash_alloc(unsigned char alg_id,
>const struct cfs_crypto_hash_type **type,
> -  struct hash_desc *desc, unsigned char *key,
> +  struct ahash_request **req,
> +  unsigned char *key,
>unsigned int key_len)
>  {
> + struct crypto_ahash *tfm;
>   int err = 0;
>  
>   *type = cfs_crypto_hash_type(alg_id);
> @@ -50,18 +52,23 @@ static int cfs_crypto_hash_alloc(unsigned char alg_id,
> alg_id, CFS_HASH_ALG_MAX);
>   return -EINVAL;
>   }
> - desc->tfm = crypto_alloc_hash((*type)->cht_name, 0, 0);
> + tfm = crypto_alloc_ahash((*type)->cht_name, 0, CRYPTO_ALG_ASYNC);
>  
> - if (desc->tfm == NULL)
> - return -EINVAL;
> -
> - if (IS_ERR(desc->tfm)) {
> + if (IS_ERR(tfm)) {
>   CDEBUG(D_INFO, "Failed to alloc crypto hash %s\n",
>  (*type)->cht_name);
> - return PTR_ERR(desc->tfm);
> + return PTR_ERR(tfm);
>   }
>  
> - desc->flags = 0;
> + *req = ahash_request_alloc(tfm, GFP_KERNEL);
> + if (!*req) {
> + CDEBUG(D_INFO, "Failed to alloc ahash_request for %s\n",
> +(*type)->cht_name);
> + crypto_free_ahash(tfm);
> + return -ENOMEM;
> + }
> +
> + ahash_request_set_callback(*req, 0, NULL, NULL);
>  
>   /** Shash have different logic for initialization then digest
>* shash: crypto_hash_setkey, crypto_hash_init
> @@ -70,23 +77,27 @@ static int cfs_crypto_hash_alloc(unsigned char alg_id,
>* cfs_crypto_hash_alloc.
>*/
>   if (key != NULL)
> - err = crypto_hash_setkey(desc->tfm, key, key_len);
> + err = crypto_ahash_setkey(tfm, key, key_len);
>   else if ((*type)->cht_key != 0)
> - err = crypto_hash_setkey(desc->tfm,
> + err = crypto_ahash_setkey(tfm,
>(unsigned char *)&((*type)->cht_key),
>(*type)->cht_size);
>  
>   if (err != 0) {
> - crypto_free_hash(desc->tfm);
> + crypto_free_ahash(tfm);
>   return err;
>   }
>  
>   CDEBUG(D_INFO, "Using crypto hash: %s (%s) speed %d MB/s\n",
> -(crypto_hash_tfm(desc->tfm))->__crt_alg->cra_name,
> -(crypto_hash_tfm(desc->tfm))->__crt_alg->cra_driver_name,
> +crypto_ahash_alg_name(tfm), crypto_ahash_driver_name(tfm),

This change breaks the build for me so I can't take this through my
tree.  I'm hoping you have a patch earlier in the series that adds these
functions, if so, feel free to take this through your tree:

Acked-by: Greg Kroah-Hartman 
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4.3.y] crypto: fix test vector for rsa

[Greg KH]

On Wed, Jan 13, 2016 at 08:57:40PM -0800, Tadeusz Struk wrote:
> Hi,
> After the fix to the asn1_decoder in commit: 0d62e9dd
> "ASN.1: Fix non-match detection failure on data overrun"
> the rsa algorithm is failing to register in 4.3 stable kernels with
> error: "alg: rsa: test failed on vector 4, err=-74"
> 
> This happens because the asn1 definition for the rsa key that has been
> added in 4.2 defined all 3 components of the key as non-optional, as
> the asn1_decoder before the fix was working fine for both the private
> and public keys.
> 
> This patch adds the missing (fake) component to one key vector to allow
> the algorithm to successfully register and be used with a valid private
> keys later. This is only to make the asn1_decoder successfully parse the
> key and the fake component is never used in the test as the vector is
> marked as public key.
> 
> This patch applies only to 4.3 kernels as the 4.2 version of asn1_decoder
> works fine with the asn1 definition.
> 4.4 is also ok because the akcipher interface has been changed, and
> the set_key function has been split into set_public_key and set_priv_key
> and there are two separate asn1 definitions for the two key formats
> with all the required components correctly defined (commit 22287b0).
> 
> Signed-off-by: Tadeusz Struk 
> ---
> diff --git a/crypto/testmgr.h b/crypto/testmgr.h
> index 64b8a80..450f30e 100644
> --- a/crypto/testmgr.h
> +++ b/crypto/testmgr.h
> @@ -270,7 +270,7 @@ static struct akcipher_testvec rsa_tv_template[] = {
>   .c_size = 256,
>   }, {
>   .key =
> - "\x30\x82\x01\x09" /* sequence of 265 bytes */
> + "\x30\x82\x01\x0C" /* sequence of 268 bytes */
>   "\x02\x82\x01\x00" /* modulus - integer of 256 bytes */
>   "\xDB\x10\x1A\xC2\xA3\xF1\xDC\xFF\x13\x6B\xED\x44\xDF\xF0\x02\x6D"
>   "\x13\xC7\x88\xDA\x70\x6B\x54\xF1\xE8\x27\xDC\xC3\x0F\x99\x6A\xFA"
> @@ -288,8 +288,9 @@ static struct akcipher_testvec rsa_tv_template[] = {
>   "\x55\xE6\x29\x69\xD1\xC2\xE8\xB9\x78\x59\xF6\x79\x10\xC6\x4E\xEB"
>   "\x6A\x5E\xB9\x9A\xC7\xC4\x5B\x63\xDA\xA3\x3F\x5E\x92\x7A\x81\x5E"
>   "\xD6\xB0\xE2\x62\x8F\x74\x26\xC2\x0C\xD3\x9A\x17\x47\xE6\x8E\xAB"
> - "\x02\x03\x01\x00\x01", /* public key - integer of 3 bytes */
> - .key_len = 269,
> + "\x02\x03\x01\x00\x01" /* public key - integer of 3 bytes */
> + "\x02\x01\x00", /* private key - integer of 1 byte */
> + .key_len = 272,
>   .m = "\x54\x85\x9b\x34\x2c\x49\xea\x2a",
>   .c =
>   "\xb2\x97\x76\xb4\xae\x3e\x38\x3c\x7e\x64\x1f\xcc\xa2\x7f\xf6\xbe"
> 

I need an ack from the maintainer of this subsystem before I can take a
patch that is not already upstream in Linus's tree...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/26] staging: rtl8192u: Use skcipher and ahash

[Greg KH]

On Sun, Jan 24, 2016 at 09:16:57PM +0800, Herbert Xu wrote:
> This patch replaces uses of blkcipher with skcipher, and the long
> obsolete hash interface with ahash.
> 
> Signed-off-by: Herbert Xu 
> ---
> 
>  drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c |   92 
> +++---
>  drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c  |   46 ---
>  2 files changed, 81 insertions(+), 57 deletions(-)

Acked-by: Greg Kroah-Hartman 
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/26] staging: rtl8192e: Replace uses of obsolete blkcipher and hash

[Greg KH]

On Sun, Jan 24, 2016 at 09:16:26PM +0800, Herbert Xu wrote:
> The interfaces blkcipher and hash are obsolete.  This patch replaces
> them with skcipher and ahash respectively.
> 
> Signed-off-by: Herbert Xu 
> ---
> 
>  drivers/staging/rtl8192e/rtllib_crypt_tkip.c |   99 
> ++-
>  drivers/staging/rtl8192e/rtllib_crypt_wep.c  |   48 +++--
>  2 files changed, 82 insertions(+), 65 deletions(-)

Acked-by: Greg Kroah-Hartman 
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Renaming dh895xcc mmp fw

[Greg KH]

On Mon, Dec 21, 2015 at 02:36:57PM +, Atta, Ahsan wrote:
> Renaming dh895xcc mmp fw
> 
> Signed-off-by: Ahsan Atta 
> ---
>  .../crypto/qat/qat_dh895xcc/adf_dh895xcc_hw_data.h |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.


--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: sun4i-ss: add missing statesize

[Greg KH]

On Tue, Nov 17, 2015 at 10:27:42PM +0800, Herbert Xu wrote:
> On Tue, Nov 17, 2015 at 03:26:03PM +0100, LABBE Corentin wrote:
> >
> > I have lots of user reporting that they cannot use the device since it wont 
> > load.
> > For me it need to go stable since it respect all "stable rules", simple, 
> > tested, minimal, and fix a real bug that bother people.
> 
> Well it will hit stable eventually.  I just don't feel comfortable
> with sending it in straight away.

You can't send it in "straight away", the rule is that it has to be in
Linus's tree first, don't worry, you are doing it correctly.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: qat - don't use userspace pointer

[Greg KH]

On Wed, Oct 21, 2015 at 12:30:45PM -0700, Tadeusz Struk wrote:
> Bugfix - don't dereference userspace pointer.
> 
> Signed-off-by: Tadeusz Struk 
> ---
>  drivers/crypto/qat/qat_common/adf_ctl_drv.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.


--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/2] crypto: qat - Fix for invalid dma mapping and numa

[Greg KH]

On Mon, Nov 17, 2014 at 07:43:08AM -0800, Tadeusz Struk wrote:
 On 10/24/2014 07:45 AM, Herbert Xu wrote:
  On Wed, Oct 15, 2014 at 07:25:45AM -0400, Prarit Bhargava wrote:
 
 
  On 10/15/2014 06:35 AM, Nikolay Aleksandrov wrote:
  On 14/10/14 03:24, Tadeusz Struk wrote:
  Hi,
  These two patches fix invalid (zero length) dma mapping and
  enforce numa configuration for maximum performance.
 
  Change log:
  v2 - Removed numa node calculation based bus number and use predefined
  functions instead.
 
  Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com
  ---
 
  Tadeusz Struk (2):
 crypto: qat - Prevent dma mapping zero length assoc data
 crypto: qat - Enforce valid numa configuration
 
 
drivers/crypto/qat/qat_common/adf_accel_devices.h |3 +-
drivers/crypto/qat/qat_common/adf_transport.c |   12 +---
drivers/crypto/qat/qat_common/qat_algs.c  |7 +++--
drivers/crypto/qat/qat_common/qat_crypto.c|8 +++--
drivers/crypto/qat/qat_dh895xcc/adf_admin.c   |2 +
drivers/crypto/qat/qat_dh895xcc/adf_drv.c |   32 
  -
drivers/crypto/qat/qat_dh895xcc/adf_isr.c |2 +
7 files changed, 32 insertions(+), 34 deletions(-)
 
 
  I just gave a quick run of these patches and they seem to fix the NUMA 
  issue and
  the 0 length warnings.
 
  Tested-by: Nikolay Aleksandrov niko...@redhat.com
 
  Thanks Nik :)
 
  Reviewed-by: Prarit Bhargava pra...@redhat.com
  
  Patch applied to crypto and I will push this to stable.
 
 Hi Greg,
 Any idea why this didn't make it to 3.17.3?

Because it showed up in Linus's tree _after_ 3.17.3-rc1 was released?
How can I go back in time?

confused,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/2] crypto: qat - Fix for invalid dma mapping and numa

[Greg KH]

On Mon, Nov 17, 2014 at 09:20:34AM -0800, Tadeusz Struk wrote:
 On 11/17/2014 08:59 AM, Greg KH wrote:
  Because it showed up in Linus's tree _after_ 3.17.3-rc1 was released?
  How can I go back in time?
 
 I thought it was already possible, no? :)
 So will it be in 3.17.4?

Possibly, if I get around to it, relax please...

greg burried in patches k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: crypto: algif - avoid excessive use of socket buffer in skcipher

[Greg KH]

On Mon, Nov 10, 2014 at 11:50:36AM +0100, Ondrej Kozina wrote:
 Hello,
 
 could you please add this patch (already landed in 3.18-rc1) to following
 stable kernels:
 
 3.17.x, 3.14.x, 3.12.x, 3.4.x, 3.2.x?
 
 The bugfix allows usage of crypto API socket on archs with PAGE_SIZE = 32
 KiB (I have a typo in original changelog).
 
 Some background for the bug (with reproducer and report from users)
 - http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg11787.html
 - http://bugzilla.redhat.com/show_bug.cgi?id=1160289

Now applied to .17, .14, and .10-stable kernels, thanks.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/25] Replace DEFINE_PCI_DEVICE_TABLE macro use

[Greg KH]

On Fri, Jul 18, 2014 at 12:22:13PM -0400, John W. Linville wrote:
 On Fri, Jul 18, 2014 at 05:26:47PM +0200, Benoit Taine wrote:
  We should prefer `const struct pci_device_id` over
  `DEFINE_PCI_DEVICE_TABLE` to meet kernel coding style guidelines.
  This issue was reported by checkpatch.
 
 Honestly, I prefer the macro -- it stands-out more.  Maybe the style
 guidelines and/or checkpatch should change instead?

The macro is horrid, no other bus has this type of thing just to save a
few characters in typing, so why should PCI be special in this regard
anymore?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/25] Replace DEFINE_PCI_DEVICE_TABLE macro use

[Greg KH]

On Fri, Jul 18, 2014 at 09:54:32AM -0700, James Bottomley wrote:
 On Fri, 2014-07-18 at 09:43 -0700, Greg KH wrote:
  On Fri, Jul 18, 2014 at 12:22:13PM -0400, John W. Linville wrote:
   On Fri, Jul 18, 2014 at 05:26:47PM +0200, Benoit Taine wrote:
We should prefer `const struct pci_device_id` over
`DEFINE_PCI_DEVICE_TABLE` to meet kernel coding style guidelines.
This issue was reported by checkpatch.
   
   Honestly, I prefer the macro -- it stands-out more.  Maybe the style
   guidelines and/or checkpatch should change instead?
  
  The macro is horrid, no other bus has this type of thing just to save a
  few characters in typing
 
 OK, so this is the macro:
 
 #define DEFINE_PCI_DEVICE_TABLE(_table) \
   const struct pci_device_id _table[]
 
 Could you explain what's so horrible?
 
 The reason it's useful today is that people forget the const (and
 sometimes the [] making it a true table instead of a pointer).  If you
 use the DEFINE_PCI_DEVICE_TABLE macro, the compile breaks if you use it
 wrongly (good) and you automatically get the correct annotations.

We have almost 1000 more uses of the non-macro version than the macro
version in the kernel today:
$ git grep -w DEFINE_PCI_DEVICE_TABLE | wc -l
262
$ git grep const struct pci_device_id | wc -l
1254

My big complaint is that we need to be consistant, either pick one or
the other and stick to it.  As the macro is the least used, it's easiest
to fix up, and it also is more consistant with all other kernel
subsystems which do not have such a macro.

As there is no need for the __init macro mess anymore, there's no real
need for the DEFINE_PCI_DEVICE_TABLE macro either.  I think checkpatch
will catch the use of non-const users for the id table already today, it
catches lots of other uses like this already.

  , so why should PCI be special in this regard
  anymore?
 
 I think the PCI usage dwarfs most other bus types now, so you could turn
 the question around.  However, I don't think majority voting is a good
 guide to best practise; lets debate the merits for their own sake.

Not really dwarf, USB is close with over 700 such structures:
$ git grep const struct usb_device_id | wc -l
725

And i2c is almost just as big as PCI:
$ git grep const struct i2c_device_id | wc -l
1223

So again, this macro is not consistent with the majority of PCI drivers,
nor with any other type of device id declaration in the kernel, which
is why I feel it should be removed.

And finally, the PCI documentation itself says to not use this macro, so
this isn't a new thing.  From Documentation/PCI/pci.txt:

The ID table is an array of struct pci_device_id entries ending with an
all-zero entry.  Definitions with static const are generally preferred.
Use of the deprecated macro DEFINE_PCI_DEVICE_TABLE should be avoided.

That wording went into the file last December, when we last talked about
this and everyone in that discussion agreed to remove the macro for the
above reasons.

Consistency matters.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Greg KH]

On Thu, Jul 17, 2014 at 05:18:15AM -0400, Theodore Ts'o wrote:

Minor nit:

 @@ -469,6 +471,8 @@ static struct entropy_store nonblocking_pool = {
   push_to_pool),
  };
  
 +DECLARE_COMPLETION(urandom_initialized);
 +

static DECLARE_COMPLETION(urandom_initialized);

instead?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Greg KH]

On Thu, Jul 17, 2014 at 05:18:15AM -0400, Theodore Ts'o wrote:
 +SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 + unsigned int, flags)
 +{
 + int r;
 +
 + if (count  256)
 + return -EINVAL;
 +
 + if (flags  GRND_RANDOM) {
 + return _random_read(!(flags  GRND_BLOCK), buf, count);
 + }
 + if (flags  GRND_BLOCK) {
 + r = wait_for_completion_interruptible(urandom_initialized);
 + if (r)
 + return r;
 + } else if (!completion_done(urandom_initialized))
 + return -EAGAIN;
 + return urandom_read(NULL, buf, count, NULL);
 +}

You should fail if any other bits are set that you don't understand in
the flags value, to make it easier for newer kernels with more flags to
fail properly on old kernel releases.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/24] crypt: return actual error code on pcrypt_init

[Greg KH]

On Tue, Jun 17, 2014 at 10:29:02PM +0800, Jeff Liu wrote:
 From: Jie Liu jeff@oracle.com
 
 Return the actual error code if call kset_create_and_add() failed
 
 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Signed-off-by: Jie Liu jeff@oracle.com
 ---
  crypto/pcrypt.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

Herbert and David, please do not take this patch, it is incorrect.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 03/22] staging: crypto: skein: allow building statically

[Greg KH]

On Tue, Mar 18, 2014 at 08:58:49AM -0400, Jason Cooper wrote:
 On Mon, Mar 17, 2014 at 02:52:52PM -0700, Greg KH wrote:
  On Tue, Mar 11, 2014 at 09:32:35PM +, Jason Cooper wrote:
   These are the minimum changes required to get the code to build
   statically in the kernel.  It's necessary to do this first so that we
   can empirically determine that future cleanup patches aren't changing
   the generated object code.
   
   Signed-off-by: Jason Cooper ja...@lakedaemon.net
  
  This doesn't apply to my latest tree :(
 
 Ah, ok.  I'll rebase this series on the staging tree.
 
   --- a/drivers/staging/Makefile
   +++ b/drivers/staging/Makefile
   @@ -65,3 +65,4 @@ obj-$(CONFIG_XILLYBUS)  += xillybus/
obj-$(CONFIG_DGNC)   += dgnc/
obj-$(CONFIG_DGAP)   += dgap/
obj-$(CONFIG_MTD_SPINAND_MT29F)  += mt29f_spinand/
   +obj-$(CONFIG_CRYPTO_SKEIN) += skein/
  
  Care to align these up with the way this file is formatted?
 
 Of course, not sure what happened there (well, other than the obvious
 :-P)
 
  And I have no objection to taking the drivers/staging/ patches, the
  script looks useful, but I can't take it through the staging tree,
  sorry.
 
 Ok, I'll pull that out as a separate branch.  Do you mind taking a
 series that depends on a topic branch from another tree?  We do it a lot
 in arm-soc, but I'm not sure how popular that is elsewhere.

It's not a dependancy at all, and I don't take git pull requests for the
staging tree, just email patches, sorry.

So just resend these patches thanks.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 03/22] staging: crypto: skein: allow building statically

[Greg KH]

On Tue, Mar 11, 2014 at 09:32:35PM +, Jason Cooper wrote:
 These are the minimum changes required to get the code to build
 statically in the kernel.  It's necessary to do this first so that we
 can empirically determine that future cleanup patches aren't changing
 the generated object code.
 
 Signed-off-by: Jason Cooper ja...@lakedaemon.net

This doesn't apply to my latest tree :(

 --- a/drivers/staging/Makefile
 +++ b/drivers/staging/Makefile
 @@ -65,3 +65,4 @@ obj-$(CONFIG_XILLYBUS)  += xillybus/
  obj-$(CONFIG_DGNC)   += dgnc/
  obj-$(CONFIG_DGAP)   += dgap/
  obj-$(CONFIG_MTD_SPINAND_MT29F)  += mt29f_spinand/
 +obj-$(CONFIG_CRYPTO_SKEIN) += skein/

Care to align these up with the way this file is formatted?

And I have no objection to taking the drivers/staging/ patches, the
script looks useful, but I can't take it through the staging tree,
sorry.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] random: use device attach events for entropy

[Greg KH]

On Sun, Nov 03, 2013 at 08:33:12AM -0500, Theodore Ts'o wrote:
 Some investigation from FreeBSD shows that there is entropy available
 from measuring the device attach times:
 
 http://lists.randombit.net/pipermail/cryptography/2013-October/005689.html
 
 This will hopefully help us more quickly initialize the entropy pools
 while the system is booting (which is one of the times when we really
 badly need more entropy, especially in the case of the first boot
 after an consumer electronics device is taken out of the box).
 
 Measurements indicate this makes a huge improvement in the security of
 /dev/urandom during the boot sequence, so I'm cc'ing this to the
 stable kernel series.  Especially for embedded systems, which use
 flash and which don't necessarily have the network enabled when they
 first generate ssh or x.509 keys (sigh), this can be a big deal.
 
 Signed-off-by: Theodore Ts'o ty...@mit.edu
 Cc: sta...@vger.kernel.org
 ---
  drivers/base/core.c| 3 +++
  drivers/char/random.c  | 7 +++
  include/linux/random.h | 2 ++
  3 files changed, 12 insertions(+)
 
 diff --git a/drivers/base/core.c b/drivers/base/core.c
 index 8856d74..5e98fc3 100644
 --- a/drivers/base/core.c
 +++ b/drivers/base/core.c
 @@ -26,6 +26,7 @@
  #include linux/async.h
  #include linux/pm_runtime.h
  #include linux/netdevice.h
 +#include linux/random.h
  
  #include base.h
  #include power/power.h
 @@ -1156,6 +1157,8 @@ int device_add(struct device *dev)
   class_intf-add_dev(dev, class_intf);
   mutex_unlock(dev-class-p-mutex);
   }
 + add_device_attach_randomness(dev);
 +
  done:
   put_device(dev);
   return error;
 diff --git a/drivers/char/random.c b/drivers/char/random.c
 index f126bd2..51153fe 100644
 --- a/drivers/char/random.c
 +++ b/drivers/char/random.c
 @@ -829,6 +829,13 @@ void add_input_randomness(unsigned int type, unsigned 
 int code,
  }
  EXPORT_SYMBOL_GPL(add_input_randomness);
  
 +void add_device_attach_randomness(struct device *dev)
 +{
 + static struct timer_rand_state attach_state = { 0, };
 +
 + add_timer_randomness(attach_state, dev-devt);

Is it an issue that dev-devt will almost always be 0,0 for this
function call?  Why not use the name instead here, that's more unique
and every device has one, not just a tiny %.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] crypto: caam - map src buffer before access

[Greg KH]

On Fri, Sep 20, 2013 at 11:24:07PM +0530, Yashpal Dutta wrote:
 KMap the buffers before copying trailing bytes during hmac into a session
 temporary buffer. This is required if pinned buffer from user-space is send
 during hmac and is safe even if hmac request is generated from within kernel.
 
 Signed-off-by: Yashpal Dutta yashpal.du...@freescale.com
 Reviewed-by: Vakul Garg va...@freescale.com
 Reviewed-by: Horia Geanta horia.gea...@freescale.com 
 ---
 Patch covers review comments on first version of patch.
 ./scripts/checkpatch.pl --strict 
 0001-crypto-caam-map-src-buffer-before-access.patch 
 
 total: 0 errors, 0 warnings, 0 checks, 62 lines checked
 
 The patch fixes driver crashes when a pinned buffer from user-space is sent 
 to CAAM driver
 for HMAC processing.
 
  drivers/crypto/caam/sg_sw_sec4.h |   34 +-
  1 file changed, 25 insertions(+), 9 deletions(-)


formletter

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

/formletter
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/crypto/nx: fix init race, alignmasks and GCM bug

[Greg KH]

On Mon, Mar 04, 2013 at 10:44:57AM -0600, Kent Yoder wrote:
   Fixes a race on driver init with registering algorithms where the
 driver status flag wasn't being set before self testing started.
 
   Added the cra_alignmask field for CBC and ECB modes.
 
   Fixed a bug in GCM where AES block size was being used instead of
 authsize.
 
 Signed-off-by: Kent Yoder k...@linux.vnet.ibm.com
 ---
  drivers/crypto/nx/nx-aes-cbc.c | 1 +
  drivers/crypto/nx/nx-aes-ecb.c | 1 +
  drivers/crypto/nx/nx-aes-gcm.c | 2 +-
  drivers/crypto/nx/nx.c | 4 ++--
  4 files changed, 5 insertions(+), 3 deletions(-)


formletter

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

/formletter
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 15/17] powerpc: crypto: sysfs routines and docs for the nx device driver

[Greg KH]

On Wed, Mar 21, 2012 at 04:41:20PM -0500, Kent Yoder wrote:
 These routines add sysfs files supporting the Power7+ in-Nest encryption
 accelerator driver.
 
 Signed-off-by: Kent Yoder k...@linux.vnet.ibm.com
 ---
  Documentation/powerpc/pfo-nx-crypto.txt |   52 

Please put sysfs file information in Documentation/ABI/ where it
belongs.

  arch/powerpc/crypto/nx/nx_sysfs.c   |  194 
 +++
  2 files changed, 246 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/powerpc/pfo-nx-crypto.txt
  create mode 100644 arch/powerpc/crypto/nx/nx_sysfs.c
 
 diff --git a/Documentation/powerpc/pfo-nx-crypto.txt 
 b/Documentation/powerpc/pfo-nx-crypto.txt
 new file mode 100644
 index 000..63440d3
 --- /dev/null
 +++ b/Documentation/powerpc/pfo-nx-crypto.txt
 @@ -0,0 +1,52 @@
 +
 +Documentation for the sysfs interfaces provided by the nx-crypto driver, 
 built
 +in arch/powerpc/crypto/nx.
 +
 +The driver provides 2 sets of sysfs files, 1 for confirming that the device 
 is
 +actually being used and 1 for error detection.

Shouldn't the first just be debugfs files, as no normal user will ever
care about such a thing?

Actually, why are these sysfs files at all, how about all of this going
into debugfs?


 +Error Detection
 +===

snip

What can anyone do with any of these files?  What use are they to users?

 +Device Use
 +==

Again, what does a user care about these items for?

 +int
 +nx_sysfs_init(struct device_driver *drv)
 +{
 + int rc;
 +
 + rc = driver_create_file(drv, driver_attr_aes_ops);
 + if (rc)
 + goto out;

snip

Oh, ${DIETY}, no.  Please don't create files one by one, we do have
functions that do all of this for you automatically, why aren't you
using them?

 +void
 +nx_sysfs_fini(struct device_driver *drv)
 +{
 + driver_remove_file(drv, driver_attr_sync_ops);
 + driver_remove_file(drv, driver_attr_aes_bytes);
 + driver_remove_file(drv, driver_attr_aes_ops);
 + driver_remove_file(drv, driver_attr_sha256_bytes);
 + driver_remove_file(drv, driver_attr_sha256_ops);
 + driver_remove_file(drv, driver_attr_sha512_bytes);
 + driver_remove_file(drv, driver_attr_sha512_ops);

Same here, don't do this, do it all at once.

 +}

Who is calling these functions?  Where in the device lifecycle are the
files being created?  Did you just race userspace with how they are
created, or are you doing it properly?  (hint, odds are, as you are
trying to manually create and remove these by hand, you aren't doing it
properly...)

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/17] powerpc: crypto: nx driver code supporting nx encryption

[Greg KH]

On Wed, Mar 21, 2012 at 04:41:08PM -0500, Kent Yoder wrote:
 +static int nx_register_algs(void)
 +{
 + int rc = -1;
 +
 + if (nx_driver.of.flags != NX_OF_FLAG_MASK_READY)
 + goto out;
 +
 + memset(nx_driver.stats, 0, sizeof(struct nx_stats));
 +
 + rc = nx_sysfs_init(nx_driver.viodriver.driver);

Ok, that's not bad, you are doing it from the probe() function of your
bus.  As long as the bus got things right on when uevents get sent to
userspace, does it?

 +static struct vio_device_id nx_crypto_driver_ids[] __devinitdata = {
 + { ibm,sym-encryption-v1, ibm,sym-encryption },
 + { ,  }
 +};
 +MODULE_DEVICE_TABLE(vio, nx_crypto_driver_ids);
 +
 +/* driver state structure */
 +struct nx_crypto_driver nx_driver = {
 + .viodriver = {
 + .id_table = nx_crypto_driver_ids,
 + .probe = nx_probe,
 + .remove = nx_remove,
 + .driver = {
 + .name  = nx,
 + .owner = THIS_MODULE,
 + },

Really?  vio drivers are supposed to look like this with the .name and
.owner field manually being set in the static initialization of the
driver?  That's sad, and should be fixed, the vio core should do this
type of thing for you.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH 00/16] Crypto keys and module signing [ver #2]

[Greg KH]

On Tue, Nov 29, 2011 at 11:42:58PM +, David Howells wrote:
 
 Here are a set of patches that create a framework for using cryptographic keys
 within the kernel.  The patches can also be found at:
 
   
 http://git.kernel.org/?p=linux/kernel/git/dhowells/linux-modsign.git;a=shortlog;h=refs/heads/devel

Very nice stuff, thanks for pushing this, it's much needed.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: mv_cesa - fix hashing of chunks 1920 bytes

[Greg KH]

On Mon, Nov 14, 2011 at 04:41:43PM +0100, Phil Sutter wrote:
 This was broken by commit 7759995c75ae0cbd4c861582908449f6b6208e7a (yes,
 myself). The basic problem here is since the digest state is only saved
 after the last chunk, the state array is only valid when handling the
 first chunk of the next buffer. Broken since linux-3.0.
 
 Signed-off-by: Phil Sutter phil.sut...@viprinet.com
 ---

formletter

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

/formletter
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2.1 1/7] crypto: GnuPG based MPI lib - source files (part 1)

[Greg KH]

On Mon, Oct 17, 2011 at 12:11:37PM +0300, Kasatkin, Dmitry wrote:
 From Kernel Docbook
 
 Similar to functionEXPORT_SYMBOL()/function except that the
 symbols exported by functionEXPORT_SYMBOL_GPL()/function can
 only be seen by modules with a
 functionMODULE_LICENSE()/function that specifies a GPL
 compatible license.  It implies that the function is considered
 an internal implementation issue, and not really an interface.
 
 not really an interface
 
 Should it really be EXPORT_SYMBOL_GPL?

Yes.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2.6.37.2 1/1] crypto: Uninitialized variable fix.

[Greg KH]

On Fri, Feb 25, 2011 at 11:28:28PM +0530, Atul Sowani wrote:
 Array inter[4] was uninitialized in anubis_crypt() and was causing
 compiler warning. Added the array initialization.
 
 Signed-off-by: Atul Sowani sow...@gmail.com
 ---
 
 diff a/crypto/anubis.c b/crypto/anubis.c
 --- a/crypto/anubis.c   2011-02-25 04:39:00.0 +0530
 +++ b/crypto/anubis.c   2011-02-25 23:07:12.0 +0530
 @@ -578,7 +578,7 @@ static void anubis_crypt(u32 roundKey[AN
 __be32 *dst = (__be32 *)ciphertext;
 int i, r;
 u32 state[4];
 -   u32 inter[4];
 +   u32 inter[4] = { 0, 0, 0, 0 };

That's nice, it looks like a bug in the compiler, preinitializing this
variable doesn't fix any real bug.

sorry,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] PPC4xx: ADMA separating SoC specific functions

[Greg KH]

On Thu, Sep 30, 2010 at 05:57:10PM -0700, Dan Williams wrote:
 [ adding Greg ]
 
 On Thu, Sep 30, 2010 at 5:16 PM, Tirumala Marri tma...@apm.com wrote:
  Where ?iop_adma_alloc_slots() is implemented differently between
  iop13xx and iop3xx. ?In this case why does ppc440spe-adma.h exist? ?If
  it has code specific to ppe440spe it should just live in the ppe440spe
  C file. ?If it is truly generic it should move to the base adma.c
  implementation. ?If you want to reuse a ppe440spe routine just link to
  it.
  [Marri]That is how I started changing the code. And I see tons of warnings
  Saying Used but not defined or Defined but not used. How should I
  suppress
  Some functions from adma.c are used in ppc440spe-adma.c and some from
  ppc440spe-adma.c
  Are used in adma.c.
 
 This is part of defining a common interface.  Maybe look at the
 linkages of how the common ioat_probe() routine is used to support all
 three versions of its dma hardware.
 
  So I created intermediate file ppc440spe-adma.h with
  inlined
  Functions. In future this will be converted into ppc4xx_adma.h and move
  existing
  SoC specific stuff to ppc440spe-adma.c file.
 
 You definitely need to be able to resolve used but not defined and
 defined but not used warnings before tackling a driver conversion
 like this.  In light of this comment I wonder if it would be
 appropriate to submit your original driver, that just duplicated
 routines from the ppc440spe driver, to the -staging tree.  Then it
 would be available for someone familiar with driver conversions to
 take a shot at unifying.
 
 Greg, is this an appropriate use of -staging?

Possibly, but I really don't like duplication if possible.  What's
keeping this code from being fixed up now properly?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][trivial] crypto: tcrypt - reduce stack size

[Greg KH]

On Wed, Feb 25, 2009 at 03:34:11PM +0100, Frank Seidel wrote:
 Thiago Galesi wrote:
  If you write static char output[1024]; (even inside a function) it's
  not allocated on the stack.
 
 Oh, yes i misunderstood Herbert, sorry. But anyway isn't
 it preferred to kmalloc such arrays?
 Greg, i thought it was you who told me so, is that right?

Generally, yes, it is preferred.  But if it can be static, that's fine
too.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [stable] Crypto Fixes for 2.6.26

[Greg KH]

On Tue, Jul 15, 2008 at 07:07:47PM +0200, Oliver Pinter wrote:
 [CC stable]
 
 On 7/10/08, Herbert Xu [EMAIL PROTECTED] wrote:
  Hi Linus:
 
  Just found a regression introduced back in 2.6.25.  This causes
  packet leaks when IPsec is in use.  When that leak grows it can
  eventually lead to hung connections (as their send buffers are
  used up) or worse.
 
  Please pull from
 
  git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git

Herbert, can you let me know what the git commit id is for this fix?

And do you want it added to the .25 and .26 -stable releases?

Oliver, thanks for the pointer.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html