Re: [PATCH] drivers/crypto/nx: Add CRC and validation support for nx842

[Dan Streetman]

On Wed, Sep 23, 2015 at 5:54 AM, Herbert Xu  wrote:
> On Tue, Sep 22, 2015 at 11:08:22AM -0400, Dan Streetman wrote:
>>
>> you think we should just strip out the 842-nx alignment/sizing code
>> and change it to fallback to the sw driver?
>
> Right, if the only intended user can provide aligned input then
> by all means make the unaligned case use the software fallback as
> there is no point in investing any effort in making it fast.

ok sounds good.  thanks!

>
> Cheers,
> --
> Email: Herbert Xu 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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

crypto_cbc_encrypt query

[pavi1729]

Hi,
  Shouldn't "walk" be memset ?

FILE:crypto/cbc.c
FUNCTION:  crypto_cbc_encrypt

"walk" is local variable and its uninitialized in
"crypto_cbc_encrypt". The same is passed to
"blkcipher_walk_virt" which does below

walk->flags &= ~BLKCIPHER_WALK_PHYS;

So neither 'walk' nor 'walk->flags' is memset or init to a proper value.

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

[PATCH 1/1] Disable fips-allowed for non-FIPS authenc ciphers

[John Haxby]

Tests that contain non-FIPS ciphers and hashes cannot themselves be
.fips-allowed because they will necessarily fail.

Signed-off-by: John Haxby 
---
 crypto/testmgr.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index fa18753..68799dc 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -2080,7 +2080,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(md5),ecb(cipher_null))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2110,7 +2109,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha1),cbc(des))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2138,7 +2136,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha1),ecb(cipher_null))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2158,7 +2155,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha224),cbc(des))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2200,7 +2196,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha256),cbc(des))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2228,7 +2223,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha384),cbc(des))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2270,7 +2264,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha512),cbc(des))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
-- 
2.4.3

--
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

[PATCHv2 1/1] Disable fips-allowed for authenc() and des() ciphers

[John Haxby]

No authenc() ciphers are FIPS approved, nor is ecb(des).
After the end of 2015, ansi_cprng will also be non-approved.

Signed-off-by: John Haxby 
---
 crypto/testmgr.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index fa18753..523c9b9 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -2080,7 +2080,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(md5),ecb(cipher_null))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2096,7 +2095,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha1),cbc(aes))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2110,7 +2108,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha1),cbc(des))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2124,7 +2121,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha1),cbc(des3_ede))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2138,7 +2134,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha1),ecb(cipher_null))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2158,7 +2153,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha224),cbc(des))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2172,7 +2166,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha224),cbc(des3_ede))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2186,7 +2179,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha256),cbc(aes))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2200,7 +2192,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha256),cbc(des))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2214,7 +2205,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha256),cbc(des3_ede))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2228,7 +2218,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha384),cbc(des))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2242,7 +2231,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha384),cbc(des3_ede))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2256,7 +2244,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha512),cbc(aes))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2270,7 +2257,6 @@ static const struct alg_test_desc alg_test_descs[] = {
}, {
.alg = "authenc(hmac(sha512),cbc(des))",
.test = alg_test_aead,
-   .fips_allowed = 1,
.suite = {
.aead = {
.enc = {
@@ -2284,7 +2270,6 @@ static const struct alg_test_desc alg_test_descs[] = {
   

[PATCHv2 0/1] fips-allowed tests fail with non-FIPS ciphers

[John Haxby]

Hello All,

"Make fips=1 work on 4.1", they said, wittily, "it'll be easy."

I suppose it wasn't that complicated, although I seem to be unearthing
other problems as I go along.  The first problem was dracut (and I owe
an upstream patch for that) and the second problem was tcrypt.

The tcrypt module was failing on authenc ciphers that wrap non-FIPS
ciphers and hashes.  These ones in fact:

authenc(hmac(md5),ecb(cipher_null))
authenc(hmac(sha1),cbc(des))
authenc(hmac(sha1),ecb(cipher_null))
authenc(hmac(sha224),cbc(des))
authenc(hmac(sha256),cbc(des))
authenc(hmac(sha384),cbc(des))
authenc(hmac(sha512),cbc(des))

Stepham Mueller pointed out that no authenc() ciphers are FIPS
approved and that ecb(des) also managed to get .fips_approved set.
The following patch removes fips_allowed for all those patches.

Again, Stephan pointed out that ansi_cprng will need to be taken off
the allowed list at the end of the year.  This patch doesn't pre-empt
that.

jch

John Haxby (1):
  Disable fips-allowed for authenc() and des() ciphers

 crypto/testmgr.c | 16 
 1 file changed, 16 deletions(-)

-- 
2.4.3

--
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/1] Disable fips-allowed for non-FIPS authenc ciphers

[John Haxby]

On 24/09/15 17:58, Stephan Mueller wrote:
> Am Donnerstag, 24. September 2015, 17:02:03 schrieb John Haxby:
> 
> Hi John,
> 
>> >Tests that contain non-FIPS ciphers and hashes cannot themselves be
>> >.fips-allowed because they will necessarily fail.
>> >
>> >Signed-off-by: John Haxby 
> This is a good finding.
> 
> In fact, all authenc() ciphers are not FIPS approved ciphers.
> 
> The flag for that should be removed for all of those.
> 
> After checking in detail, the following FIPS flags should be removed as well:
> 
> - ecb(des)
> 
> - ansi_cprng (at least at the end of this year)

Thanks Stephan.

Updated patch on its way.

jch
--
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/1] Disable fips-allowed for non-FIPS authenc ciphers

[Stephan Mueller]

Am Donnerstag, 24. September 2015, 17:02:03 schrieb John Haxby:

Hi John,

>Tests that contain non-FIPS ciphers and hashes cannot themselves be
>.fips-allowed because they will necessarily fail.
>
>Signed-off-by: John Haxby 

This is a good finding.

In fact, all authenc() ciphers are not FIPS approved ciphers.

The flag for that should be removed for all of those.

After checking in detail, the following FIPS flags should be removed as well:

- ecb(des)

- ansi_cprng (at least at the end of this year)


Ciao
Stephan
--
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 8/9] zram: use crypto API for compression

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 02:19:03PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> > -static struct zcomp_backend *find_backend(const char *compress)
> > +static const char *find_backend(const char *compress)
> >  {
> > int i = 0;
> > while (backends[i]) {
> > -   if (sysfs_streq(compress, backends[i]->name))
> > +   if (sysfs_streq(compress, backends[i]) &&
> > +   crypto_has_comp(compress, 0, 0))
> 
> ok, just for note. zcomp does sysfs_streq(), because sysfs data
> usually contain a trailing new line, crypto_has_comp() does strcmp().

Okay. Will check.

> 
> 
> >  int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > -   const unsigned char *src, size_t *dst_len)
> > +   const unsigned char *src, unsigned int *dst_len)
> >  {
> > -   return comp->backend->compress(src, zstrm->buffer, dst_len,
> > -   zstrm->private);
> > +   *dst_len = PAGE_SIZE << 1;
> > +
> 
> hm... wouldn't it be better to say crypto api that we have a PAGE_SIZE
> buffer instead of PAGE_SIZE << 1, so in case of buffer overrun (or
> whatever is the correct term here) it will stop compression earlier
> (well, possibly)? zram will drop compressed data larger than PAGE_SIZE
> anyway, so if passing a smaller buffer size can save us CPU time then
> let's do it.

It can be implemented and maybe good way to go. But, in this patchset,
it isn't needed. It is better to do in separate patch.

> 
> > +   return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE,
> > +   zstrm->buffer, dst_len);
> >  }
> >  
> >  int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm,
> > const unsigned char *src,
> > -   size_t src_len, unsigned char *dst)
> > +   unsigned int src_len, unsigned char *dst)
> >  {
> > -   return comp->backend->decompress(src, src_len, dst);
> > +   unsigned int size = PAGE_SIZE;
> > +
> > +   return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, );
> 
>    tab?
> 
> >  }
> >  
> >  void zcomp_destroy(struct zcomp *comp)
> > @@ -359,7 +365,7 @@ void zcomp_destroy(struct zcomp *comp)
> >  struct zcomp *zcomp_create(const char *compress, int max_strm)
> >  {
> > struct zcomp *comp;
> > -   struct zcomp_backend *backend;
> > +   const char *backend;
> 
> rebase against the current linux-next. this and the next patch do not
> apply cleanly. the function was touched recently: '+int error'.

Okay.

> 
> >  
> > backend = find_backend(compress);
> > if (!backend)
> > diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
> > index 4c09c01..4f9df8e 100644
> > --- a/drivers/block/zram/zcomp.h
> > +++ b/drivers/block/zram/zcomp.h
> > @@ -11,38 +11,22 @@
> >  #define _ZCOMP_H_
> >  
> >  #include 
> > +#include 
> >  
> >  struct zcomp_strm {
> > +   struct crypto_comp *tfm;
> > +
> > /* compression/decompression buffer */
> > void *buffer;
> > -   /*
> > -* The private data of the compression stream, only compression
> > -* stream backend can touch this (e.g. compression algorithm
> > -* working memory)
> > -*/
> > -   void *private;
> > +
> > /* used in multi stream backend, protected by backend strm_lock */
> > struct list_head list;
> >  };
> >  
> > -/* static compression backend */
> > -struct zcomp_backend {
> > -   int (*compress)(const unsigned char *src, unsigned char *dst,
> > -   size_t *dst_len, void *private);
> > -
> > -   int (*decompress)(const unsigned char *src, size_t src_len,
> > -   unsigned char *dst);
> > -
> > -   void *(*create)(void);
> > -   void (*destroy)(void *private);
> > -
> > -   const char *name;
> > -};
> > -
> >  /* dynamic per-device compression frontend */
> >  struct zcomp {
> > void *stream;
> > -   struct zcomp_backend *backend;
> > +   const char *backend;
> 
>   ^^ what for?

Will remove.

Thanks.
--
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 8/9] zram: use crypto API for compression

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 12:45:12PM +0900, Minchan Kim wrote:
> Hi Joonsoo,
> 
> On Fri, Sep 18, 2015 at 02:19:23PM +0900, Joonsoo Kim wrote:
> > Until now, zram uses compression algorithm through direct call
> > to core algorithm function, but, it has drawback that we need to add
> > compression algorithm manually to zram if needed. Without this work,
> > we cannot utilize various compression algorithms in the system.
> > Moreover, to add new compression algorithm, we need to know how to use it
> > and this is somewhat time-consuming.
> > 
> > When I tested new algorithms such as zlib, these problems make me hard
> > to test them. To prevent these problem in the future, I think that
> > using crypto API for compression is better approach and this patch
> > implements it.
> > 
> > The reason we need to support vairous compression algorithms is that
> > zram's performance is highly depend on workload and compression algorithm
> > and architecture. Every compression algorithm has it's own strong point.
> > For example, zlib is the best for compression ratio, but, it's
> > (de)compression speed is rather slow. Against my expectation, in my kernel
> > build test with zram swap, in low-memory condition on x86, zlib shows best
> > performance than others. In this case, I guess that compression ratio is
> > the most important factor. Unlike this situation, on ARM, maybe fast
> > (de)compression speed is the most important because it's computation speed
> > is slower than x86.
> > 
> > We can't expect what algorithm is the best fit for one's system, because
> > it needs too complex calculation. We need to test it in case by case and
> > easy to use new compression algorithm by this patch will help
> > that situation.
> > 
> > There is one problem that crypto API requires tfm object to (de)compress
> > something and zram abstract it on zstrm which is very limited resource.
> > If number of zstrm is set to low and is contended, zram's performace could
> > be down-graded due to this change. But, following patch support to use
> > crypto decompress_noctx API and would restore the performance as is.
> > 
> > Signed-off-by: Joonsoo Kim 
> > ---
> >  drivers/block/zram/Kconfig |  8 +++
> >  drivers/block/zram/Makefile|  4 +---
> >  drivers/block/zram/zcomp.c | 54 
> > +++---
> >  drivers/block/zram/zcomp.h | 30 ++-
> >  drivers/block/zram/zcomp_lz4.c | 47 
> >  drivers/block/zram/zcomp_lz4.h | 17 -
> >  drivers/block/zram/zcomp_lzo.c | 47 
> >  drivers/block/zram/zcomp_lzo.h | 17 -
> >  drivers/block/zram/zram_drv.c  |  6 ++---
> >  9 files changed, 44 insertions(+), 186 deletions(-)
> >  delete mode 100644 drivers/block/zram/zcomp_lz4.c
> >  delete mode 100644 drivers/block/zram/zcomp_lz4.h
> >  delete mode 100644 drivers/block/zram/zcomp_lzo.c
> >  delete mode 100644 drivers/block/zram/zcomp_lzo.h
> > 
> > diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> > index 386ba3d..7619bed 100644
> > --- a/drivers/block/zram/Kconfig
> > +++ b/drivers/block/zram/Kconfig
> > @@ -1,8 +1,7 @@
> >  config ZRAM
> > tristate "Compressed RAM block device support"
> > depends on BLOCK && SYSFS && ZSMALLOC
> > -   select LZO_COMPRESS
> > -   select LZO_DECOMPRESS
> > +   select CRYPTO_LZO
> > default n
> > help
> >   Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
> > @@ -18,9 +17,8 @@ config ZRAM
> >  config ZRAM_LZ4_COMPRESS
> > bool "Enable LZ4 algorithm support"
> > depends on ZRAM
> > -   select LZ4_COMPRESS
> > -   select LZ4_DECOMPRESS
> > +   select CRYPTO_LZ4
> 
> It is out of my expectation.
> 
> When I heard crypto support for zRAM first, I imagined zram user can
> use any crypto compressor algorithm easily if system has the algorithm.
> IOW, I expect zRAM shouldn't bind CONFIG_CRYPTO_ALGONAME statically
> except the default one(ie, CONFIG_CRYPTO_LZO) like zswap and It seems
> you did in eariler version.
> 
> You seem to have a trouble to adapt crypto to current comp_algorithm
> because crypto doesn't export any API to enumerate algorithm name
> while zram should export supporting algorithm name via comp_algorithm
> and I heard crypto maintainer doesn't want to export it. Instead,
> user can use /proc/crypto to know what kinds of compressor system
> can support.
> 
> Hmm,
> At the first glance, I was about to say "let's sort it out with
> futher patches" but I changed my mind. ;-).
> 
> We should sort it out before you are adding zlib support(ie, please
> include zlib support patch with number data in this patchset). Otherwise,
> we should add new hard-wired stuff for zlib like lzo, lz4 to
> comp_algorithm and will depreate soon.
> 
> My idea is ABI change of comp_algorithm. Namely, let's deprecate it
> and guide to use /proc/crypto to show available compressor.
> 

Re: [PATCH v3 1/9] crypto: introduce decompression API that can be called via sharable tfm object

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 02:38:59PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> > @@ -61,7 +61,8 @@ static struct crypto_alg alg = {
> > .cra_module = THIS_MODULE,
> > .cra_u  = { .compress = {
> > .coa_compress   = crypto842_compress,
> > -   .coa_decompress = crypto842_decompress } }
> > +   .coa_decompress = crypto842_decompress,
> > +   .coa_decompress_noctx   = NULL } }
> >  };
> >  
> >  static int __init crypto842_mod_init(void)
> > diff --git a/crypto/compress.c b/crypto/compress.c
> > index c33f076..abb36a8 100644
> > --- a/crypto/compress.c
> > +++ b/crypto/compress.c
> > @@ -33,12 +33,21 @@ static int crypto_decompress(struct crypto_tfm *tfm,
> >dlen);
> >  }
> >  
> > +static int crypto_decompress_noctx(struct crypto_tfm *tfm,
> > +   const u8 *src, unsigned int slen,
> > +   u8 *dst, unsigned int *dlen)
> > +{
> > +   return tfm->__crt_alg->cra_compress.coa_decompress_noctx(src, slen,
> > +   dst, dlen);
> > +}
> 
> 
> hm... well... sorry, if irrelevant.
> if the algorithm can have a _noctx() decompression function, does it
> automatically guarantee that this algorithm never dereferences a passed
> `struct crypto_tfm *tfm' pointer in its decompress function? in other words,
> can we simply pass that `shared tfm pointer' to the existing decompress
> function instead of defining new symbols, new callbacks, etc.?
> 
>   cot_decompress_noctx()  ==  cot_decompress(shared_ftm) ?
> 
> just a thought.

Will think it if I implement next version in this way. Due to Herbert
comment, interface will be changed so much in next version.

> 
> [..]
> > +struct crypto_comp *crypto_alloc_comp_noctx(const char *alg_name,
> > +   u32 type, u32 mask)
> > +{
> > +   struct crypto_comp *comp;
> > +   struct crypto_tfm *tfm;
> > +   struct compress_tfm *ops;
> > +
> > +   comp = crypto_alloc_comp(alg_name, type, mask);
> > +   if (IS_ERR(comp))
> > +   return comp;
> > +
> > +   tfm = crypto_comp_tfm(comp);
> > +   if (!tfm->__crt_alg->cra_compress.coa_decompress_noctx) {
> > +   crypto_free_comp(comp);
> > +   return ERR_PTR(-EINVAL);
> 
>   -ENOMEM?

No, it's failure isn't related to NOMEM. Just not support it.

Thanks.

--
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 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 04:56:00PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> > +   /*
> > +* Prepare to use crypto decompress_noctx API. One tfm is required
> > +* to initialize crypto algorithm properly and fetch corresponding
> > +* function pointer. But, it is sharable for multiple concurrent
> > +* decompress users.
> > +*/
> 
> if comp algorithm require zstrm for both compression and decompression,
> then there seems to be no need in allocating sharable ->tfm_noctx, we
> will never use it.
> 

Yes, in that case, NULL will be returned. I should describe it on
somewhere.

Thanks.
--
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: [PATCHv2 1/1] Disable fips-allowed for authenc() and des() ciphers

[Stephan Mueller]

Am Donnerstag, 24. September 2015, 18:24:35 schrieb John Haxby:

Hi John,

>No authenc() ciphers are FIPS approved, nor is ecb(des).
>After the end of 2015, ansi_cprng will also be non-approved.
>
>Signed-off-by: John Haxby 

Acked-by: Stephan Mueller 


Ciao
Stephan
--
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 0/9] zram: introduce crypto decompress noctx API and use it on zram

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 12:58:12PM +0900, Minchan Kim wrote:
> On Fri, Sep 18, 2015 at 02:19:15PM +0900, Joonsoo Kim wrote:
> > This patchset makes zram to use crypto API in order to support
> > more compression algorithm.
> > 
> > The reason we need to support vairous compression algorithms is that
> > zram's performance is highly depend on workload and compression algorithm
> > and architecture. Every compression algorithm has it's own strong point.
> > For example, zlib is the best for compression ratio, but, it's
> > (de)compression speed is rather slow. Against my expectation, in my kernel
> > build test with zram swap, in low-memory condition on x86, zlib shows best
> > performance than others. In this case, I guess that compression ratio is
> > the most important factor. Unlike this situation, on ARM, maybe fast
> > (de)compression speed is the most important because it's computation speed
> > is slower than x86.
> 
> Fair enough. lzo and lz4 cannot cover all of workloads so surely, there are
> workloads other compressor algorithm can win. As well, we could support
> H/W compressor with crypto support so I think crypto is a way to go.

Hello, Minchan.

Good!

> 
> One thing I have a concern is I don't want to bind some of compressor
> algorithm to zram statically. User can see what kinds of crypto compressor
> support in his system via /proc/crypto and use it dynamically.
> I hope zram supports it.

Okay. I will handle it next version.

> > 
> > Anyway, there is a concern from Sergey to use crypto API in zram. Current
> > crypto API has a limitation that always require tfm object to (de)compress
> > something because some of (de)compression function requires scratch buffer
> > embedded on tfm even if some of (de)compression function doesn't
> > require it. Due to above reason, using crypto API rather than calling
> 
> Nice catch from Sergey!
> 
> > compression library directly causes more memory footprint and this is
> > why zram doesn't use crypto API before.
> > 
> > In this patchset, crypto compress noctx API is introduced to reduce memory
> > footprint caused by maintaining multiple tfm and zram uses it. Before
> > noctx API, zram's performace is down-graded if tfm is insufficient. But,
> > after applying noctx API, performace is restored.
> > 
> > This addresses Sergey's concern perfectly and provides possibility to use
> > various compression algorithm in zram.
> > 
> > Following is zram's read performance number.
> > 
> > * iozone -t 4 -R -r 16K -s 60M -I +Z -i 0 -i 1
> > * max_stream is set to 1
> > * Output is in Kbytes/sec
> > 
> > zram-base vs zram-crypto vs zram-crypto-noctx
> > 
> > Read10411701.88 6426911.62  9423894.38 
> > Re-read 10017386.62 6428218.88  1163.50 
> 
> Another patch and number we need to merge this patchset is zlib support
> patchset and number you had experiement.

Okay. Will include.

Thanks.

> > 
> > Thanks.
> > 
> > Joonsoo Kim (7):
> >   crypto: introduce decompression API that can be called via sharable
> > tfm object
> >   crypto/lzo: support decompress_noctx
> >   crypyo/lz4: support decompress_noctx
> >   crypto/lz4hc: support decompress_noctx
> >   crypto/842: support decompress_noctx
> >   zram: use crypto API for compression
> >   zram: use crypto decompress_noctx API
> > 
> > Sergey Senozhatsky (2):
> >   zram: make stream find and release functions static
> >   zram: pass zstrm down to decompression path
> > 
> >  crypto/842.c   |   9 +++-
> >  crypto/compress.c  |  36 +++
> >  crypto/crypto_null.c   |   3 +-
> >  crypto/deflate.c   |   3 +-
> >  crypto/lz4.c   |   9 +++-
> >  crypto/lz4hc.c |   9 +++-
> >  crypto/lzo.c   |   9 +++-
> >  drivers/block/zram/Kconfig |   8 ++--
> >  drivers/block/zram/Makefile|   4 +-
> >  drivers/block/zram/zcomp.c | 102 
> > +++--
> >  drivers/block/zram/zcomp.h |  42 +++--
> >  drivers/block/zram/zcomp_lz4.c |  47 ---
> >  drivers/block/zram/zcomp_lz4.h |  17 ---
> >  drivers/block/zram/zcomp_lzo.c |  47 ---
> >  drivers/block/zram/zcomp_lzo.h |  17 ---
> >  drivers/block/zram/zram_drv.c  |  32 +
> >  include/linux/crypto.h |  20 
> >  17 files changed, 211 insertions(+), 203 deletions(-)
> >  delete mode 100644 drivers/block/zram/zcomp_lz4.c
> >  delete mode 100644 drivers/block/zram/zcomp_lz4.h
> >  delete mode 100644 drivers/block/zram/zcomp_lzo.c
> >  delete mode 100644 drivers/block/zram/zcomp_lzo.h
> > 
> > -- 
> > 1.9.1
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line 

Re: [PATCH v3 1/9] crypto: introduce decompression API that can be called via sharable tfm object

[Joonsoo Kim]

On Tue, Sep 22, 2015 at 08:43:46PM +0800, Herbert Xu wrote:
> On Fri, Sep 18, 2015 at 02:19:16PM +0900, Joonsoo Kim wrote:
> > Until now, tfm object embeds (de)compression context in it and
> > (de)compression in crypto API requires tfm object to use
> > this context. But, there are some algorithms that doesn't need
> > such context to operate. Therefore, this patch introduce new crypto
> > decompression API that calls decompression function via sharable tfm
> > object. Concurrent calls to decompress_noctx function through sharable
> > tfm object will be okay because caller don't need any context in tfm and
> > tfm is only used for fetching function pointer to decompress_noctx
> > function. This can reduce overhead of maintaining multiple tfm
> > if decompression doesn't require any context to operate.
> > 
> > Signed-off-by: Joonsoo Kim 
> 
> Have you looked into include/crypto/compress.h?

Okay. Now, I have looked it.

> 
> The compress type itself is obsolete and should be replaced with
> the pcomp interface.
> 
> However, we made some mistakes with the pcomp interface.  First
> of all it doesn't have a non-partial interface like the digest
> function for crypto_shash.
> 
> But the biggest problem is that it should be completely stateless
> but is not.  IOW we should not store anything in the tfm that
> is per-request.  That should all go into the request structure.

I'm not a expert on this area but I have different opinion.
IIUC, what partial compression aims at is to support partial data
compression. It isn't for stateless compression and they are
different. Current implementation would work well for it's own
purpose.

And, making it stateless looks not good to me. If we make it
stateless, we should include algorithm's state in request object.
It enforces much memory to request object and, unlike crypto_shash
which can be allocated in stack, user needs to manage life time of
request objects and it requires additional complexity to
compression user.

Moreover, to use sharable tfm concurrently from my original intend,
many request objects would be needed and if it cannot be allocated
in the stack, it requires much memory. It's not suitable
for my purpose.

If compression is obsolete, what I think the best is leaving pcomp
as is and implementing sharable tfm in pcomp, And then, make lzo
and others support pcomp interface and sharable tfm.

Maybe, I'm wrong so please correct me.

Thanks.

> Fortunately it seems that pcomp doesn't actually have any users
> so we can just rip it out and start from scratch and redo it
> properly.
> 
> So to recap, please abandon any efforts on the crypto_compress
> interface as it is old and obsolete.  Instead reshape crypto_pcomp
> into a proper stateless interface which should then give you what
> you want.
> 
> When you do so, just keep in mind that we need to find a way to
> support IPComp.  That means the ability to allocate requests in
> thread context and then use them to compress/decompress in IRQ
> context.
> 
> Cheers,
> -- 
> Email: Herbert Xu 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
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 1/9] crypto: introduce decompression API that can be called via sharable tfm object

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 03:18:17PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> [..]
> >  static int __init lzo_mod_init(void)
> > diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> > index e71cb70..31152b1 100644
> > --- a/include/linux/crypto.h
> > +++ b/include/linux/crypto.h
> > @@ -355,6 +355,8 @@ struct compress_alg {
> > unsigned int slen, u8 *dst, unsigned int *dlen);
> > int (*coa_decompress)(struct crypto_tfm *tfm, const u8 *src,
> >   unsigned int slen, u8 *dst, unsigned int *dlen);
> > +   int (*coa_decompress_noctx)(const u8 *src, unsigned int slen,
> > +   u8 *dst, unsigned int *dlen);
> >  };
> >  
> >  
> > @@ -538,6 +540,9 @@ struct compress_tfm {
> > int (*cot_decompress)(struct crypto_tfm *tfm,
> >   const u8 *src, unsigned int slen,
> >   u8 *dst, unsigned int *dlen);
> > +   int (*cot_decompress_noctx)(struct crypto_tfm *tfm,
> > +   const u8 *src, unsigned int slen,
> > +   u8 *dst, unsigned int *dlen);
> >  };
> >  
> >  #define crt_ablkcipher crt_u.ablkcipher
> > @@ -1836,6 +1841,14 @@ static inline void crypto_free_comp(struct 
> > crypto_comp *tfm)
> > crypto_free_tfm(crypto_comp_tfm(tfm));
> >  }
> >  
> > +struct crypto_comp *crypto_alloc_comp_noctx(const char *alg_name,
> > +   u32 type, u32 mask);
> > +
> 
> this should be EXPORT_SYMBOL_GPL().
> 

Will do in next version.

Thanks.
--
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 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 12:51:28PM +0900, Minchan Kim wrote:
> On Fri, Sep 18, 2015 at 02:19:24PM +0900, Joonsoo Kim wrote:
> > Crypto subsystem now supports decompress_noctx API that requires
> > special tfm_noctx. This tfm can be shared by multiple concurrent
> > decompress user because this API doesn't rely on this tfm object
> > except to fetch decompress function pointer.
> > 
> > Until changing to use crypto API, zram doesn't require any zstrm
> > on decompress so decompress is parallelized unlimitedly. But, previous
> > patch make zram to use crypto API and this requires one zstrm on every
> > decompress users so, in some zstrm contended situations, zram's
> > performance would be degraded.
> > 
> > This patch makes zram use decompress_noctx API and restore zram's
> > performance as the time that zram doesn't use crypto API.
> > 
> > Following is zram's read performance number.
> > 
> > * iozone -t 4 -R -r 16K -s 60M -I +Z -i 0 -i 1
> > * max_stream is set to 1
> > * Output is in Kbytes/sec
> > 
> > zram-base vs zram-crypto vs zram-crypto-noctx
> > 
> > Read10411701.88 6426911.62  9423894.38
> > Re-read 10017386.62 6428218.88  1163.50
> > 
> > Signed-off-by: Joonsoo Kim 
> > ---
> >  drivers/block/zram/zcomp.c | 24 +++-
> >  drivers/block/zram/zcomp.h |  1 +
> >  2 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> > index c2ed7c8..a020200 100644
> > --- a/drivers/block/zram/zcomp.c
> > +++ b/drivers/block/zram/zcomp.c
> > @@ -319,9 +319,12 @@ void zcomp_compress_end(struct zcomp *comp, struct 
> > zcomp_strm *zstrm)
> > zcomp_strm_release(comp, zstrm);
> >  }
> >  
> > -/* Never return NULL, may sleep */
> > +/* May return NULL, may sleep */
> >  struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
> >  {
> > +   if (comp->tfm_noctx)
> > +   return NULL;
> 
> Hmm,, I understand why returns NULL but it seems to be awkward to use NULL
> to express meaningful semantic and following functions relies on.
> If I have a time, I will think over.

I also think that it's not good thing but I can't think any better
idea. Please let me know if you have better one.

> 
> > +
> > return zcomp_strm_find(comp);
> >  }
> >  
> > @@ -346,11 +349,18 @@ int zcomp_decompress(struct zcomp *comp, struct 
> > zcomp_strm *zstrm,
> >  {
> > unsigned int size = PAGE_SIZE;
> >  
> > +   if (!zstrm) {
> > +   return crypto_comp_decompress_noctx(comp->tfm_noctx,
> > +   src, src_len, dst, );
> > +   }
> > +
> > return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, );
> >  }
> >  
> >  void zcomp_destroy(struct zcomp *comp)
> >  {
> > +   if (comp->tfm_noctx)
> > +   crypto_free_comp_noctx(comp->tfm_noctx);
> > comp->destroy(comp);
> > kfree(comp);
> >  }
> > @@ -366,6 +376,7 @@ struct zcomp *zcomp_create(const char *compress, int 
> > max_strm)
> >  {
> > struct zcomp *comp;
> > const char *backend;
> > +   struct crypto_comp *tfm;
> >  
> > backend = find_backend(compress);
> > if (!backend)
> > @@ -384,5 +395,16 @@ struct zcomp *zcomp_create(const char *compress, int 
> > max_strm)
> > kfree(comp);
> > return ERR_PTR(-ENOMEM);
> > }
> > +
> > +   /*
> > +* Prepare to use crypto decompress_noctx API. One tfm is required
> > +* to initialize crypto algorithm properly and fetch corresponding
> > +* function pointer. But, it is sharable for multiple concurrent
> > +* decompress users.
> > +*/
> 
> Please comment out that this API will return NULL if the compressor doesn't
> support noctx mode.

Will do.

Thanks.
--
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 9/9] zram: use crypto decompress_noctx API

[Joonsoo Kim]

On Mon, Sep 21, 2015 at 02:29:18PM +0900, Sergey Senozhatsky wrote:
> On (09/18/15 14:19), Joonsoo Kim wrote:
> > -/* Never return NULL, may sleep */
> > +/* May return NULL, may sleep */
> >  struct zcomp_strm *zcomp_decompress_begin(struct zcomp *comp)
> >  {
> > +   if (comp->tfm_noctx)
> > +   return NULL;
> > +
> > return zcomp_strm_find(comp);
> >  }
> >  
> > @@ -346,11 +349,18 @@ int zcomp_decompress(struct zcomp *comp, struct 
> > zcomp_strm *zstrm,
> >  {
> > unsigned int size = PAGE_SIZE;
> >  
> > +   if (!zstrm) {
> > +   return crypto_comp_decompress_noctx(comp->tfm_noctx,
> > +   src, src_len, dst, );
> > +   }
> 
> unneeded braces.

It's more readable for me. But, if you don't like it, I will remove brace.

Thanks.
--
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