linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>,
	pbonzini@redhat.com, ebiggers@kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	ardb@kernel.org, kraxel@redhat.com, philmd@linaro.org
Subject: Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
Date: Fri, 30 Dec 2022 20:54:11 +0100	[thread overview]
Message-ID: <Y69B40T9kWfxZpmf@zn.tnic> (raw)
In-Reply-To: <CAHmME9oPUJemVRvO3HX0q4BJGTFuzbLYANeizuRcNq2=Ykk1Gg@mail.gmail.com>

On Fri, Dec 30, 2022 at 06:07:24PM +0100, Jason A. Donenfeld wrote:
> Look closer at the boot process. The compressed image is initially at
> 0x100000, but it gets relocated to a safer area at the end of
> startup_64:

That is the address we're executing here from, rip here looks like 0x100xxx.

> /*
>  * Copy the compressed kernel to the end of our buffer
>  * where decompression in place becomes safe.
>  */
>         pushq   %rsi
>         leaq    (_bss-8)(%rip), %rsi
>         leaq    rva(_bss-8)(%rbx), %rdi

when you get to here, it looks something like this:

        leaq    (_bss-8)(%rip), %rsi		# 0x9e7ff8
        leaq    rva(_bss-8)(%rbx), %rdi		# 0xc6eeff8

so the source address is that _bss thing and we copy...

>         movl    $(_bss - startup_32), %ecx
>         shrl    $3, %ecx
>         std

... backwards since DF=1.

Up to:

# rsi = 0xffff8
# rdi = 0xbe06ff8

Ok, so the source address is 0x100000. Good.

> HOWEVER, qemu currently appends setup_data to the end of the
> compressed kernel image,

Yeah, you mean the kernel which starts executing at 0x100000, i.e., that part
which is compressed/head_64.S and which does the above and the relocation etc.

> and this part isn't moved, and setup_data links aren't walked/relocated. So
> that means the original address remains, of 0x100000.

See above: when it starts copying the kernel image backwards to a higher
address, that last byte is at 0x9e7ff8 so I'm guessing qemu has put setup_data
*after* that address. And that doesn't get copied ofc.

So far, so good.

Now later, we extract the compressed kernel created with the mkpiggy magic:

input_data:
.incbin "arch/x86/boot/compressed/vmlinux.bin.gz"
input_data_end:

by doing

/*
 * Do the extraction, and jump to the new kernel..
 */

        pushq   %rsi                    /* Save the real mode argument */	0x13d00
        movq    %rsi, %rdi              /* real mode address */			0x13d00
        leaq    boot_heap(%rip), %rsi   /* malloc area for uncompression */	0xc6ef000
        leaq    input_data(%rip), %rdx  /* input_data */			0xbe073a8
        movl    input_len(%rip), %ecx   /* input_len */				0x8cfe13
        movq    %rbp, %r8               /* output target address */		0x1000000
        movl    output_len(%rip), %r9d  /* decompressed length, end of relocs */
        call    extract_kernel          /* returns kernel location in %rax */
        popq    %rsi

(actual addresses at the end.)

Now, when you say you triplefault somewhere in initialize_identity_maps() when
trying to access setup_data, then if you look a couple of lines before that call
we do

	call load_stage2_idt

which sets up a boottime #PF handler do_boot_page_fault() and it actually does
call kernel_add_identity_map() so *actually* it should map any unmapped
setup_data addresses.

So why doesn't it do that and why do you triplefault?

Hmmm.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

  reply	other threads:[~2022-12-30 19:54 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-28 14:38 [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data Jason A. Donenfeld
2022-12-28 16:02 ` Philippe Mathieu-Daudé
2022-12-28 16:30   ` Jason A. Donenfeld
2022-12-28 16:57     ` Jason A. Donenfeld
2022-12-28 23:58       ` H. Peter Anvin
2022-12-29  2:13         ` H. Peter Anvin
2022-12-29  2:31         ` Jason A. Donenfeld
2022-12-29  7:28           ` Philippe Mathieu-Daudé
2022-12-29  7:30           ` H. Peter Anvin
2022-12-29  7:31           ` H. Peter Anvin
2022-12-29 12:47             ` Borislav Petkov
2022-12-30 15:54               ` Jason A. Donenfeld
2022-12-30 17:01                 ` Borislav Petkov
2022-12-30 17:07                   ` Jason A. Donenfeld
2022-12-30 19:54                     ` Borislav Petkov [this message]
2022-12-30 21:58                       ` H. Peter Anvin
2022-12-30 22:10                         ` Jason A. Donenfeld
2022-12-31  1:06                           ` H. Peter Anvin
2022-12-31  1:14                             ` H. Peter Anvin
2022-12-31 12:55                             ` Jason A. Donenfeld
2022-12-31 13:40                             ` Borislav Petkov
2022-12-31 13:44                               ` Jason A. Donenfeld
2022-12-31 13:48                                 ` Borislav Petkov
2022-12-31 13:51                                   ` Jason A. Donenfeld
2022-12-31 14:24                                     ` Borislav Petkov
2022-12-31 18:22                                       ` Jason A. Donenfeld
2022-12-31 19:00                                         ` Borislav Petkov
2023-01-01  3:21                                           ` H. Peter Anvin
2023-01-01  3:31                                             ` H. Peter Anvin
2023-01-02  6:01                                               ` Borislav Petkov
2023-01-02  6:17                                                 ` Borislav Petkov
2023-01-02  9:32                                                   ` Ard Biesheuvel
2023-01-02 13:36                                                     ` Borislav Petkov
2023-01-02 15:03                                                       ` Ard Biesheuvel
2023-01-02  5:50                                             ` Borislav Petkov
2023-01-01  4:33                                         ` H. Peter Anvin
2023-01-01  4:55                                           ` Mika Penttilä
2023-01-01  5:13                                             ` H. Peter Anvin
2022-12-30 15:59             ` Jason A. Donenfeld
2022-12-30 16:21               ` Jason A. Donenfeld
2022-12-30 19:13               ` H. Peter Anvin
2022-12-31  9:48               ` Borislav Petkov
2022-12-31 12:54                 ` Jason A. Donenfeld
2022-12-31 13:35                   ` Borislav Petkov
2022-12-31 13:42                     ` Jason A. Donenfeld
2022-12-30 18:30 ` Jason A. Donenfeld

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=Y69B40T9kWfxZpmf@zn.tnic \
    --to=bp@alien8.de \
    --cc=Jason@zx2c4.com \
    --cc=ardb@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=hpa@zytor.com \
    --cc=kraxel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=x86@kernel.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).