Re: [PATCH] libkmod: allow to fallback to user-space decompression

[Andrea Righi <andrea.righi@canonical.com>]

On Tue, Aug 29, 2023 at 07:47:05AM -0700, Lucas De Marchi wrote:
> On Tue, Aug 29, 2023 at 02:38:08PM +0200, Andrea Righi wrote:
> > Fallback to user-space decompression when the kernel cannot allocate
> > enough memory to perform the decompression.
> > 
> > This can happen with large modules, such as xfs on linux 6.5 for
> > example, an ENOMEM would be returned and the module fails to load.
> > 
> > It seems more reliable to try again with user-space decompression
> > rather than reporting an error and failing to load the module.
> > 
> > Fixes: 09c9f8c ("libkmod: Use kernel decompression when available")
> > Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
> > ---
> > libkmod/libkmod-module.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
> > index 585da41..d2d4815 100644
> > --- a/libkmod/libkmod-module.c
> > +++ b/libkmod/libkmod-module.c
> > @@ -978,7 +978,7 @@ KMOD_EXPORT int kmod_module_insert_module(struct kmod_module *mod,
> > 	}
> > 
> > 	err = do_finit_module(mod, flags, args);
> > -	if (err == -ENOSYS)
> > +	if (err == -ENOSYS || err == -ENOMEM)
> 
> oh... ENOMEM can be returned in several places. I don't think it's a great
> interface to just retry in userspace.
> 
> Luis, can we do better on the kernel side?
> 
> Andrea, did you track the exact location triggering the ENOMEM? Is it
> the return of kvmalloc_array() or alloc_page() from
> module_get_next_page()?  Or one of the previous kmalloc we use for the
> different deflate methods?

It was this kmalloc() right here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/decompress.c?h=v6.5#n244

For the records, I've also sent this patch to address the issue from a
kernel perspective:
https://lore.kernel.org/lkml/20230829120508.317611-1-andrea.righi@canonical.com/T/#u

That seems to solve the issue in my particular case, but I'm not sure if
that's a valid solution across all architectures.

Thanks,
-Andrea