[PATCH 0/9] crypto: add zbufsize() interface

[PATCH 0/9] crypto: add zbufsize() interface

[Kees Cook]

When pstore was refactored to use the crypto compress API in:

  commit cb3bee0369bc ("pstore: Use crypto compress API")

nearly all the pstore-specific compression routines were replaced with
the existing crypto compression API. One case remained: calculating the
"worst case" compression sizes ahead of time so it could have a buffer
preallocated for doing compression (which was called "zbufsize").

To make pstore fully algorithm-agnostic, the compression API needs to
grow this functionality. This adds the interface to support querying the
"worst case" estimate, with a new "zbufsize" routine that each compressor
can implement.

This series adds the interface, updates each compressor, and refactors
pstore. (Though note, this is based on v4.18-rc2, and pstore will be
gaining zstd support during the v4.19 merge window, so the last patch
in this series will need to be coordinated, but I left it to show how
things would look right now.)

-Kees


Kees Cook (9):
  crypto: add zbufsize() interface
  crypto, 842: implement zbufsize()
  crypto, null: Implement zbufsize()
  crypto, lzo: Implement zbufsize()
  crypto, deflate: Implement zbufsize()
  crypto, zstd: Implement zbufsize()
  crypto, lz4: Implement zbufsize()
  crypto, lz4hc: Implement zbufsize()
  pstore: Use crypto_comp_zbufsize()

 crypto/842.c                        |  17 ++-
 crypto/compress.c                   |   9 ++
 crypto/crypto_null.c                |  10 +-
 crypto/deflate.c                    |  48 +++++++-
 crypto/lz4.c                        |  22 +++-
 crypto/lz4hc.c                      |  22 +++-
 crypto/lzo.c                        |  22 +++-
 crypto/zstd.c                       |  22 +++-
 fs/pstore/Kconfig                   |  91 +++++----------
 fs/pstore/inode.c                   |   2 -
 fs/pstore/internal.h                |   3 -
 fs/pstore/platform.c                | 171 ++++++----------------------
 include/crypto/internal/scompress.h |  11 ++
 include/linux/crypto.h              |  12 ++
 include/linux/sw842.h               |   9 ++
 15 files changed, 257 insertions(+), 214 deletions(-)

-- 
2.17.1

[PATCH 1/9] crypto: add zbufsize() interface

[Kees Cook]

When pstore was refactored to use the crypto compress API in:

  commit cb3bee0369bc ("pstore: Use crypto compress API")

nearly all the pstore-specific compression routines were replaced with
the existing crypto compression API. One case remained: calculating the
"worst case" compression sizes ahead of time so it could have a buffer
preallocated for doing compression (which was called "zbufsize").

To make pstore fully algorithm-agnostic, the compression API needs to
grow this functionality. This adds the interface to support querying the
"worst case" estimate, with a new "zbufsize" routine that each compressor
can implement. The per-compressor implementations come in later commits.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 crypto/compress.c                   |  9 +++++++++
 include/crypto/internal/scompress.h | 11 +++++++++++
 include/linux/crypto.h              | 12 ++++++++++++
 3 files changed, 32 insertions(+)

diff --git a/crypto/compress.c b/crypto/compress.c
index f2d522924a07..29a80bb3b9d3 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_zbufsize(struct crypto_tfm *tfm,
+			   unsigned int slen, unsigned int *dlen)
+{
+	if (!tfm->__crt_alg->cra_compress.coa_zbufsize)
+		return -ENOTSUPP;
+	return tfm->__crt_alg->cra_compress.coa_zbufsize(tfm, slen, dlen);
+}
+
 int crypto_init_compress_ops(struct crypto_tfm *tfm)
 {
 	struct compress_tfm *ops = &tfm->crt_compress;
 
 	ops->cot_compress = crypto_compress;
 	ops->cot_decompress = crypto_decompress;
+	ops->cot_zbufsize = crypto_zbufsize;
 
 	return 0;
 }
diff --git a/include/crypto/internal/scompress.h b/include/crypto/internal/scompress.h
index 0f6ddac1acfc..a4a2a55080ad 100644
--- a/include/crypto/internal/scompress.h
+++ b/include/crypto/internal/scompress.h
@@ -39,6 +39,8 @@ struct scomp_alg {
 	int (*decompress)(struct crypto_scomp *tfm, const u8 *src,
 			  unsigned int slen, u8 *dst, unsigned int *dlen,
 			  void *ctx);
+	int (*zbufsize)(struct crypto_scomp *tfm, unsigned int slen,
+			unsigned int *dlen, void *ctx);
 	struct crypto_alg base;
 };
 
@@ -94,6 +96,15 @@ static inline int crypto_scomp_decompress(struct crypto_scomp *tfm,
 						 ctx);
 }
 
+static inline int crypto_scomp_zbufsize(struct crypto_scomp *tfm,
+					unsigned int slen,
+					unsigned int *dlen, void *ctx)
+{
+	if (!crypto_scomp_alg(tfm)->zbufsize)
+		return -ENOTSUPP;
+	return crypto_scomp_alg(tfm)->zbufsize(tfm, slen, dlen, ctx);
+}
+
 int crypto_init_scomp_ops_async(struct crypto_tfm *tfm);
 struct acomp_req *crypto_acomp_scomp_alloc_ctx(struct acomp_req *req);
 void crypto_acomp_scomp_free_ctx(struct acomp_req *req);
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 6eb06101089f..376c056447e7 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -362,6 +362,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_zbufsize)(struct crypto_tfm *tfm, unsigned int slen,
+			    unsigned int *dlen);
 };
 
 
@@ -578,6 +580,8 @@ struct compress_tfm {
 	int (*cot_decompress)(struct crypto_tfm *tfm,
 	                      const u8 *src, unsigned int slen,
 	                      u8 *dst, unsigned int *dlen);
+	int (*cot_zbufsize)(struct crypto_tfm *tfm,
+			    unsigned int slen, unsigned int *dlen);
 };
 
 #define crt_ablkcipher	crt_u.ablkcipher
@@ -1660,5 +1664,13 @@ static inline int crypto_comp_decompress(struct crypto_comp *tfm,
 						    src, slen, dst, dlen);
 }
 
+static inline int crypto_comp_zbufsize(struct crypto_comp *tfm,
+				       unsigned int slen,
+				       unsigned int *dlen)
+{
+	return crypto_comp_crt(tfm)->cot_zbufsize(crypto_comp_tfm(tfm),
+						  slen, dlen);
+}
+
 #endif	/* _LINUX_CRYPTO_H */
 
-- 
2.17.1

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Herbert Xu]

On Thu, Aug 02, 2018 at 02:51:10PM -0700, Kees Cook wrote:
> When pstore was refactored to use the crypto compress API in:
> 
>   commit cb3bee0369bc ("pstore: Use crypto compress API")
> 
> nearly all the pstore-specific compression routines were replaced with
> the existing crypto compression API. One case remained: calculating the
> "worst case" compression sizes ahead of time so it could have a buffer
> preallocated for doing compression (which was called "zbufsize").
> 
> To make pstore fully algorithm-agnostic, the compression API needs to
> grow this functionality. This adds the interface to support querying the
> "worst case" estimate, with a new "zbufsize" routine that each compressor
> can implement. The per-compressor implementations come in later commits.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  crypto/compress.c                   |  9 +++++++++
>  include/crypto/internal/scompress.h | 11 +++++++++++
>  include/linux/crypto.h              | 12 ++++++++++++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/crypto/compress.c b/crypto/compress.c
> index f2d522924a07..29a80bb3b9d3 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_zbufsize(struct crypto_tfm *tfm,
> +			   unsigned int slen, unsigned int *dlen)
> +{
> +	if (!tfm->__crt_alg->cra_compress.coa_zbufsize)
> +		return -ENOTSUPP;
> +	return tfm->__crt_alg->cra_compress.coa_zbufsize(tfm, slen, dlen);
> +}

Please don't add new features to the old compress interface.  Any
new improvements should be added to scomp/acomp only.  Users who
need new features should be converted.

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

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Kees Cook]

On Tue, Aug 7, 2018 at 2:45 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Thu, Aug 02, 2018 at 02:51:10PM -0700, Kees Cook wrote:
>> When pstore was refactored to use the crypto compress API in:
>>
>>   commit cb3bee0369bc ("pstore: Use crypto compress API")
>>
>> nearly all the pstore-specific compression routines were replaced with
>> the existing crypto compression API. One case remained: calculating the
>> "worst case" compression sizes ahead of time so it could have a buffer
>> preallocated for doing compression (which was called "zbufsize").
>>
>> To make pstore fully algorithm-agnostic, the compression API needs to
>> grow this functionality. This adds the interface to support querying the
>> "worst case" estimate, with a new "zbufsize" routine that each compressor
>> can implement. The per-compressor implementations come in later commits.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  crypto/compress.c                   |  9 +++++++++
>>  include/crypto/internal/scompress.h | 11 +++++++++++
>>  include/linux/crypto.h              | 12 ++++++++++++
>>  3 files changed, 32 insertions(+)
>>
>> diff --git a/crypto/compress.c b/crypto/compress.c
>> index f2d522924a07..29a80bb3b9d3 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_zbufsize(struct crypto_tfm *tfm,
>> +                        unsigned int slen, unsigned int *dlen)
>> +{
>> +     if (!tfm->__crt_alg->cra_compress.coa_zbufsize)
>> +             return -ENOTSUPP;
>> +     return tfm->__crt_alg->cra_compress.coa_zbufsize(tfm, slen, dlen);
>> +}
>
> Please don't add new features to the old compress interface.  Any
> new improvements should be added to scomp/acomp only.  Users who
> need new features should be converted.

So, keep crypto_scomp_zbufsize() and drop crypto_comp_zbufsize() and
crypto_zbufsize()? Should I add crypto_acomp_zbufsize()?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Herbert Xu]

On Tue, Aug 07, 2018 at 11:10:10AM -0700, Kees Cook wrote:
>
> > Please don't add new features to the old compress interface.  Any
> > new improvements should be added to scomp/acomp only.  Users who
> > need new features should be converted.
> 
> So, keep crypto_scomp_zbufsize() and drop crypto_comp_zbufsize() and
> crypto_zbufsize()? Should I add crypto_acomp_zbufsize()?

Yes and yes.  acomp is the primary interface and should support
all the features in scomp.

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

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Kees Cook]

On Wed, Aug 08, 2018 at 10:53:19AM +0800, Herbert Xu wrote:
> On Tue, Aug 07, 2018 at 11:10:10AM -0700, Kees Cook wrote:
> >
> > > Please don't add new features to the old compress interface.  Any
> > > new improvements should be added to scomp/acomp only.  Users who
> > > need new features should be converted.
> > 
> > So, keep crypto_scomp_zbufsize() and drop crypto_comp_zbufsize() and
> > crypto_zbufsize()? Should I add crypto_acomp_zbufsize()?
> 
> Yes and yes.  acomp is the primary interface and should support
> all the features in scomp.

*thread necromancy*

Okay, I'm looking at this again because of the need in the module loader
to know "worst case decompression size"[1]. I am at a loss for how (or
why) the acomp interface is the "primary interface".

For modules, all that would be wanted is this, where the buffer size can
be allocated on demand:

u8 *decompressed = NULL;
size_t decompressed_size = 0;

decompressed = decompress(decompressed, compressed, compressed_size, &decompressed_size);

For pstore, the compressed_size is fixed and the decompression buffer
must be preallocated (for catching panic dumps), so the worst-case size
needs to be known in advance:

u8 *decompressed = NULL;
size_t decompressed_worst_size = 0;
size_t decompressed_size = 0;

worst_case(&decompressed_worst_size, compressed_size);

decompressed = kmalloc(decompressed_worst_size, GFP_KERNEL);
...
decompressed_size = decompressed_worst_size;
decompress(decompressed, compressed, compressed_size, &decompressed_size);


I don't see anything like this in the kernel for handling a simple
buffer-to-buffer decompression besides crypto_comp_decompress(). The
acomp interface is wildly over-complex for this. What the right
way to do this? (I can't find any documentation that discusses
compress/decompress[2].)

-Kees

[1] https://lore.kernel.org/linux-modules/YaMYJv539OEBz5B%2F@google.com/
[2] https://www.kernel.org/doc/html/latest/crypto/api-samples.html

-- 
Kees Cook

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Herbert Xu]

On Wed, Dec 01, 2021 at 03:39:06PM -0800, Kees Cook wrote:
>
> Okay, I'm looking at this again because of the need in the module loader
> to know "worst case decompression size"[1]. I am at a loss for how (or
> why) the acomp interface is the "primary interface".

This is similar to how we transitioned from the old hash interface
to shash/ahash.

Basically the legacy interface is synchronous only and cannot support
hardware devices without having the CPU spinning while waiting for the
result to come back.

If you only care about synchronous support and don't need to access
these hardware devices then you should use the new scomp interface
that's equivalent to the old compress interface but built in a way
to allow acomp users to easily access sync algorithms, but if you
are processing large amounts of data and wish to access offload devices
then you should consider using the async acomp interface.

> For modules, all that would be wanted is this, where the buffer size can
> be allocated on demand:
> 
> u8 *decompressed = NULL;
> size_t decompressed_size = 0;
> 
> decompressed = decompress(decompressed, compressed, compressed_size, &decompressed_size);
> 
> For pstore, the compressed_size is fixed and the decompression buffer
> must be preallocated (for catching panic dumps), so the worst-case size
> needs to be known in advance:
> 
> u8 *decompressed = NULL;
> size_t decompressed_worst_size = 0;
> size_t decompressed_size = 0;
> 
> worst_case(&decompressed_worst_size, compressed_size);
> 
> decompressed = kmalloc(decompressed_worst_size, GFP_KERNEL);
> ...
> decompressed_size = decompressed_worst_size;
> decompress(decompressed, compressed, compressed_size, &decompressed_size);
> 
> 
> I don't see anything like this in the kernel for handling a simple
> buffer-to-buffer decompression besides crypto_comp_decompress(). The
> acomp interface is wildly over-complex for this. What the right
> way to do this? (I can't find any documentation that discusses
> compress/decompress[2].)

I think you're asking about a different issue, which is that we
don't have an interface for on-the-go allocation of decompressed
results so every user has to allocate the maximum worst-case buffer.

This is definitely an area that should be addressed but a lot of work
needs to be done to get there.  Essentially we'd need to convert
the underlying algorithms to a model where they decompress into a
list of pages and then they could simply allocate a new page if they
need extra space.

The result can then be returned to the original caller as an SG list.

Would you be willing to work on something like this? This would benefit
all existing users too.  For example, IPsec would no longer need to
allocate those 64K buffers for IPcomp.

Unfortunately not many people care deeply about compression so
volunteers are hard to find.

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

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Kees Cook]

On Thu, Dec 02, 2021 at 12:58:20PM +1100, Herbert Xu wrote:
> On Wed, Dec 01, 2021 at 03:39:06PM -0800, Kees Cook wrote:
> >
> > Okay, I'm looking at this again because of the need in the module loader
> > to know "worst case decompression size"[1]. I am at a loss for how (or
> > why) the acomp interface is the "primary interface".
> 
> This is similar to how we transitioned from the old hash interface
> to shash/ahash.
> 
> Basically the legacy interface is synchronous only and cannot support
> hardware devices without having the CPU spinning while waiting for the
> result to come back.
> 
> If you only care about synchronous support and don't need to access
> these hardware devices then you should use the new scomp interface
> that's equivalent to the old compress interface but built in a way
> to allow acomp users to easily access sync algorithms, but if you
> are processing large amounts of data and wish to access offload devices
> then you should consider using the async acomp interface.

But the scomp API appears to be "internal only":

include/crypto/internal/scompress.h:static inline int crypto_scomp_decompress(struct crypto_scomp *tfm,

What's the correct API calling sequence to do a simple decompress?

> > For modules, all that would be wanted is this, where the buffer size can
> > be allocated on demand:
> > 
> > u8 *decompressed = NULL;
> > size_t decompressed_size = 0;
> > 
> > decompressed = decompress(decompressed, compressed, compressed_size, &decompressed_size);
> > 
> > For pstore, the compressed_size is fixed and the decompression buffer
> > must be preallocated (for catching panic dumps), so the worst-case size
> > needs to be known in advance:
> > 
> > u8 *decompressed = NULL;
> > size_t decompressed_worst_size = 0;
> > size_t decompressed_size = 0;
> > 
> > worst_case(&decompressed_worst_size, compressed_size);
> > 
> > decompressed = kmalloc(decompressed_worst_size, GFP_KERNEL);
> > ...
> > decompressed_size = decompressed_worst_size;
> > decompress(decompressed, compressed, compressed_size, &decompressed_size);
> > 
> > 
> > I don't see anything like this in the kernel for handling a simple
> > buffer-to-buffer decompression besides crypto_comp_decompress(). The
> > acomp interface is wildly over-complex for this. What the right
> > way to do this? (I can't find any documentation that discusses
> > compress/decompress[2].)
> 
> I think you're asking about a different issue, which is that we
> don't have an interface for on-the-go allocation of decompressed
> results so every user has to allocate the maximum worst-case buffer.
> 
> This is definitely an area that should be addressed but a lot of work
> needs to be done to get there.  Essentially we'd need to convert
> the underlying algorithms to a model where they decompress into a
> list of pages and then they could simply allocate a new page if they
> need extra space.
> 
> The result can then be returned to the original caller as an SG list.
> 
> Would you be willing to work on something like this? This would benefit
> all existing users too.  For example, IPsec would no longer need to
> allocate those 64K buffers for IPcomp.
> 
> Unfortunately not many people care deeply about compression so
> volunteers are hard to find.

Dmitry has, I think, a bit of this already in [1] that could maybe be
generalized if it'd needed?

pstore still needs the "worst case" API to do a preallocation, though.

Anyway, if I could have an example of how to use scomp in pstore, I
could better see where to wire up the proposed zbufsize API...

Thanks!

[1] https://lore.kernel.org/linux-modules/YaMYJv539OEBz5B%2F@google.com/#Z31kernel:module_decompress.c

-- 
Kees Cook

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Herbert Xu]

On Wed, Dec 01, 2021 at 07:51:25PM -0800, Kees Cook wrote:
>
> But the scomp API appears to be "internal only":
> 
> include/crypto/internal/scompress.h:static inline int crypto_scomp_decompress(struct crypto_scomp *tfm,
> 
> What's the correct API calling sequence to do a simple decompress?

OK we haven't wired up scomp to users because there was no user
to start with.  So if you like you can create it just as we did
for shash.

The question becomes do you want to restrict your use-case to
synchronous-only algorithms, i.e., you will never be able to access
offload devices that support compression?

Typically this would only make sense if you process a very small
amount of data, but this seems counter-intuitive with compression
(it does make sense with hashing where we often hash just 16 bytes).

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

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Kees Cook]

On Thu, Dec 02, 2021 at 02:57:27PM +1100, Herbert Xu wrote:
> On Wed, Dec 01, 2021 at 07:51:25PM -0800, Kees Cook wrote:
> >
> > But the scomp API appears to be "internal only":
> > 
> > include/crypto/internal/scompress.h:static inline int crypto_scomp_decompress(struct crypto_scomp *tfm,
> > 
> > What's the correct API calling sequence to do a simple decompress?
> 
> OK we haven't wired up scomp to users because there was no user
> to start with.  So if you like you can create it just as we did
> for shash.
> 
> The question becomes do you want to restrict your use-case to
> synchronous-only algorithms, i.e., you will never be able to access
> offload devices that support compression?

I'd rather just have a simple API that hid all the async (or sync) details
and would work with whatever was the "best" implementation. Neither pstore
nor the module loader has anything else to do while decompression happens.

> Typically this would only make sense if you process a very small
> amount of data, but this seems counter-intuitive with compression
> (it does make sense with hashing where we often hash just 16 bytes).

pstore works on usually a handful of small buffers. (One of the largest
I've seen is used by Chrome OS: 6 128K buffers.) Speed is not important
(done at most 6 times at boot, and 1 time on panic), and, in fact,
offload is probably a bad idea just to keep the machinery needed to
store a panic log as small as possible.

The module loader is also doing non-fast-path decompression of modules,
with each of those being maybe a couple megabytes. This isn't fast-path
either: if it's not the kernel, it'd be userspace doing the decompression,
and it only happens once per module, usually at boot.

Why can't crypto_comp_*() be refactored to wrap crypto_acomp_*() (and
crypto_scomp_*())? I can see so many other places that would benefit from
this. Here are just some of the places that appear to be hand-rolling
compression/decompression routines that might benefit from this kind of
code re-use and compression alg agnosticism:

fs/pstore/platform.c
drivers/gpu/drm/i915/i915_gpu_error.c
kernel/power/swap.c
arch/powerpc/kernel/nvram_64.c
security/apparmor/policy_unpack.c
drivers/base/regmap/regcache-lzo.c
fs/btrfs/lzo.c
fs/btrfs/zlib.c
fs/f2fs/compress.c
fs/jffs2/compr_lzo.c
drivers/net/ethernet/chelsio/cxgb4/cudbg_zlib.h
drivers/net/ppp/ppp_deflate.c
fs/jffs2/compr_lzo.c
fs/jffs2/compr_zlib.c

But right now there isn't a good way to just do a simple one-off:

	dst = decompress_named(alg_name, dst, dst_len, src, src_len);

or if it happens more than once:

	alg = compressor(alg_name);
	set_comp_alg_param(param, value);
        ...
        for (...) {
		...
		dst = compress(alg, dst, dst_len, src, src_len);
		...
	}
        ...
        free_compressor(alg);

-Kees

-- 
Kees Cook

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Herbert Xu]

On Thu, Dec 02, 2021 at 12:10:13AM -0800, Kees Cook wrote:
>
> I'd rather just have a simple API that hid all the async (or sync) details
> and would work with whatever was the "best" implementation. Neither pstore
> nor the module loader has anything else to do while decompression happens.

Well that's exactly what the acomp interface is supposed to be.
It supports any algorithm, whether sync or async.  However, for
obvious reasons this interface has to be async.

> > Typically this would only make sense if you process a very small
> > amount of data, but this seems counter-intuitive with compression
> > (it does make sense with hashing where we often hash just 16 bytes).
> 
> pstore works on usually a handful of small buffers. (One of the largest
> I've seen is used by Chrome OS: 6 128K buffers.) Speed is not important
> (done at most 6 times at boot, and 1 time on panic), and, in fact,
> offload is probably a bad idea just to keep the machinery needed to
> store a panic log as small as possible.

In that case creating an scomp user interface is probably the best
course of action.

> Why can't crypto_comp_*() be refactored to wrap crypto_acomp_*() (and
> crypto_scomp_*())? I can see so many other places that would benefit from
> this. Here are just some of the places that appear to be hand-rolling
> compression/decompression routines that might benefit from this kind of
> code re-use and compression alg agnosticism:

We cannot provide async hardware through a sync-only interface
because that may lead to dead-lock.  For your use-cases you should
avoid using any async implementations.

The scomp interface is meant to be pretty much identical to the
legacy comp interface except that it supports integration with
acomp.

Because nobody has had a need for scomp we have not added an
interface for it so it only exists as part of the low-level API.
You're most welcome to expose it if you don't need the async
support part of acomp.

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

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Dmitry Torokhov]

On Fri, Dec 03, 2021 at 01:28:21PM +1100, Herbert Xu wrote:
> On Thu, Dec 02, 2021 at 12:10:13AM -0800, Kees Cook wrote:
> >
> > I'd rather just have a simple API that hid all the async (or sync) details
> > and would work with whatever was the "best" implementation. Neither pstore
> > nor the module loader has anything else to do while decompression happens.
> 
> Well that's exactly what the acomp interface is supposed to be.
> It supports any algorithm, whether sync or async.  However, for
> obvious reasons this interface has to be async.
> 
> > > Typically this would only make sense if you process a very small
> > > amount of data, but this seems counter-intuitive with compression
> > > (it does make sense with hashing where we often hash just 16 bytes).
> > 
> > pstore works on usually a handful of small buffers. (One of the largest
> > I've seen is used by Chrome OS: 6 128K buffers.) Speed is not important
> > (done at most 6 times at boot, and 1 time on panic), and, in fact,
> > offload is probably a bad idea just to keep the machinery needed to
> > store a panic log as small as possible.
> 
> In that case creating an scomp user interface is probably the best
> course of action.
> 
> > Why can't crypto_comp_*() be refactored to wrap crypto_acomp_*() (and
> > crypto_scomp_*())? I can see so many other places that would benefit from
> > this. Here are just some of the places that appear to be hand-rolling
> > compression/decompression routines that might benefit from this kind of
> > code re-use and compression alg agnosticism:
> 
> We cannot provide async hardware through a sync-only interface
> because that may lead to dead-lock.  For your use-cases you should
> avoid using any async implementations.
> 
> The scomp interface is meant to be pretty much identical to the
> legacy comp interface except that it supports integration with
> acomp.
> 
> Because nobody has had a need for scomp we have not added an
> interface for it so it only exists as part of the low-level API.
> You're most welcome to expose it if you don't need the async
> support part of acomp.

I must be getting lost in terminology, and it feels to me that what is
discussed here is most likely of no interest to a lot of potential
users, especially ones that do compression/decompression. In majority of
cases they want to simply compress or decompress data, and they just
want to do it quickly and with minimal amount of memory consumed. They
do not particularly care if the task is being offloaded or executed on
the main CPU, either on separate thread or on the same thread, so the
discussion about scomp/acomp/etc is of no interest to them. From their
perspective they'd be totally fine with a wrapper that would do:

int decompress(...) {
	prepare_request()
	send_request()
	wait_for_request()
}

and from their perspective this would be a synchronous API they are
happy with.

So from POV of such users what is actually missing is streaming mode of
compressing/decompressing where core would allow supplying additonal
data on demand and allow consuming output as it is being produced, and I
do not see anything like that in either scomp or acomp.

Thanks.

-- 
Dmitry

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Herbert Xu]

On Fri, Dec 03, 2021 at 12:49:26PM -0800, Dmitry Torokhov wrote:
>
> I must be getting lost in terminology, and it feels to me that what is
> discussed here is most likely of no interest to a lot of potential
> users, especially ones that do compression/decompression. In majority of
> cases they want to simply compress or decompress data, and they just
> want to do it quickly and with minimal amount of memory consumed. They
> do not particularly care if the task is being offloaded or executed on
> the main CPU, either on separate thread or on the same thread, so the
> discussion about scomp/acomp/etc is of no interest to them. From their
> perspective they'd be totally fine with a wrapper that would do:
> 
> int decompress(...) {
> 	prepare_request()
> 	send_request()
> 	wait_for_request()
> }
> 
> and from their perspective this would be a synchronous API they are
> happy with.

You can certainly do that as a Crypto API user.  And we do have
some users who do exactly this (for example, testmgr does that
when testing async algorithms).  However, this can't be a part of
the API itself since many of our users execute in atomic contexts.

> So from POV of such users what is actually missing is streaming mode of
> compressing/decompressing where core would allow supplying additonal
> data on demand and allow consuming output as it is being produced, and I
> do not see anything like that in either scomp or acomp.

That is indeed a very crucial part of the compression API that is
missing.  And I would love someone to donate some time to addressing
this.

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

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Dmitry Torokhov]

On Tue, Dec 07, 2021 at 04:20:29PM +1100, Herbert Xu wrote:
> On Fri, Dec 03, 2021 at 12:49:26PM -0800, Dmitry Torokhov wrote:
> >
> > I must be getting lost in terminology, and it feels to me that what is
> > discussed here is most likely of no interest to a lot of potential
> > users, especially ones that do compression/decompression. In majority of
> > cases they want to simply compress or decompress data, and they just
> > want to do it quickly and with minimal amount of memory consumed. They
> > do not particularly care if the task is being offloaded or executed on
> > the main CPU, either on separate thread or on the same thread, so the
> > discussion about scomp/acomp/etc is of no interest to them. From their
> > perspective they'd be totally fine with a wrapper that would do:
> > 
> > int decompress(...) {
> > 	prepare_request()
> > 	send_request()
> > 	wait_for_request()
> > }
> > 
> > and from their perspective this would be a synchronous API they are
> > happy with.
> 
> You can certainly do that as a Crypto API user.  And we do have
> some users who do exactly this (for example, testmgr does that
> when testing async algorithms).  However, this can't be a part of
> the API itself since many of our users execute in atomic contexts.

That is what I am confused about: why can't it be a part of API? Users
that are running in atomic contexts would not be able to use it, but we
have a lot of precedents for it. See for example spi_sync() vs
spi_async(). Callers have a choice as to which one to use, based on
their needs.

Thanks.

-- 
Dmitry

Re: [PATCH 1/9] crypto: add zbufsize() interface

[Herbert Xu]

On Mon, Dec 06, 2021 at 10:24:47PM -0800, Dmitry Torokhov wrote:
>
> That is what I am confused about: why can't it be a part of API? Users
> that are running in atomic contexts would not be able to use it, but we
> have a lot of precedents for it. See for example spi_sync() vs
> spi_async(). Callers have a choice as to which one to use, based on
> their needs.

We already have a helper in the form of crypto_wait_req.  If you
have any suggestions of making this easier to use then I'm more than
happy to consider them.

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

[PATCH 2/9] crypto, 842: implement zbufsize()

[Kees Cook]

This implements the worst-case compression size querying interface for
842, based on the logic originally used by pstore.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 crypto/842.c          | 17 ++++++++++++++++-
 include/linux/sw842.h |  9 +++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/crypto/842.c b/crypto/842.c
index bc26dc942821..c6d9ad6512f8 100644
--- a/crypto/842.c
+++ b/crypto/842.c
@@ -101,6 +101,19 @@ static int crypto842_sdecompress(struct crypto_scomp *tfm,
 	return sw842_decompress(src, slen, dst, dlen);
 }
 
+static int crypto842_zbufsize(struct crypto_tfm *tfm,
+			      unsigned int slen, unsigned int *dlen)
+{
+	return sw842_zbufsize(slen, dlen);
+}
+
+static int crypto842_szbufsize(struct crypto_scomp *tfm,
+			       unsigned int slen, unsigned int *dlen,
+			       void *ctx)
+{
+	return sw842_zbufsize(slen, dlen);
+}
+
 static struct crypto_alg alg = {
 	.cra_name		= "842",
 	.cra_driver_name	= "842-generic",
@@ -112,7 +125,8 @@ static struct crypto_alg alg = {
 	.cra_exit		= crypto842_exit,
 	.cra_u			= { .compress = {
 	.coa_compress		= crypto842_compress,
-	.coa_decompress		= crypto842_decompress } }
+	.coa_decompress		= crypto842_decompress,
+	.coa_zbufsize		= crypto842_zbufsize } }
 };
 
 static struct scomp_alg scomp = {
@@ -120,6 +134,7 @@ static struct scomp_alg scomp = {
 	.free_ctx		= crypto842_free_ctx,
 	.compress		= crypto842_scompress,
 	.decompress		= crypto842_sdecompress,
+	.zbufsize		= crypto842_szbufsize,
 	.base			= {
 		.cra_name	= "842",
 		.cra_driver_name = "842-scomp",
diff --git a/include/linux/sw842.h b/include/linux/sw842.h
index 3e29f5dcc62b..9760554beb26 100644
--- a/include/linux/sw842.h
+++ b/include/linux/sw842.h
@@ -10,4 +10,13 @@ int sw842_compress(const u8 *src, unsigned int srclen,
 int sw842_decompress(const u8 *src, unsigned int srclen,
 		     u8 *dst, unsigned int *destlen);
 
+static inline int sw842_zbufsize(unsigned int srclen,
+				 unsigned int *destlen)
+{
+	/* Worst case is uncompressed sized. */
+	*destlen = srclen;
+
+	return 0;
+}
+
 #endif
-- 
2.17.1

[PATCH 3/9] crypto, null: Implement zbufsize()

[Kees Cook]

This implements the worst-case compression size querying interface for
the "null" compression implementation (i.e. no change in size).

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 crypto/crypto_null.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/crypto/crypto_null.c b/crypto/crypto_null.c
index 20ff2c746e0b..d1b6107f8443 100644
--- a/crypto/crypto_null.c
+++ b/crypto/crypto_null.c
@@ -39,6 +39,13 @@ static int null_compress(struct crypto_tfm *tfm, const u8 *src,
 	return 0;
 }
 
+static int null_zbufsize(struct crypto_tfm *tfm, unsigned int slen,
+			 unsigned int *dlen)
+{
+	*dlen = slen;
+	return 0;
+}
+
 static int null_init(struct shash_desc *desc)
 {
 	return 0;
@@ -146,7 +153,8 @@ static struct crypto_alg null_algs[3] = { {
 	.cra_module		=	THIS_MODULE,
 	.cra_u			=	{ .compress = {
 	.coa_compress		=	null_compress,
-	.coa_decompress		=	null_compress } }
+	.coa_decompress		=	null_compress,
+	.coa_zbufsize		=	null_zbufsize } }
 } };
 
 MODULE_ALIAS_CRYPTO("compress_null");
-- 
2.17.1

[PATCH 4/9] crypto, lzo: Implement zbufsize()

[Kees Cook]

This implements the worst-case compression size querying interface for
lzo, based on the logic originally used by pstore.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 crypto/lzo.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/crypto/lzo.c b/crypto/lzo.c
index 218567d717d6..ea18f62c5247 100644
--- a/crypto/lzo.c
+++ b/crypto/lzo.c
@@ -120,6 +120,24 @@ static int lzo_sdecompress(struct crypto_scomp *tfm, const u8 *src,
 	return __lzo_decompress(src, slen, dst, dlen);
 }
 
+static int __lzo_zbufsize(unsigned int slen, unsigned int *dlen)
+{
+	*dlen = lzo1x_worst_compress(slen);
+	return 0;
+}
+
+static int lzo_zbufsize(struct crypto_tfm *tfm, unsigned int slen,
+			unsigned int *dlen)
+{
+	return __lzo_zbufsize(slen, dlen);
+}
+
+static int lzo_szbufsize(struct crypto_scomp *tfm, unsigned int slen,
+			 unsigned int *dlen, void *ctx)
+{
+	return __lzo_zbufsize(slen, dlen);
+}
+
 static struct crypto_alg alg = {
 	.cra_name		= "lzo",
 	.cra_flags		= CRYPTO_ALG_TYPE_COMPRESS,
@@ -129,7 +147,8 @@ static struct crypto_alg alg = {
 	.cra_exit		= lzo_exit,
 	.cra_u			= { .compress = {
 	.coa_compress		= lzo_compress,
-	.coa_decompress		= lzo_decompress } }
+	.coa_decompress		= lzo_decompress,
+	.coa_zbufsize		= lzo_zbufsize } }
 };
 
 static struct scomp_alg scomp = {
@@ -137,6 +156,7 @@ static struct scomp_alg scomp = {
 	.free_ctx		= lzo_free_ctx,
 	.compress		= lzo_scompress,
 	.decompress		= lzo_sdecompress,
+	.zbufsize		= lzo_szbufsize,
 	.base			= {
 		.cra_name	= "lzo",
 		.cra_driver_name = "lzo-scomp",
-- 
2.17.1

[PATCH 5/9] crypto, deflate: Implement zbufsize()

[Kees Cook]

This implements the worst-case compression size querying interface for
deflate, based on the logic originally used by pstore.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 crypto/deflate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/crypto/deflate.c b/crypto/deflate.c
index 94ec3b36a8e8..2665af8d49b4 100644
--- a/crypto/deflate.c
+++ b/crypto/deflate.c
@@ -277,6 +277,47 @@ static int deflate_sdecompress(struct crypto_scomp *tfm, const u8 *src,
 	return __deflate_decompress(src, slen, dst, dlen, ctx);
 }
 
+static int __deflate_zbufsize(unsigned int slen, unsigned int *dlen)
+{
+	size_t size = slen;
+	size_t cmpr;
+
+	/* Estimated ranges */
+	switch (size) {
+	case 1000 ... 2000:
+		cmpr = 56;
+		break;
+	case 2001 ... 3000:
+		cmpr = 54;
+		break;
+	case 3001 ... 3999:
+		cmpr = 52;
+		break;
+	case 4000 ... 10000:
+		cmpr = 45;
+		break;
+	default:
+		cmpr = 60;
+		break;
+	}
+
+	*dlen = (size * 100) / cmpr;
+
+	return 0;
+}
+
+static int deflate_zbufsize(struct crypto_tfm *tfm, unsigned int slen,
+			    unsigned int *dlen)
+{
+	return __deflate_zbufsize(slen, dlen);
+}
+
+static int deflate_szbufsize(struct crypto_scomp *tfm, unsigned int slen,
+			     unsigned int *dlen, void *ctx)
+{
+	return __deflate_zbufsize(slen, dlen);
+}
+
 static struct crypto_alg alg = {
 	.cra_name		= "deflate",
 	.cra_flags		= CRYPTO_ALG_TYPE_COMPRESS,
@@ -285,8 +326,9 @@ static struct crypto_alg alg = {
 	.cra_init		= deflate_init,
 	.cra_exit		= deflate_exit,
 	.cra_u			= { .compress = {
-	.coa_compress 		= deflate_compress,
-	.coa_decompress  	= deflate_decompress } }
+	.coa_compress		= deflate_compress,
+	.coa_decompress		= deflate_decompress,
+	.coa_zbufsize		= deflate_zbufsize } }
 };
 
 static struct scomp_alg scomp[] = { {
@@ -294,6 +336,7 @@ static struct scomp_alg scomp[] = { {
 	.free_ctx		= deflate_free_ctx,
 	.compress		= deflate_scompress,
 	.decompress		= deflate_sdecompress,
+	.zbufsize		= deflate_szbufsize,
 	.base			= {
 		.cra_name	= "deflate",
 		.cra_driver_name = "deflate-scomp",
@@ -304,6 +347,7 @@ static struct scomp_alg scomp[] = { {
 	.free_ctx		= deflate_free_ctx,
 	.compress		= deflate_scompress,
 	.decompress		= deflate_sdecompress,
+	.zbufsize		= deflate_szbufsize,
 	.base			= {
 		.cra_name	= "zlib-deflate",
 		.cra_driver_name = "zlib-deflate-scomp",
-- 
2.17.1

[PATCH 6/9] crypto, zstd: Implement zbufsize()

[Kees Cook]

This implements the worst-case compression size querying interface for
zstd, based on the logic originally used by pstore.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 crypto/zstd.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/crypto/zstd.c b/crypto/zstd.c
index 9a76b3ed8b8b..ce8efd40f27f 100644
--- a/crypto/zstd.c
+++ b/crypto/zstd.c
@@ -212,6 +212,24 @@ static int zstd_sdecompress(struct crypto_scomp *tfm, const u8 *src,
 	return __zstd_decompress(src, slen, dst, dlen, ctx);
 }
 
+static int __zstd_zbufsize(unsigned int slen, unsigned int *dlen)
+{
+	*dlen = ZSTD_compressBound(slen);
+	return 0;
+}
+
+static int zstd_zbufsize(struct crypto_tfm *tfm, unsigned int slen,
+			 unsigned int *dlen)
+{
+	return __zstd_zbufsize(slen, dlen);
+}
+
+static int zstd_szbufsize(struct crypto_scomp *tfm, unsigned int slen,
+			  unsigned int *dlen, void *ctx)
+{
+	return __zstd_zbufsize(slen, dlen);
+}
+
 static struct crypto_alg alg = {
 	.cra_name		= "zstd",
 	.cra_flags		= CRYPTO_ALG_TYPE_COMPRESS,
@@ -221,7 +239,8 @@ static struct crypto_alg alg = {
 	.cra_exit		= zstd_exit,
 	.cra_u			= { .compress = {
 	.coa_compress		= zstd_compress,
-	.coa_decompress		= zstd_decompress } }
+	.coa_decompress		= zstd_decompress,
+	.coa_zbufsize		= zstd_zbufsize } }
 };
 
 static struct scomp_alg scomp = {
@@ -229,6 +248,7 @@ static struct scomp_alg scomp = {
 	.free_ctx		= zstd_free_ctx,
 	.compress		= zstd_scompress,
 	.decompress		= zstd_sdecompress,
+	.zbufsize		= zstd_szbufsize,
 	.base			= {
 		.cra_name	= "zstd",
 		.cra_driver_name = "zstd-scomp",
-- 
2.17.1

[PATCH 7/9] crypto, lz4: Implement zbufsize()

[Kees Cook]

This implements the worst-case compression size querying interface for
lz4, based on the logic originally used by pstore.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 crypto/lz4.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/crypto/lz4.c b/crypto/lz4.c
index 2ce2660d3519..42003d080967 100644
--- a/crypto/lz4.c
+++ b/crypto/lz4.c
@@ -117,6 +117,24 @@ static int lz4_decompress_crypto(struct crypto_tfm *tfm, const u8 *src,
 	return __lz4_decompress_crypto(src, slen, dst, dlen, NULL);
 }
 
+static int __lz4_zbufsize_crypto(unsigned int slen, unsigned int *dlen)
+{
+	*dlen = LZ4_compressBound(slen);
+	return 0;
+}
+
+static int lz4_szbufsize(struct crypto_scomp *tfm, unsigned int slen,
+			 unsigned int *dlen, void *ctx)
+{
+	return __lz4_zbufsize_crypto(slen, dlen);
+}
+
+static int lz4_zbufsize_crypto(struct crypto_tfm *tfm, unsigned int slen,
+			       unsigned int *dlen)
+{
+	return __lz4_zbufsize_crypto(slen, dlen);
+}
+
 static struct crypto_alg alg_lz4 = {
 	.cra_name		= "lz4",
 	.cra_flags		= CRYPTO_ALG_TYPE_COMPRESS,
@@ -127,7 +145,8 @@ static struct crypto_alg alg_lz4 = {
 	.cra_exit		= lz4_exit,
 	.cra_u			= { .compress = {
 	.coa_compress		= lz4_compress_crypto,
-	.coa_decompress		= lz4_decompress_crypto } }
+	.coa_decompress		= lz4_decompress_crypto,
+	.coa_zbufsize		= lz4_zbufsize_crypto } }
 };
 
 static struct scomp_alg scomp = {
@@ -135,6 +154,7 @@ static struct scomp_alg scomp = {
 	.free_ctx		= lz4_free_ctx,
 	.compress		= lz4_scompress,
 	.decompress		= lz4_sdecompress,
+	.zbufsize		= lz4_szbufsize,
 	.base			= {
 		.cra_name	= "lz4",
 		.cra_driver_name = "lz4-scomp",
-- 
2.17.1

[PATCH 8/9] crypto, lz4hc: Implement zbufsize()

[Kees Cook]

This implements the worst-case compression size querying interface for
lz4hc, based on the logic originally used by pstore.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 crypto/lz4hc.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c
index 2be14f054daf..cf6f38a1c35b 100644
--- a/crypto/lz4hc.c
+++ b/crypto/lz4hc.c
@@ -118,6 +118,24 @@ static int lz4hc_decompress_crypto(struct crypto_tfm *tfm, const u8 *src,
 	return __lz4hc_decompress_crypto(src, slen, dst, dlen, NULL);
 }
 
+static int __lz4hc_zbufsize_crypto(unsigned int slen, unsigned int *dlen)
+{
+	*dlen = LZ4_compressBound(slen);
+	return 0;
+}
+
+static int lz4hc_szbufsize(struct crypto_scomp *tfm, unsigned int slen,
+			   unsigned int *dlen, void *ctx)
+{
+	return __lz4hc_zbufsize_crypto(slen, dlen);
+}
+
+static int lz4hc_zbufsize_crypto(struct crypto_tfm *tfm, unsigned int slen,
+				 unsigned int *dlen)
+{
+	return __lz4hc_zbufsize_crypto(slen, dlen);
+}
+
 static struct crypto_alg alg_lz4hc = {
 	.cra_name		= "lz4hc",
 	.cra_flags		= CRYPTO_ALG_TYPE_COMPRESS,
@@ -128,7 +146,8 @@ static struct crypto_alg alg_lz4hc = {
 	.cra_exit		= lz4hc_exit,
 	.cra_u			= { .compress = {
 	.coa_compress		= lz4hc_compress_crypto,
-	.coa_decompress		= lz4hc_decompress_crypto } }
+	.coa_decompress		= lz4hc_decompress_crypto,
+	.coa_zbufsize		= lz4hc_zbufsize_crypto } }
 };
 
 static struct scomp_alg scomp = {
@@ -136,6 +155,7 @@ static struct scomp_alg scomp = {
 	.free_ctx		= lz4hc_free_ctx,
 	.compress		= lz4hc_scompress,
 	.decompress		= lz4hc_sdecompress,
+	.zbufsize		= lz4hc_szbufsize,
 	.base			= {
 		.cra_name	= "lz4hc",
 		.cra_driver_name = "lz4hc-scomp",
-- 
2.17.1

[PATCH 9/9] pstore: Use crypto_comp_zbufsize()

[Kees Cook]

Now that the crypto compression API has a zbufsize interface, use that
instead, so pstore doesn't need to keep getting updated for each new
compression algo that gets added to the kernel. This reorganizes the
code slightly at initialization time (since we must allocate the algo
first now), but otherwise is a massive cleanup of the code, making
pstore kernel code algorithm-agnostic now.

The Kconfig is also adjusted to continue to provide build-time "default
algorithm" selection while minimizing what is needed when new algorithms
are added.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/pstore/Kconfig    |  91 +++++++----------------
 fs/pstore/inode.c    |   2 -
 fs/pstore/internal.h |   3 -
 fs/pstore/platform.c | 171 +++++++++----------------------------------
 4 files changed, 61 insertions(+), 206 deletions(-)

diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
index 09c19ef91526..1010304e0996 100644
--- a/fs/pstore/Kconfig
+++ b/fs/pstore/Kconfig
@@ -1,6 +1,5 @@
 config PSTORE
 	tristate "Persistent store support"
-	select CRYPTO if PSTORE_COMPRESS
 	default n
 	help
 	   This option enables generic access to platform level
@@ -13,88 +12,54 @@ config PSTORE
 	   If you don't have a platform persistent store driver,
 	   say N.
 
-config PSTORE_DEFLATE_COMPRESS
-	tristate "DEFLATE (ZLIB) compression"
-	default y
-	depends on PSTORE
-	select CRYPTO_DEFLATE
-	help
-	  This option enables DEFLATE (also known as ZLIB) compression
-	  algorithm support.
-
-config PSTORE_LZO_COMPRESS
-	tristate "LZO compression"
-	depends on PSTORE
-	select CRYPTO_LZO
-	help
-	  This option enables LZO compression algorithm support.
-
-config PSTORE_LZ4_COMPRESS
-	tristate "LZ4 compression"
-	depends on PSTORE
-	select CRYPTO_LZ4
-	help
-	  This option enables LZ4 compression algorithm support.
-
-config PSTORE_LZ4HC_COMPRESS
-	tristate "LZ4HC compression"
-	depends on PSTORE
-	select CRYPTO_LZ4HC
-	help
-	  This option enables LZ4HC (high compression) mode algorithm.
-
-config PSTORE_842_COMPRESS
-	bool "842 compression"
-	depends on PSTORE
-	select CRYPTO_842
-	help
-	  This option enables 842 compression algorithm support.
-
 config PSTORE_COMPRESS
-	def_bool y
+	bool "Perform compression on pstore records"
 	depends on PSTORE
-	depends on PSTORE_DEFLATE_COMPRESS || PSTORE_LZO_COMPRESS ||	\
-		   PSTORE_LZ4_COMPRESS || PSTORE_LZ4HC_COMPRESS ||	\
-		   PSTORE_842_COMPRESS
+	default y
+	select CRYPTO
+	help
+	  Normally, pstore will store records uncompressed. However,
+	  since some backends have limited storage and records can get
+	  long, it may be more efficient to have pstore compress the
+	  records. Enabling this feature will enable compression
+	  support.
 
 choice
 	prompt "Default pstore compression algorithm"
 	depends on PSTORE_COMPRESS
 	help
 	  This option chooses the default active compression algorithm.
-	  This change be changed at boot with "pstore.compress=..." on
-	  the kernel command line.
-
-	  Currently, pstore has support for 5 compression algorithms:
-	  deflate, lzo, lz4, lz4hc and 842.
+	  Selecting "None" means an algorithm must be chosen on the
+	  kernel command line via "pstore.compress=..."
 
-	  The default compression algorithm is deflate.
+	config PSTORE_COMPRESS_DEFAULT_DEFLATE
+		bool "deflate" if CRYPTO_DEFLATE
 
-	config PSTORE_DEFLATE_COMPRESS_DEFAULT
-		bool "deflate" if PSTORE_DEFLATE_COMPRESS
+	config PSTORE_COMPRESS_DEFAULT_LZO
+		bool "lzo" if CRYPTO_LZO
 
-	config PSTORE_LZO_COMPRESS_DEFAULT
-		bool "lzo" if PSTORE_LZO_COMPRESS
+	config PSTORE_COMPRESS_DEFAULT_LZ4
+		bool "lz4" if CRYPTO_LZ4
 
-	config PSTORE_LZ4_COMPRESS_DEFAULT
-		bool "lz4" if PSTORE_LZ4_COMPRESS
+	config PSTORE_COMPRESS_DEFAULT_LZ4HC
+		bool "lz4hc" if CRYPTO_LZ4HC
 
-	config PSTORE_LZ4HC_COMPRESS_DEFAULT
-		bool "lz4hc" if PSTORE_LZ4HC_COMPRESS
+	config PSTORE_COMPRESS_DEFAULT_842
+		bool "842" if CRYPTO_842
 
-	config PSTORE_842_COMPRESS_DEFAULT
-		bool "842" if PSTORE_842_COMPRESS
+	config PSTORE_COMPRESS_DEFAULT_NONE
+		bool "None"
 
 endchoice
 
 config PSTORE_COMPRESS_DEFAULT
 	string
 	depends on PSTORE_COMPRESS
-	default "deflate" if PSTORE_DEFLATE_COMPRESS_DEFAULT
-	default "lzo" if PSTORE_LZO_COMPRESS_DEFAULT
-	default "lz4" if PSTORE_LZ4_COMPRESS_DEFAULT
-	default "lz4hc" if PSTORE_LZ4HC_COMPRESS_DEFAULT
-	default "842" if PSTORE_842_COMPRESS_DEFAULT
+	default "deflate" if PSTORE_COMPRESS_DEFAULT_DEFLATE
+	default "lzo" if PSTORE_COMPRESS_DEFAULT_LZO
+	default "lz4" if PSTORE_COMPRESS_DEFAULT_LZ4
+	default "lz4hc" if PSTORE_COMPRESS_DEFAULT_LZ4HC
+	default "842" if PSTORE_COMPRESS_DEFAULT_842
 
 config PSTORE_CONSOLE
 	bool "Log kernel console messages"
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 5fcb845b9fec..d814723fb27d 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -486,8 +486,6 @@ static int __init init_pstore_fs(void)
 {
 	int err;
 
-	pstore_choose_compression();
-
 	/* Create a convenient mount point for people to access pstore */
 	err = sysfs_create_mount_point(fs_kobj, "pstore");
 	if (err)
diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h
index fb767e28aeb2..c029314478fa 100644
--- a/fs/pstore/internal.h
+++ b/fs/pstore/internal.h
@@ -37,7 +37,4 @@ extern bool	pstore_is_mounted(void);
 extern void	pstore_record_init(struct pstore_record *record,
 				   struct pstore_info *psi);
 
-/* Called during module_init() */
-extern void __init pstore_choose_compression(void);
-
 #endif
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index c238ab8ba31d..eca7588afa92 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -28,12 +28,6 @@
 #include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pstore.h>
-#if IS_ENABLED(CONFIG_PSTORE_LZO_COMPRESS)
-#include <linux/lzo.h>
-#endif
-#if IS_ENABLED(CONFIG_PSTORE_LZ4_COMPRESS) || IS_ENABLED(CONFIG_PSTORE_LZ4HC_COMPRESS)
-#include <linux/lz4.h>
-#endif
 #include <linux/crypto.h>
 #include <linux/string.h>
 #include <linux/timer.h>
@@ -82,13 +76,8 @@ static char *compress =
 /* Compression parameters */
 static struct crypto_comp *tfm;
 
-struct pstore_zbackend {
-	int (*zbufsize)(size_t size);
-	const char *name;
-};
-
 static char *big_oops_buf;
-static size_t big_oops_buf_sz;
+static unsigned int big_oops_buf_sz;
 
 /* How much of the console log to snapshot */
 unsigned long kmsg_bytes = PSTORE_DEFAULT_KMSG_BYTES;
@@ -142,92 +131,6 @@ bool pstore_cannot_block_path(enum kmsg_dump_reason reason)
 }
 EXPORT_SYMBOL_GPL(pstore_cannot_block_path);
 
-#if IS_ENABLED(CONFIG_PSTORE_DEFLATE_COMPRESS)
-static int zbufsize_deflate(size_t size)
-{
-	size_t cmpr;
-
-	switch (size) {
-	/* buffer range for efivars */
-	case 1000 ... 2000:
-		cmpr = 56;
-		break;
-	case 2001 ... 3000:
-		cmpr = 54;
-		break;
-	case 3001 ... 3999:
-		cmpr = 52;
-		break;
-	/* buffer range for nvram, erst */
-	case 4000 ... 10000:
-		cmpr = 45;
-		break;
-	default:
-		cmpr = 60;
-		break;
-	}
-
-	return (size * 100) / cmpr;
-}
-#endif
-
-#if IS_ENABLED(CONFIG_PSTORE_LZO_COMPRESS)
-static int zbufsize_lzo(size_t size)
-{
-	return lzo1x_worst_compress(size);
-}
-#endif
-
-#if IS_ENABLED(CONFIG_PSTORE_LZ4_COMPRESS) || IS_ENABLED(CONFIG_PSTORE_LZ4HC_COMPRESS)
-static int zbufsize_lz4(size_t size)
-{
-	return LZ4_compressBound(size);
-}
-#endif
-
-#if IS_ENABLED(CONFIG_PSTORE_842_COMPRESS)
-static int zbufsize_842(size_t size)
-{
-	return size;
-}
-#endif
-
-static const struct pstore_zbackend *zbackend __ro_after_init;
-
-static const struct pstore_zbackend zbackends[] = {
-#if IS_ENABLED(CONFIG_PSTORE_DEFLATE_COMPRESS)
-	{
-		.zbufsize	= zbufsize_deflate,
-		.name		= "deflate",
-	},
-#endif
-#if IS_ENABLED(CONFIG_PSTORE_LZO_COMPRESS)
-	{
-		.zbufsize	= zbufsize_lzo,
-		.name		= "lzo",
-	},
-#endif
-#if IS_ENABLED(CONFIG_PSTORE_LZ4_COMPRESS)
-	{
-		.zbufsize	= zbufsize_lz4,
-		.name		= "lz4",
-	},
-#endif
-#if IS_ENABLED(CONFIG_PSTORE_LZ4HC_COMPRESS)
-	{
-		.zbufsize	= zbufsize_lz4,
-		.name		= "lz4hc",
-	},
-#endif
-#if IS_ENABLED(CONFIG_PSTORE_842_COMPRESS)
-	{
-		.zbufsize	= zbufsize_842,
-		.name		= "842",
-	},
-#endif
-	{ }
-};
-
 static int pstore_compress(const void *in, void *out,
 			   unsigned int inlen, unsigned int outlen)
 {
@@ -256,42 +159,48 @@ static int pstore_decompress(void *in, void *out,
 	return outlen;
 }
 
+static void free_buf_for_compression(void)
+{
+	if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && !IS_ERR_OR_NULL(tfm))
+		crypto_free_comp(tfm);
+	tfm = NULL;
+	kfree(big_oops_buf);
+	big_oops_buf = NULL;
+	big_oops_buf_sz = 0;
+}
+
 static void allocate_buf_for_compression(void)
 {
-	if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS) || !zbackend)
+	if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS) || !compress)
 		return;
 
-	if (!crypto_has_comp(zbackend->name, 0, 0)) {
-		pr_err("No %s compression\n", zbackend->name);
-		return;
+	if (!crypto_has_comp(compress, 0, 0)) {
+		pr_err("No %s compression available\n", compress);
+		goto fail;
 	}
 
-	big_oops_buf_sz = zbackend->zbufsize(psinfo->bufsize);
-	if (big_oops_buf_sz <= 0)
-		return;
+	tfm = crypto_alloc_comp(compress, 0, 0);
+	if (IS_ERR_OR_NULL(tfm)) {
+		pr_err("crypto_alloc_comp(\"%s\") failed!\n", compress);
+		goto fail;
+	}
+
+	if (crypto_comp_zbufsize(tfm, psinfo->bufsize, &big_oops_buf_sz)) {
+		pr_err("crypto_comp_zbufsize() for %s failed!\n", compress);
+		goto fail;
+	}
 
 	big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL);
 	if (!big_oops_buf) {
 		pr_err("allocate compression buffer error!\n");
-		return;
+		goto fail;
 	}
 
-	tfm = crypto_alloc_comp(zbackend->name, 0, 0);
-	if (IS_ERR_OR_NULL(tfm)) {
-		kfree(big_oops_buf);
-		big_oops_buf = NULL;
-		pr_err("crypto_alloc_comp() failed!\n");
-		return;
-	}
-}
+	return;
 
-static void free_buf_for_compression(void)
-{
-	if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && !IS_ERR_OR_NULL(tfm))
-		crypto_free_comp(tfm);
-	kfree(big_oops_buf);
-	big_oops_buf = NULL;
-	big_oops_buf_sz = 0;
+fail:
+	compress = NULL;
+	free_buf_for_compression();
 }
 
 /*
@@ -587,7 +496,9 @@ int pstore_register(struct pstore_info *psi)
 	 */
 	backend = psi->name;
 
-	pr_info("Registered %s as persistent store backend\n", psi->name);
+	pr_info("Registered %s as backend%s%s.\n",
+		psi->name, compress ? " compressed with " : "",
+		compress ?: "");
 
 	module_put(owner);
 
@@ -748,22 +659,6 @@ static void pstore_timefunc(struct timer_list *unused)
 			  jiffies + msecs_to_jiffies(pstore_update_ms));
 }
 
-void __init pstore_choose_compression(void)
-{
-	const struct pstore_zbackend *step;
-
-	if (!compress)
-		return;
-
-	for (step = zbackends; step->name; step++) {
-		if (!strcmp(compress, step->name)) {
-			zbackend = step;
-			pr_info("using %s compression\n", zbackend->name);
-			return;
-		}
-	}
-}
-
 module_param(compress, charp, 0444);
 MODULE_PARM_DESC(compress, "Pstore compression to use");
 
-- 
2.17.1