Re: [PATCH] module: add in-kernel support for decompressing

From: Dmitry Torokhov
Date: Fri Dec 03 2021 - 16:06:13 EST


On Thu, Dec 02, 2021 at 12:14:18AM -0800, Kees Cook wrote:
> On Wed, Dec 01, 2021 at 12:19:08PM -0800, Dmitry Torokhov wrote:
> > On Wed, Dec 01, 2021 at 11:24:35AM -0800, Kees Cook wrote:
> > > On Sat, Nov 27, 2021 at 09:48:22PM -0800, Dmitry Torokhov wrote:
> > > > /* Flags for sys_finit_module: */
> > > > #define MODULE_INIT_IGNORE_MODVERSIONS 1
> > > > #define MODULE_INIT_IGNORE_VERMAGIC 2
> > > > +#define MODULE_INIT_COMPRESSED_DATA 4
> > >
> > > bikeshedding: adding "_DATA" seems redundant/misleading? The entire
> > > module is compressed, so maybe call it just MODULE_INIT_COMPRESSED ?
> >
> > OK, or maybe MODULE_INIT_COMPRESSED_FILE since we are indeed dealing
> > with a file?
>
> Sounds good to me! :)
>
> As far as my tangent on using the crypto subsystem, I think that looks
> like a long path, so your existing routines are likely the right place
> to start. I still wonder if it might be able to use of the "acomp" API
> instead of calling directly into the specific decompressor.

I looked at the acomp and it still needs preallocation of the output
buffer, so it has to wait for your "get worst size" API additions before
it can be used. But ideally I'd have some streaming option where I could
feed it chunks of data and consume output...

Also, regarding your comment about not changing free_copy() but move
this logic to load_module(): free copy is called twice there, both in
success and error paths, and can't really be reduced to one call site
even with "jumping backwards goto", so I'd be forced to duplicate this
logic in 2 places, which is not great. Or did I misunderstand your idea?

Thanks.

--
Dmitry