linux-kernel - Re: [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if available

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]

Date:   Mon, 7 Oct 2019 18:44:39 +0200
From:   Ralf Ramsauer <ralf.ramsauer@...-regensburg.de>
To:     Borislav Petkov <bp@...en8.de>
CC:     Jan Kiszka <jan.kiszka@...mens.com>, <x86@...nel.org>,
        <jailhouse-dev@...glegroups.com>, <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH v5 2/2] x86/jailhouse: Only enable platform UARTs if
 available



On 10/7/19 6:26 PM, Borislav Petkov wrote:
> On Mon, Oct 07, 2019 at 02:38:19PM +0200, Ralf Ramsauer wrote:
>> ACPI tables aren't available if Linux runs as guest of the hypervisor
>> Jailhouse. This makes the 8250 driver probe for all platform UARTs as
>> it assumes that all platform are present in case of !ACPI. Jailhouse
> 
> I think you mean s/platform/UARTs/ here.
> 
>> will stop execution of Linux guest due to port access violation.
>>
>> So far, these access violations could be solved by tuning the
>> 8250.nr_uarts parameter but it has limitations: We can, e.g., only map
> 
> Another "We" you can get rid of.
> 
>> consecutive platform UARTs to Linux, and only in the sequence 0x3f8,
>> 0x2f8, 0x3e8, 0x2e8.
>>
>> Beginning from setup_data version 2, Jailhouse will place information of
>> available platform UARTs in setup_data. This allows for selective
>> activation of platform UARTs.
>>
>> This patch queries the setup_data version and activates only available
> 
> s/This patch queries/Query/
> 
>> UARTS. It comes with backward compatibility, and will still support
>> older setup_data versions. In this case, Linux falls back to the old
>> behaviour.
>>
>> Signed-off-by: Ralf Ramsauer <ralf.ramsauer@...-regensburg.de>
>> ---
>>  arch/x86/include/uapi/asm/bootparam.h |  3 +
>>  arch/x86/kernel/jailhouse.c           | 83 +++++++++++++++++++++++----
>>  2 files changed, 74 insertions(+), 12 deletions(-)
> 
> ...
> 
>> @@ -138,6 +147,53 @@ static int __init jailhouse_pci_arch_init(void)
>>  	return 0;
>>  }
>>  
>> +#ifdef CONFIG_SERIAL_8250
>> +static bool jailhouse_uart_enabled(unsigned int uart_nr)
>> +{
>> +	return setup_data.v2.flags & BIT(uart_nr);
>> +}
>> +
>> +static void jailhouse_serial_fixup(int port, struct uart_port *up,
>> +				   u32 *capabilities)
>> +{
>> +	static const u16 pcuart_base[] = {0x3f8, 0x2f8, 0x3e8, 0x2e8};
>> +	unsigned int n;
>> +
>> +	for (n = 0; n < ARRAY_SIZE(pcuart_base); n++) {
>> +		if (pcuart_base[n] != up->iobase)
>> +			continue;
>> +
>> +		if (jailhouse_uart_enabled(n)) {
>> +			pr_info("Enabling UART%u (port 0x%lx)\n", n,
>> +				up->iobase);
>> +			jailhouse_setup_irq(up->irq);
>> +		} else {
>> +			/* Deactivate UART if access isn't allowed */
>> +			up->iobase = 0;
>> +		}
>> +		break;
>> +	}
>> +}
> 
> WARNING: vmlinux.o(.text+0x4fdb0): Section mismatch in reference from the function jailhouse_serial_fixup() to the variable .init.data:can_use_brk_pgt
> The function jailhouse_serial_fixup() references
> the variable __initdata can_use_brk_pgt.
> This is often because jailhouse_serial_fixup lacks a __initdata 
> annotation or the annotation of can_use_brk_pgt is wrong.
> 

Yep, jailhouse_serial_fixup can become __init, I didn't see that, but
you're right, thanks. I'm curious, how did you find that? "We" didn't
notice yet. :-)

BTW, we refers to the Jailhouse folks, but I will rewrite that.

Thanks
  Ralf