Re: [PATCH] BaseTools: GCC: move most AutoGen.obj contents back to .data section

["Gao, Liming" <liming.gao@intel.com>]

Reviewed-by: Liming Gao <liming.gao@intel.com>

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Friday, February 24, 2017 6:29 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: Re: [PATCH] BaseTools: GCC: move most AutoGen.obj contents back to .data section

On 23 February 2017 at 19:53, Laszlo Ersek <lersek@redhat.com> wrote:
> On 02/23/17 19:30, Ard Biesheuvel wrote:
>> The generated AutoGen.c files mostly contain read-only data, but due to
>> lacking annotations, all of it is emitted into the .data section by the
>> compiler.
>>
>> Given that GUIDs are UEFI's gaffer tape, having writable GUIDs is a
>> security hazard, and this was the main rationale for putting AutoGen.obj
>> in the .text section. However, as it turns out, patchable PCDs are emitted
>> there as well, which can legally be modified at runtime.
>>
>> So update the wildcard pattern to only match g...Guid sections, and move
>> everything else back to .data (Note that this relies on -fdata-sections,
>> without that option, everything is emitted into .data)
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  BaseTools/Scripts/GccBase.lds | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
>> index 900848747144..41e5c0b4a769 100644
>> --- a/BaseTools/Scripts/GccBase.lds
>> +++ b/BaseTools/Scripts/GccBase.lds
>> @@ -32,11 +32,14 @@ SECTIONS {
>>      *(.got .got.*)
>>
>>      /*
>> -     * The contents of AutoGen.c files are constant from the POV of the program,
>> -     * but most of its contents end up in .data or .bss by default since few of
>> +     * The contents of AutoGen.c files are mostly constant from the POV of the
>> +     * program, but most of it ends up in .data or .bss by default since few of
>>       * the variable definitions that get emitted are declared as CONST.
>> +     * Unfortunately, we cannot pull it into the .text section entirely, since
>> +     * patchable PCDs are also emitted here, but we can at least move all of the
>> +     * emitted GUIDs here.
>>       */
>> -    *:AutoGen.obj(.data .data.* .bss .bss.*)
>> +    *:AutoGen.obj(.data.g*Guid)
>>    }
>>
>>    /*
>>
>
> Do you agree to add:
>
> Fixes: 233bd25b000f92fc4bbe181fa48edcd72808de8e
>
> to the commit message, or to reference that commit in some other form?
>

Yes, that makes sense.

> Either way:
>
> Tested-by: Laszlo Ersek <lersek@redhat.com>
>
> Thank you very much for the quick fix!

No problem.