From: Julien Grall <julien@xen.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
Ian Jackson <iwj@xenproject.org>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Charles Chiou <cchiou@ambarella.com>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: dom0less boot two compressed kernel images out-of-memory work-around
Date: Thu, 4 Mar 2021 10:28:23 +0000 [thread overview]
Message-ID: <e455c5c9-c660-b11e-71c9-6072c1d40057@xen.org> (raw)
In-Reply-To: <511603c8-40b4-5e22-7eba-7722d89895fe@suse.com>
Hi Jan,
On 04/03/2021 08:34, Jan Beulich wrote:
> On 03.03.2021 20:36, Julien Grall wrote:
>> (BCCing xen-users, CCing xen-devel + a few folks)
>> How about the following (untested):
>>
>> commit e1cd2d85234c8d0aa62ad32c824a5568a57be930 (HEAD -> dev)
>> Author: Julien Grall <jgrall@amazon.com>
>> Date: Wed Mar 3 19:27:56 2021 +0000
>>
>> xen/gunzip: Allow perform_gunzip() to be called multiple times
>>
>> Currently perform_gunzip() can only be called once because the the
>> internal allocator is not fully re-initialized.
>>
>> This works fine if you are only booting dom0. But this will break when
>> booting multiple using the dom0less that uses compressed kernel images.
>>
>> This can be resolved by re-initializing malloc_ptr and malloc_count
>> every time perform_gunzip() is called.
>>
>> Note the latter is only re-initialized for hardening purpose as
>> there is
>> no guarantee that every malloc() are followed by free() (It should in
>> theory!).
>>
>> Take the opportunity to check the return of alloc_heap_pages() to
>> return
>> an error rather than dereferencing a NULL pointer later on failure.
>>
>> Reported-by: Charles Chiou <cchiou@ambarella.com>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>
>> ---
>>
>> This patch is candidate for Xen 4.15. Without this patch, it will
>> not be
>> possible to boot multiple domain using dom0less when they are using
>> compressed kernel images.
>
> Other decompression methods are unaffected?
Arm is only using gzip so far. I quickly looked through bunzip2 and
unlz4 (I know there are others), they look fine because they don't
allocate a large global pool.
We probably want to check the others.
>
>> --- a/xen/common/gunzip.c
>> +++ b/xen/common/gunzip.c
>> @@ -114,7 +114,11 @@ __init int perform_gunzip(char *output, char
>> *image, unsigned long image_len)
>> window = (unsigned char *)output;
>>
>> free_mem_ptr = (unsigned long)alloc_xenheap_pages(HEAPORDER, 0);
>> + if ( !free_mem_ptr )
>> + return -ENOMEM;
>> +
>> free_mem_end_ptr = free_mem_ptr + (PAGE_SIZE << HEAPORDER);
>> + init_allocator();
>>
>> inbuf = (unsigned char *)image;
>> insize = image_len;
>> diff --git a/xen/common/inflate.c b/xen/common/inflate.c
>> index f99c985d6135..d8c28a3e9593 100644
>> --- a/xen/common/inflate.c
>> +++ b/xen/common/inflate.c
>> @@ -238,6 +238,12 @@ STATIC const ush mask_bits[] = {
>> static unsigned long INITDATA malloc_ptr;
>> static int INITDATA malloc_count;
>>
>> +static void init_allocator(void)
>
> Please add INIT here. (I wouldn't mind if you used __init instead,
> as there's going to be file-wide replacement after 4.15 anyway,
> but of course this would render things inconsistent until then.)
I will use INIT. I will wait a bit before sending the patch formally
(I'd like a confirmation that it solves the problem reported).
Cheers,
--
Julien Grall
prev parent reply other threads:[~2021-03-04 10:28 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <c1b3f92388c34788b76193b70c150521@ambarella.com>
2021-03-03 19:36 ` dom0less boot two compressed kernel images out-of-memory work-around Julien Grall
2021-03-04 8:34 ` Jan Beulich
2021-03-04 10:28 ` Julien Grall [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e455c5c9-c660-b11e-71c9-6072c1d40057@xen.org \
--to=julien@xen.org \
--cc=andrew.cooper3@citrix.com \
--cc=cchiou@ambarella.com \
--cc=iwj@xenproject.org \
--cc=jbeulich@suse.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).