| Message ID | 20191011000609.29728-2-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | vmlinux.lds.h: Refactor EXCEPTION_TABLE and NOTES | expand |
Kees Cook <keescook@chromium.org> writes: > The Program Header identifiers are internal to the linker scripts. In > preparation for moving the NOTES segment declaration into RO_DATA, > standardize the identifier for the PT_NOTE entry to "note" as used by > all other architectures that emit PT_NOTE. > > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > arch/powerpc/kernel/vmlinux.lds.S | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Acked-by: Michael Ellerman <mpe@ellerman.id.au> cheers > diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S > index 060a1acd7c6d..81e672654789 100644 > --- a/arch/powerpc/kernel/vmlinux.lds.S > +++ b/arch/powerpc/kernel/vmlinux.lds.S > @@ -19,7 +19,7 @@ ENTRY(_stext) > > PHDRS { > kernel PT_LOAD FLAGS(7); /* RWX */ > - notes PT_NOTE FLAGS(0); > + note PT_NOTE FLAGS(0); > dummy PT_NOTE FLAGS(0); > > /* binutils < 2.18 has a bug that makes it misbehave when taking an > @@ -177,7 +177,7 @@ SECTIONS > #endif > EXCEPTION_TABLE(0) > > - NOTES :kernel :notes > + NOTES :kernel :note > > /* The dummy segment contents for the bug workaround mentioned above > near PHDRS. */ > -- > 2.17.1
On Thu, Oct 10, 2019 at 05:05:41PM -0700, Kees Cook wrote: > The Program Header identifiers are internal to the linker scripts. In > preparation for moving the NOTES segment declaration into RO_DATA, > standardize the identifier for the PT_NOTE entry to "note" as used by > all other architectures that emit PT_NOTE. All other archs are wrong, and "notes" is a much better name. This segment does not contain a single "note", but multiple "notes". Segher
On Fri, Oct 11, 2019 at 03:25:19AM -0500, Segher Boessenkool wrote: > On Thu, Oct 10, 2019 at 05:05:41PM -0700, Kees Cook wrote: > > The Program Header identifiers are internal to the linker scripts. In > > preparation for moving the NOTES segment declaration into RO_DATA, > > standardize the identifier for the PT_NOTE entry to "note" as used by > > all other architectures that emit PT_NOTE. > > All other archs are wrong, and "notes" is a much better name. This > segment does not contain a single "note", but multiple "notes". True, but the naming appears to be based off the Program Header name of "PT_NOTE". Regardless, it is an entirely internal-to-the-linker-script identifier, so I am just consolidating on a common name with the least number of collateral changes.
On Fri, Oct 11, 2019 at 09:11:43AM -0700, Kees Cook wrote: > On Fri, Oct 11, 2019 at 03:25:19AM -0500, Segher Boessenkool wrote: > > On Thu, Oct 10, 2019 at 05:05:41PM -0700, Kees Cook wrote: > > > The Program Header identifiers are internal to the linker scripts. In > > > preparation for moving the NOTES segment declaration into RO_DATA, > > > standardize the identifier for the PT_NOTE entry to "note" as used by > > > all other architectures that emit PT_NOTE. > > > > All other archs are wrong, and "notes" is a much better name. This > > segment does not contain a single "note", but multiple "notes". > > True, but the naming appears to be based off the Program Header name of > "PT_NOTE". Ah, so that's why the kernel segment (which isn't text btw, it's rwx) is called "load" :-P (Not convinced. Some arch just got it wrong, and many others blindly copied it? That sounds a lot more likely imo.) > Regardless, it is an entirely internal-to-the-linker-script > identifier, so I am just consolidating on a common name with the least > number of collateral changes. Yes, that's what I'm complaining about. Names *matter*, internal names doubly so. So why replace a good name with a worse name? Because it is slightly less work for you? Segher p.s. Thanks for doing this, removing the powerpc workaround etc. :-)
On Fri, Oct 11, 2019 at 11:25:52AM -0500, Segher Boessenkool wrote: > Names *matter*, internal names doubly so. So why replace a good name with > a worse name? Because it is slightly less work for you? So if we agree on the name "notes" and we decide to rename the other arches, this should all be done in a separate patchset anyway, and ontop of this one. And I believe Kees wouldn't mind doing it ontop since he's gotten his hands dirty already. :-P Thx.
On Tue, Oct 15, 2019 at 06:54:13PM +0200, Borislav Petkov wrote: > On Fri, Oct 11, 2019 at 11:25:52AM -0500, Segher Boessenkool wrote: > > Names *matter*, internal names doubly so. So why replace a good name with > > a worse name? Because it is slightly less work for you? > > So if we agree on the name "notes" and we decide to rename the other > arches, this should all be done in a separate patchset anyway, and ontop > of this one. And I believe Kees wouldn't mind doing it ontop since he's > gotten his hands dirty already. :-P Yeah, I'm fine with that. I would prefer to do it as a separate step, just to minimize the logical steps each patch takes. Shall I spin a v3 with the Acks added and a final rename for this?
On Tue, Oct 15, 2019 at 06:54:13PM +0200, Borislav Petkov wrote: > On Fri, Oct 11, 2019 at 11:25:52AM -0500, Segher Boessenkool wrote: > > Names *matter*, internal names doubly so. So why replace a good name with > > a worse name? Because it is slightly less work for you? > > So if we agree on the name "notes" and we decide to rename the other > arches, this should all be done in a separate patchset anyway, and ontop > of this one. And I believe Kees wouldn't mind doing it ontop since he's > gotten his hands dirty already. :-P I've added more rationale to patch #1 in the just-sent v3 of this series. If I still can't convince you Segher, I'm happy to send "patch 30/29" to do a bulk rename to "notes". Let me know. :)
On Tue, Oct 29, 2019 at 02:15:39PM -0700, Kees Cook wrote: > On Tue, Oct 15, 2019 at 06:54:13PM +0200, Borislav Petkov wrote: > > On Fri, Oct 11, 2019 at 11:25:52AM -0500, Segher Boessenkool wrote: > > > Names *matter*, internal names doubly so. So why replace a good name with > > > a worse name? Because it is slightly less work for you? > > > > So if we agree on the name "notes" and we decide to rename the other > > arches, this should all be done in a separate patchset anyway, and ontop > > of this one. And I believe Kees wouldn't mind doing it ontop since he's > > gotten his hands dirty already. :-P > > I've added more rationale to patch #1 in the just-sent v3 of this > series. If I still can't convince you Segher, I'm happy to send "patch > 30/29" to do a bulk rename to "notes". Let me know. :) I am still not convinced the worse name is a better name, no :-) But if you don't want to do the work, and instead prefer the much smaller change, that is of course a fine decision. Thank you! (I would be happy with such a 30/29 as well, of course.) Segher
On Tue, Oct 29, 2019 at 08:01:17PM -0500, Segher Boessenkool wrote: > I am still not convinced the worse name is a better name, no :-) But if > you don't want to do the work, and instead prefer the much smaller change, > that is of course a fine decision. Thank you! > > (I would be happy with such a 30/29 as well, of course.) Ok, thanks. I'll start picking up the pile and the renaming patch can then go ontop.
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S index 060a1acd7c6d..81e672654789 100644 --- a/arch/powerpc/kernel/vmlinux.lds.S +++ b/arch/powerpc/kernel/vmlinux.lds.S @@ -19,7 +19,7 @@ ENTRY(_stext) PHDRS { kernel PT_LOAD FLAGS(7); /* RWX */ - notes PT_NOTE FLAGS(0); + note PT_NOTE FLAGS(0); dummy PT_NOTE FLAGS(0); /* binutils < 2.18 has a bug that makes it misbehave when taking an @@ -177,7 +177,7 @@ SECTIONS #endif EXCEPTION_TABLE(0) - NOTES :kernel :notes + NOTES :kernel :note /* The dummy segment contents for the bug workaround mentioned above near PHDRS. */
The Program Header identifiers are internal to the linker scripts. In preparation for moving the NOTES segment declaration into RO_DATA, standardize the identifier for the PT_NOTE entry to "note" as used by all other architectures that emit PT_NOTE. Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/powerpc/kernel/vmlinux.lds.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)