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:

quoted

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?

quoted

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 (local)

-- 
Kees Cook