Date:   Fri, 10 Dec 2021 15:21:03 -0800
From:   Dmitry Torokhov <dmitry.torokhov@gmail.com>
To:     Luis Chamberlain <mcgrof@kernel.org>
Cc:     Jessica Yu <jeyu@kernel.org>, Kees Cook <keescook@chromium.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] module: add in-kernel support for decompressing
Message-ID: <YbPg32ceRfwxIgqK@google.com>
References: <YbLvDWdyFivlj7pP@google.com>
 <YbPPHTO3URnz+tFy@bombadil.infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YbPPHTO3URnz+tFy@bombadil.infradead.org>
Precedence: bulk

On Fri, Dec 10, 2021 at 02:05:17PM -0800, Luis Chamberlain wrote:
> On Thu, Dec 09, 2021 at 10:09:17PM -0800, Dmitry Torokhov wrote:
> > +static ssize_t module_xz_decompress(struct load_info *info,
> > +				    const void *buf, size_t size)
> > +{
> > +	static const u8 signature[] = { 0xfd, '7', 'z', 'X', 'Z', 0 };
> > +	struct xz_dec *xz_dec;
> > +	struct xz_buf xz_buf;
> > +	enum xz_ret xz_ret;
> > +	size_t new_size = 0;
> > +	ssize_t retval;
> > +
> > +	if (size < sizeof(signature) ||
> > +	    memcmp(buf, signature, sizeof(signature))) {
> > +		pr_err("not an xz compressed module\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	xz_dec = xz_dec_init(XZ_DYNALLOC, (u32)-1);
> > +	if (!xz_dec)
> > +		return -ENOMEM;
> > +
> > +	xz_buf.in_size = size;
> > +	xz_buf.in = buf;
> > +	xz_buf.in_pos = 0;
> > +
> > +	do {
> > +		struct page *page = module_get_next_page(info);
> > +		if (!page) {
> > +			retval = -ENOMEM;
> > +			goto out;
> > +		}
> 
> This looks very similar to fw_decompress_xz_pages() on
> drivers/base/firmware_loader/main.c
> 
> Is there any sharing possible with decompression between the two?

Ideally we'd get proper crypto API allowing this (page by page)
decompression and then we can switch to it in both places and stop
calling into particular algorithms directly. Unfortunately this is not
something I can sign up for at this time.

As it is I am not sure if it worth to try to split out XZ code and share
it between module and firmware loader.

Thanks.

-- 
Dmitry