| Message ID | 1487874651-19202-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 3cf41b8728a39d1e0d3ee7ae1a1be5f4ecf3b8d9 |
| Headers | show |
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? Either way: Tested-by: Laszlo Ersek <lersek@redhat.com> Thank you very much for the quick fix! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 02/24/17 04:20, Gao, Liming wrote: > Reviewed-by: Liming Gao <liming.gao@intel.com> Thank you both, pushed as 3cf41b8728a3. Laszlo > -----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. > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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) } /*
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(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel