linux-kernel - Re: [PATCH 5/9] apm32: remove paravirt

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160219205806.GS25240@wotan.suse.de>
Date:	Fri, 19 Feb 2016 21:58:06 +0100
From:	"Luis R. Rodriguez" <mcgrof@...nel.org>
To:	Boris Ostrovsky <boris.ostrovsky@...cle.com>
Cc:	"Luis R. Rodriguez" <mcgrof@...nel.org>, bp@...en8.de,
	x86@...nel.org, linux-kernel@...r.kernel.org, luto@...capital.net,
	rusty@...tcorp.com.au, david.vrabel@...rix.com,
	konrad.wilk@...cle.com, xen-devel@...ts.xensource.com
Subject: Re: [PATCH 5/9] apm32: remove paravirt_enabled() use

On Fri, Feb 19, 2016 at 10:08:43AM -0500, Boris Ostrovsky wrote:
> 
> 
> On 02/19/2016 08:08 AM, Luis R. Rodriguez wrote:
> >There is already a check for apm_info.bios == 0, the
> >apm_info.bios is set from the boot_params.apm_bios_info.
> >Both Xen and lguest, which are also the only ones that set
> >paravirt_enabled to true) do never set the apm_bios_info,
> >the paravirt_enabled() check is simply not needed.
> 
> We need to guarantee that boot_params is filled with zeroes. On
> baremetal path we clear .bss (which is where boot_params live)
> before copying data from zero page.

To be clear Xen is the only user of this that just sets it
outright instead of poking at the file and using that as
reference, and setting what it needs. Its a good point
though that things like this which are 0 still should
probably be set.

> So we need to at least memset(&boot_params, 0, sz)

This use case would be just Xen specific, for lguest
we can just set the things we need clearly on the
launcher. So like this (I can fold in a separate
patch):


diff --git a/tools/lguest/lguest.c b/tools/lguest/lguest.c
index ff0aa580c6e1..0aa75af6e862 100644
--- a/tools/lguest/lguest.c
+++ b/tools/lguest/lguest.c
@@ -3357,6 +3357,12 @@ int main(int argc, char *argv[])
 	/* Tell the entry path not to try to reload segment registers. */
 	boot->hdr.loadflags |= KEEP_SEGMENTS;
 
+	/* We don't support tboot */
+	boot->tboot_addr = 0;
+
+	/* Ensure this is 0 to prevent apm from loading */
+	boot->apm_bios_info.version = 0;
+
 	/* We tell the kernel to initialize the Guest. */
 	tell_kernel(start);
 

> in xen_start_kernel(). Better yet, clear whole .bss.
> 
> (This applies to the next patch as well).

So clear_bss() -- oh look, another call that xen_start_kernel() could have made
good use of. :)  Can you send a respective patch I can old into this series?
I'm afraid it is by no means obvious to me where it would be safe to do this on
xen_start_kernel().

  Luis