linux-kernel - Re: [PATCH v2] x86/boot: save fields explicitly, zero out everything else

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: <1ee72eca-1a09-0b71-f202-b1a909ca85b7@nvidia.com>
Date:   Wed, 7 Aug 2019 12:43:08 -0700
From:   John Hubbard <jhubbard@...dia.com>
To:     David Laight <David.Laight@...LAB.COM>,
        "H . Peter Anvin" <hpa@...or.com>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "x86@...nel.org" <x86@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] x86/boot: save fields explicitly, zero out everything
 else

On 8/7/19 4:41 AM, David Laight wrote:
> From: john.hubbard@...il.com
>> Sent: 31 July 2019 06:46
...
>>  	if (boot_params->sentinel) {
>> -		/* fields in boot_params are left uninitialized, clear them */
>> -		boot_params->acpi_rsdp_addr = 0;
>> -		memset(&boot_params->ext_ramdisk_image, 0,
>> -		       (char *)&boot_params->efi_info -
>> -			(char *)&boot_params->ext_ramdisk_image);
>> -		memset(&boot_params->kbd_status, 0,
>> -		       (char *)&boot_params->hdr -
>> -		       (char *)&boot_params->kbd_status);
>> -		memset(&boot_params->_pad7[0], 0,
>> -		       (char *)&boot_params->edd_mbr_sig_buffer[0] -
>> -			(char *)&boot_params->_pad7[0]);
>> -		memset(&boot_params->_pad8[0], 0,
>> -		       (char *)&boot_params->eddbuf[0] -
>> -			(char *)&boot_params->_pad8[0]);
>> -		memset(&boot_params->_pad9[0], 0, sizeof(boot_params->_pad9));
> ...
> 
> How about replacing the above first using:
> #define zero_struct_fields(ptr, from, to) memset(&ptr->from, 0, (char *)&ptr->to - (char *)&ptr->from)
> 	zero_struct_fields(boot_params, ext_ramdisk_image, efi_info);
> 	...
> Which is absolutely identical to the original code.
> 
> The replacing the define with:
> 	#define so(s, m) offsetof(typeof(*s), m)
> 	#define zero_struct_fields(ptr, from, to) memset((char *)ptr + so(ptr, from), 0, so(ptr, to) - so(ptr, from))
> which gcc probably doesn't complain about, but should generate identical code again.
> There might be an existing define for so().
> 

Hi David,

There was discussion about that [1], but preference ending up being to
flip this around, in order to more closely match the original intent
of this function (zero out everything except for certain carefully
selected fields), and to therefore be more likely to keep working if 
fields are added. 


[1] https://lore.kernel.org/lkml/alpine.DEB.2.21.1907252358240.1791@nanos.tec.linutronix.de/

thanks,
-- 
John Hubbard
NVIDIA