linux-kernel - RE: [PATCH resend v4] dell_rbu: make firmware payload memory uncachable

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: <eca11a9c20d14480ae02b2912b7a6447@ausx13mpc120.AMER.DELL.COM>
Date:   Fri, 6 Jul 2018 16:28:32 +0000
From:   <Mario.Limonciello@...l.com>
To:     <stuart.w.hayes@...il.com>, <linux-kernel@...r.kernel.org>,
        <Stuart.Hayes@...l.com>
CC:     <tiwai@...e.de>, <andy.shevchenko@...il.com>,
        <dvhart@...radead.org>, <platform-driver-x86@...r.kernel.org>
Subject: RE: [PATCH resend v4] dell_rbu: make firmware payload memory
 uncachable



> -----Original Message-----
> From: platform-driver-x86-owner@...r.kernel.org [mailto:platform-driver-x86-
> owner@...r.kernel.org] On Behalf Of Stuart Hayes
> Sent: Friday, July 6, 2018 11:19 AM
> To: linux-kernel@...r.kernel.org
> Cc: Takashi Iwai; Andy Shevchenko; dvhart@...radead.org; Platform Driver
> Subject: [PATCH resend v4] dell_rbu: make firmware payload memory uncachable
> 
> The dell_rbu driver takes firmware update payloads and puts them in memory so
> the system BIOS can find them after a reboot.  This sometimes fails (though
> rarely), because the memory containing the payload is in the CPU cache but
> never gets written back to main memory before the system is rebooted (CPU
> cache contents are lost on reboot).
> 
> With this patch, the payload memory will be changed to uncachable to ensure
> that the payload is actually in main memory before the system is rebooted.
> 
> Signed-off-by: Stuart Hayes <stuart.w.hayes@...il.com>
> Reviewed-by: Takashi Iwai <tiwai@...e.de>
> Signed-off-by: Takashi Iwai <tiwai@...e.de>

Reviewed-by: Mario Limonciello <mario.limonciello@...l.com>

> ---
> v2 Added include, removed extra parentheses
> v3 Corrected formatting and include line
> v4 Moved set_memory_uc() outside the while loop so that the memory is
>    definitely allocated before it is set to uncachable
> 
> This driver has no maintainer.

Stuart,

Outside of this patch, I think it would make sense to send some follow up patch
series that do the following:

1) Let you take over as maintainer (Update MAINTAINERS)
2) Move this driver to platform-x86
3) Update dell_rbu.txt with the correct URLs and mention that it's only for enterprise
hardware.    Client hardware has instead opted to support UEFI capsules for updates
from Linux.

Since it's really a platform feature and specific to only Dell enterprise hardware
I think it makes sense to move over to an area with subsystem maintainers that
can help to review patches as needed.

> 
> 
> diff --git a/drivers/firmware/dell_rbu.c b/drivers/firmware/dell_rbu.c
> index 2f452f1f7c8a..53f27a6e2d76 100644
> --- a/drivers/firmware/dell_rbu.c
> +++ b/drivers/firmware/dell_rbu.c
> @@ -45,6 +45,7 @@
>  #include <linux/moduleparam.h>
>  #include <linux/firmware.h>
>  #include <linux/dma-mapping.h>
> +#include <asm/set_memory.h>
> 
>  MODULE_AUTHOR("Abhay Salunke <abhay_salunke@...l.com>");
>  MODULE_DESCRIPTION("Driver for updating BIOS image on DELL systems");
> @@ -181,6 +182,11 @@ static int create_packet(void *data, size_t length)
>  			packet_data_temp_buf = NULL;
>  		}
>  	}
> +	/*
> +	 * set to uncachable or it may never get written back before reboot
> +	 */
> +	set_memory_uc((unsigned long)packet_data_temp_buf, 1 << ordernum);
> +
>  	spin_lock(&rbu_data.lock);
> 
>  	newpacket->data = packet_data_temp_buf;
> @@ -349,6 +355,8 @@ static void packet_empty_list(void)
>  		 * to make sure there are no stale RBU packets left in memory
>  		 */
>  		memset(newpacket->data, 0, rbu_data.packetsize);
> +		set_memory_wb((unsigned long)newpacket->data,
> +			1 << newpacket->ordernum);
>  		free_pages((unsigned long) newpacket->data,
>  			newpacket->ordernum);
>  		kfree(newpacket);