lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 10 Dec 2021 17:09:23 -0800 From: Dmitry Torokhov <dmitry.torokhov@...il.com> To: Luis Chamberlain <mcgrof@...nel.org> Cc: Jessica Yu <jeyu@...nel.org>, Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org Subject: Re: [PATCH v3] module: add in-kernel support for decompressing On Fri, Dec 10, 2021 at 04:11:21PM -0800, Luis Chamberlain wrote: > On Thu, Dec 09, 2021 at 10:09:17PM -0800, Dmitry Torokhov wrote: > > diff --git a/init/Kconfig b/init/Kconfig > > index cd23faa163d1..d90774ff7610 100644 > > --- a/init/Kconfig > > +++ b/init/Kconfig > > @@ -2305,6 +2305,19 @@ config MODULE_COMPRESS_ZSTD > > > > endchoice > > > > +config MODULE_DECOMPRESS > > + bool "Support in-kernel module decompression" > > + depends on MODULE_COMPRESS_GZIP || MODULE_COMPRESS_XZ > > + select ZLIB_INFLATE if MODULE_COMPRESS_GZIP > > + select XZ_DEC if MODULE_COMPRESS_XZ > > What if MODULE_COMPRESS_GZIP and MODULE_COMPRESS_XZ are enabled? > These are not mutually exclusive. They are mutually exclusive, the kernel uses the same (one) compression method for all kernel modules that it generates (i.e we do not compress drivers/usb/... with gzip while drivers/net/... with xz). The idea here is to allow the kernel consume the same format that was used when generating modules. Supporting multiple formats at once is overkill IMO. > > > + help > > + > > + Support for decompressing kernel modules by the kernel itself > > + instead of relying on userspace to perform this task. Useful when > > + load pinning security policy is enabled. > > Shouldn't kernel decompression be faster too? If so, what's the > point of doing it in userspace? Make the kernel smaller? Have more flexibility with exotic compression formats? > > > diff --git a/kernel/module_decompress.c b/kernel/module_decompress.c > > new file mode 100644 > > index 000000000000..590ca00aa098 > > --- /dev/null > > +++ b/kernel/module_decompress.c > > @@ -0,0 +1,271 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright 2021 Google LLC. > > + */ > > + > > +#include <linux/init.h> > > +#include <linux/highmem.h> > > +#include <linux/kobject.h> > > +#include <linux/mm.h> > > +#include <linux/module.h> > > +#include <linux/slab.h> > > +#include <linux/sysfs.h> > > +#include <linux/vmalloc.h> > > + > > +#include "module-internal.h" > > + > > +static int module_extend_max_pages(struct load_info *info, unsigned int extent) > > +{ > > + struct page **new_pages; > > + > > + new_pages = kvmalloc_array(info->max_pages + extent, > > + sizeof(info->pages), GFP_KERNEL); > > + if (!new_pages) > > + return -ENOMEM; > > + > > + memcpy(new_pages, info->pages, info->max_pages * sizeof(info->pages)); > > + kvfree(info->pages); > > + info->pages = new_pages; > > + info->max_pages += extent; > > + > > + return 0; > > +} > > + > > +static struct page *module_get_next_page(struct load_info *info) > > +{ > > + struct page *page; > > + int error; > > + > > + if (info->max_pages == info->used_pages) { > > + error = module_extend_max_pages(info, info->used_pages); > > + if (error) > > + return ERR_PTR(error); > > + } > > + > > + page = alloc_page(GFP_KERNEL | __GFP_HIGHMEM); > > + if (!page) > > + return ERR_PTR(-ENOMEM); > > + > > + info->pages[info->used_pages++] = page; > > + return page; > > +} > > + > > +#ifdef CONFIG_MODULE_COMPRESS_GZIP > > +#include <linux/zlib.h> > > +#define MODULE_COMPRESSION gzip > > +#define MODULE_DECOMPRESS_FN module_gzip_decompress > > So gzip is assumed if your kernel has both gzip and xz. That seems odd. No, gzuo is used when CONFIG_MODULE_COMPRESS_GZIP is enabled. That means that CONFIG_MODULE_COMPRESS_XZ is not enabled. > > <-- snip --> > > > +#elif CONFIG_MODULE_COMPRESS_XZ > > +#include <linux/xz.h> > > +#define MODULE_COMPRESSION xz > > +#define MODULE_DECOMPRESS_FN module_xz_decompress > > +#else > > <-- snip --> > > > +#error "Unexpected configuration for CONFIG_MODULE_DECOMPRESS" > > Using "depends on" logic on the kconfig symbol would resolve this and > make this not needed. > > Why can't we just inspect the module and determine? Or, why not just add > a new kconfig symbol under MODULE_DECOMPRESS which lets you specify the > MODULE_COMPRESSION_TYPE. This way this is explicit. It is a possibility, I just wanted to decompression scheme match compression one so there is less potential for misconfiguration. > > > +module_init(module_decompress_sysfs_init); > > This seems odd, altough it works, can you use late_initcall instead()? Yes, I can change this. Thanks. -- Dmitry
Powered by blists - more mailing lists