linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
@ 2024-03-17 15:05 Guixiong Wei
  2024-03-18 21:40 ` Kees Cook
  2024-03-22  8:56 ` [tip: x86/boot] x86/boot: Ignore relocations in .notes sections in walk_relocs() too tip-bot2 for Guixiong Wei
  0 siblings, 2 replies; 14+ messages in thread
From: Guixiong Wei @ 2024-03-17 15:05 UTC (permalink / raw)
  To: keescook, tglx
  Cc: jgross, mingo, bp, dave.hansen, x86, hpa, peterz, gregkh,
	tony.luck, adobriyan, linux-kernel, linux-hardening, weiguixiong

The commit aaa8736370db ("x86, relocs: Ignore relocations in
.notes section") only ignore .note section on print_absolute_relocs,
but it also need to add on walk_relocs to avoid relocations in .note
section.

Fixes: aaa8736370db ("x86, relocs: Ignore relocations in .notes section")
Fixes: 5ead97c84fa7 ("xen: Core Xen implementation")
Fixes: da1a679cde9b ("Add /sys/kernel/notes")
Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
---
 arch/x86/tools/relocs.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index b029fb81ebee..33844b33b8b9 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -746,6 +746,16 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel,
 		if (!(sec_applies->shdr.sh_flags & SHF_ALLOC)) {
 			continue;
 		}
+
+		/*
+		 * Do not perform relocations in .notes section; any
+		 * values there are meant for pre-boot consumption (e.g.
+		 * startup_xen).
+		 */
+		if (sec_applies->shdr.sh_type == SHT_NOTE) {
+			continue;
+		}
+
 		sh_symtab = sec_symtab->symtab;
 		sym_strtab = sec_symtab->link->strtab;
 		for (j = 0; j < sec->shdr.sh_size/sizeof(Elf_Rel); j++) {
-- 
2.24.3 (Apple Git-128)


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-17 15:05 [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs Guixiong Wei
@ 2024-03-18 21:40 ` Kees Cook
  2024-03-18 21:56   ` Borislav Petkov
  2024-03-22  8:45   ` Ingo Molnar
  2024-03-22  8:56 ` [tip: x86/boot] x86/boot: Ignore relocations in .notes sections in walk_relocs() too tip-bot2 for Guixiong Wei
  1 sibling, 2 replies; 14+ messages in thread
From: Kees Cook @ 2024-03-18 21:40 UTC (permalink / raw)
  To: tglx, Guixiong Wei
  Cc: Kees Cook, jgross, mingo, bp, dave.hansen, x86, hpa, peterz,
	gregkh, tony.luck, adobriyan, linux-kernel, linux-hardening

On Sun, 17 Mar 2024 23:05:47 +0800, Guixiong Wei wrote:
> The commit aaa8736370db ("x86, relocs: Ignore relocations in
> .notes section") only ignore .note section on print_absolute_relocs,
> but it also need to add on walk_relocs to avoid relocations in .note
> section.
> 
> 

Applied to for-next/hardening, thanks!

[1/1] x86, relocs: Ignore relocations in .notes section on walk_relocs
      https://git.kernel.org/kees/c/6ba438a29b5d

Take care,

-- 
Kees Cook


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-18 21:40 ` Kees Cook
@ 2024-03-18 21:56   ` Borislav Petkov
  2024-03-18 23:45     ` Kees Cook
  2024-03-22  8:45   ` Ingo Molnar
  1 sibling, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2024-03-18 21:56 UTC (permalink / raw)
  To: Kees Cook
  Cc: tglx, Guixiong Wei, jgross, mingo, dave.hansen, x86, hpa, peterz,
	gregkh, tony.luck, adobriyan, linux-kernel, linux-hardening

On Mon, Mar 18, 2024 at 02:40:50PM -0700, Kees Cook wrote:
> Applied to for-next/hardening

Why?

This is a patch that should go through the tip tree, if at all.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-18 21:56   ` Borislav Petkov
@ 2024-03-18 23:45     ` Kees Cook
  2024-03-19  8:16       ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: Kees Cook @ 2024-03-18 23:45 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: tglx, Guixiong Wei, jgross, mingo, dave.hansen, x86, hpa, peterz,
	gregkh, tony.luck, adobriyan, linux-kernel, linux-hardening

On Mon, Mar 18, 2024 at 10:56:12PM +0100, Borislav Petkov wrote:
> On Mon, Mar 18, 2024 at 02:40:50PM -0700, Kees Cook wrote:
> > Applied to for-next/hardening
> 
> Why?
> 
> This is a patch that should go through the tip tree, if at all.

The commit it refs to landed via -hardening, so I was taking the
responsibility of landing this fix too. But it's fine to go via
-tip if you prefer?

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-18 23:45     ` Kees Cook
@ 2024-03-19  8:16       ` Borislav Petkov
  2024-03-19 16:56         ` Kees Cook
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2024-03-19  8:16 UTC (permalink / raw)
  To: Kees Cook
  Cc: tglx, Guixiong Wei, jgross, mingo, dave.hansen, x86, hpa, peterz,
	gregkh, tony.luck, adobriyan, linux-kernel, linux-hardening

On Mon, Mar 18, 2024 at 04:45:45PM -0700, Kees Cook wrote:
> The commit it refs to landed via -hardening,

Yap, saw that. It should've gone through tip too as it is clearly a tip
tree patch.

> responsibility of landing this fix too. But it's fine to go via -tip
> if you prefer?

Yes, please. Just send a Reviewed-by and it'll get picked up.

Btw, while looking at that second patch, why does it have *three* Fixes:
tags? I think it wants to fix only your

aaa8736370db ("x86, relocs: Ignore relocations in .notes section")

?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-19  8:16       ` Borislav Petkov
@ 2024-03-19 16:56         ` Kees Cook
  2024-03-22 19:46           ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: Kees Cook @ 2024-03-19 16:56 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: tglx, Guixiong Wei, jgross, mingo, dave.hansen, x86, hpa, peterz,
	gregkh, tony.luck, adobriyan, linux-kernel, linux-hardening

On Tue, Mar 19, 2024 at 09:16:40AM +0100, Borislav Petkov wrote:
> On Mon, Mar 18, 2024 at 04:45:45PM -0700, Kees Cook wrote:
> > The commit it refs to landed via -hardening,
> 
> Yap, saw that. It should've gone through tip too as it is clearly a tip
> tree patch.
> 
> > responsibility of landing this fix too. But it's fine to go via -tip
> > if you prefer?
> 
> Yes, please. Just send a Reviewed-by and it'll get picked up.

Okay, thanks!

Reviewed-by: Kees Cook <keescook@chromium.org>

> 
> Btw, while looking at that second patch, why does it have *three* Fixes:
> tags? I think it wants to fix only your
> 
> aaa8736370db ("x86, relocs: Ignore relocations in .notes section")

As I understand it, the first first was incomplete.

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-18 21:40 ` Kees Cook
  2024-03-18 21:56   ` Borislav Petkov
@ 2024-03-22  8:45   ` Ingo Molnar
  2024-03-22 23:41     ` Kees Cook
  1 sibling, 1 reply; 14+ messages in thread
From: Ingo Molnar @ 2024-03-22  8:45 UTC (permalink / raw)
  To: Kees Cook
  Cc: tglx, Guixiong Wei, jgross, mingo, bp, dave.hansen, x86, hpa,
	peterz, gregkh, tony.luck, adobriyan, linux-kernel,
	linux-hardening


* Kees Cook <keescook@chromium.org> wrote:

> On Sun, 17 Mar 2024 23:05:47 +0800, Guixiong Wei wrote:
> > The commit aaa8736370db ("x86, relocs: Ignore relocations in
> > .notes section") only ignore .note section on print_absolute_relocs,
> > but it also need to add on walk_relocs to avoid relocations in .note
> > section.
> > 
> > 
> 
> Applied to for-next/hardening, thanks!
> 
> [1/1] x86, relocs: Ignore relocations in .notes section on walk_relocs
>       https://git.kernel.org/kees/c/6ba438a29b5d

Please don't - these are x86 patches, plus it contains an eyesore - see 
below ...

Thanks,

	Ingo

 relocs.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Index: tip/arch/x86/tools/relocs.c
===================================================================
--- tip.orig/arch/x86/tools/relocs.c
+++ tip/arch/x86/tools/relocs.c
@@ -752,9 +752,8 @@ static void walk_relocs(int (*process)(s
 		 * values there are meant for pre-boot consumption (e.g.
 		 * startup_xen).
 		 */
-		if (sec_applies->shdr.sh_type == SHT_NOTE) {
+		if (sec_applies->shdr.sh_type == SHT_NOTE)
 			continue;
-		}
 
 		sh_symtab = sec_symtab->symtab;
 		sym_strtab = sec_symtab->link->strtab;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tip: x86/boot] x86/boot: Ignore relocations in .notes sections in walk_relocs() too
  2024-03-17 15:05 [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs Guixiong Wei
  2024-03-18 21:40 ` Kees Cook
@ 2024-03-22  8:56 ` tip-bot2 for Guixiong Wei
  1 sibling, 0 replies; 14+ messages in thread
From: tip-bot2 for Guixiong Wei @ 2024-03-22  8:56 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Guixiong Wei, Ingo Molnar, Kees Cook, x86, linux-kernel

The following commit has been merged into the x86/boot branch of tip:

Commit-ID:     76e9762d66373354b45c33b60e9a53ef2a3c5ff2
Gitweb:        https://git.kernel.org/tip/76e9762d66373354b45c33b60e9a53ef2a3c5ff2
Author:        Guixiong Wei <weiguixiong@bytedance.com>
AuthorDate:    Sun, 17 Mar 2024 23:05:47 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 22 Mar 2024 09:48:59 +01:00

x86/boot: Ignore relocations in .notes sections in walk_relocs() too

Commit:

  aaa8736370db ("x86, relocs: Ignore relocations in .notes section")

... only started ignoring the .notes sections in print_absolute_relocs(),
but the same logic should also by applied in walk_relocs() to avoid
such relocations.

[ mingo: Fixed various typos in the changelog, removed extra curly braces from the code. ]

Fixes: aaa8736370db ("x86, relocs: Ignore relocations in .notes section")
Fixes: 5ead97c84fa7 ("xen: Core Xen implementation")
Fixes: da1a679cde9b ("Add /sys/kernel/notes")
Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20240317150547.24910-1-weiguixiong@bytedance.com
---
 arch/x86/tools/relocs.c |  9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index b029fb8..e7a44a7 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -746,6 +746,15 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel,
 		if (!(sec_applies->shdr.sh_flags & SHF_ALLOC)) {
 			continue;
 		}
+
+		/*
+		 * Do not perform relocations in .notes sections; any
+		 * values there are meant for pre-boot consumption (e.g.
+		 * startup_xen).
+		 */
+		if (sec_applies->shdr.sh_type == SHT_NOTE)
+			continue;
+
 		sh_symtab = sec_symtab->symtab;
 		sym_strtab = sec_symtab->link->strtab;
 		for (j = 0; j < sec->shdr.sh_size/sizeof(Elf_Rel); j++) {

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-19 16:56         ` Kees Cook
@ 2024-03-22 19:46           ` Borislav Petkov
  2024-03-22 23:40             ` Kees Cook
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2024-03-22 19:46 UTC (permalink / raw)
  To: Kees Cook
  Cc: tglx, Guixiong Wei, jgross, mingo, dave.hansen, x86, hpa, peterz,
	gregkh, tony.luck, adobriyan, linux-kernel, linux-hardening

On Tue, Mar 19, 2024 at 09:56:29AM -0700, Kees Cook wrote:
> > Yes, please. Just send a Reviewed-by and it'll get picked up.
> 
> Okay, thanks!

Dammit, how did this commit land upstream and in stable?!

Forgot to zap it from your tree and sent the branch to Linus anyway?

Kees, please refrain from taking tip patches in the future. You know how
this works - get_maintainers.pl.

Thx.

Date: Fri, 22 Mar 2024 14:47:05 -0400
From: Sasha Levin <sashal@kernel.org>
To: stable-commits@vger.kernel.org, keescook@chromium.org
Cc: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
 Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>,
 x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>
Subject: Patch "x86, relocs: Ignore relocations in .notes section" has been
 added to the 5.4-stable tree
X-Mailer: git-send-email 2.43.0
Message-ID: <20240322184705.144463-1-sashal@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=utf-8

This is a note to let you know that I've just added the patch titled

    x86, relocs: Ignore relocations in .notes section

to the 5.4-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     x86-relocs-ignore-relocations-in-.notes-section.patch
and it can be found in the queue-5.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.



commit 91aa857ccbd1212a23cd80bb45f71715f2db7144
Author: Kees Cook <keescook@chromium.org>
Date:   Tue Feb 27 09:51:12 2024 -0800

    x86, relocs: Ignore relocations in .notes section
    
    [ Upstream commit aaa8736370db1a78f0e8434344a484f9fd20be3b ]
    
    When building with CONFIG_XEN_PV=y, .text symbols are emitted into
    the .notes section so that Xen can find the "startup_xen" entry point.
    This information is used prior to booting the kernel, so relocations
    are not useful. In fact, performing relocations against the .notes
    section means that the KASLR base is exposed since /sys/kernel/notes
    is world-readable.
    
    To avoid leaking the KASLR base without breaking unprivileged tools that
    are expecting to read /sys/kernel/notes, skip performing relocations in
    the .notes section. The values readable in .notes are then identical to
    those found in System.map.
    
    Reported-by: Guixiong Wei <guixiongwei@gmail.com>
    Closes: https://lore.kernel.org/all/20240218073501.54555-1-guixiongwei@gmail.com/
    Fixes: 5ead97c84fa7 ("xen: Core Xen implementation")
    Fixes: da1a679cde9b ("Add /sys/kernel/notes")
    Reviewed-by: Juergen Gross <jgross@suse.com>
    Signed-off-by: Kees Cook <keescook@chromium.org>
    Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 1c3a1962cade6..0043fd374a62f 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -596,6 +596,14 @@ static void print_absolute_relocs(void)
 		if (!(sec_applies->shdr.sh_flags & SHF_ALLOC)) {
 			continue;
 		}
+		/*
+		 * Do not perform relocations in .notes section; any
+		 * values there are meant for pre-boot consumption (e.g.
+		 * startup_xen).
+		 */
+		if (sec_applies->shdr.sh_type == SHT_NOTE) {
+			continue;
+		}
 		sh_symtab  = sec_symtab->symtab;
 		sym_strtab = sec_symtab->link->strtab;
 		for (j = 0; j < sec->shdr.sh_size/sizeof(Elf_Rel); j++) {

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-22 19:46           ` Borislav Petkov
@ 2024-03-22 23:40             ` Kees Cook
  2024-03-23 10:38               ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: Kees Cook @ 2024-03-22 23:40 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: tglx, Guixiong Wei, jgross, mingo, dave.hansen, x86, hpa, peterz,
	gregkh, tony.luck, adobriyan, linux-kernel, linux-hardening

On Fri, Mar 22, 2024 at 08:46:58PM +0100, Borislav Petkov wrote:
> On Tue, Mar 19, 2024 at 09:56:29AM -0700, Kees Cook wrote:
> > > Yes, please. Just send a Reviewed-by and it'll get picked up.
> > 
> > Okay, thanks!
> 
> Dammit, how did this commit land upstream and in stable?!

There are 2 related commits. This one ("... on walk_relocs") isn't in
Linus's tree nor stable.

(Thank you Ingo for taking it now.)

> Forgot to zap it from your tree and sent the branch to Linus anyway?
> 
> Kees, please refrain from taking tip patches in the future. You know how
> this works - get_maintainers.pl.

The earlier patch, commit aaa8736370db ("x86, relocs: Ignore relocations
in .notes section"), landed via my tree. It was sent out on Feb 22nd
(v1[1]) and got a suggestion from HPA and a Review from Juergen Gross.
I sent v2 Feb 27th[2] and it sat ignored for two weeks. Since it was a
10 year old kernel address exposure, I sent it to Linus on Mar 12th[3].

-Kees

[1] https://lore.kernel.org/all/20240222171840.work.027-kees@kernel.org/
[2] https://lore.kernel.org/all/20240227175746.it.649-kees@kernel.org/
[3] https://lore.kernel.org/lkml/202403111702.828C918E55@keescook/

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-22  8:45   ` Ingo Molnar
@ 2024-03-22 23:41     ` Kees Cook
  2024-03-24  3:57       ` [PATCH] x86/build: Clean up arch/x86/tools/relocs.c a bit Ingo Molnar
  0 siblings, 1 reply; 14+ messages in thread
From: Kees Cook @ 2024-03-22 23:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: tglx, Guixiong Wei, jgross, mingo, bp, dave.hansen, x86, hpa,
	peterz, gregkh, tony.luck, adobriyan, linux-kernel,
	linux-hardening

On Fri, Mar 22, 2024 at 09:45:12AM +0100, Ingo Molnar wrote:
> 
> * Kees Cook <keescook@chromium.org> wrote:
> 
> > On Sun, 17 Mar 2024 23:05:47 +0800, Guixiong Wei wrote:
> > > The commit aaa8736370db ("x86, relocs: Ignore relocations in
> > > .notes section") only ignore .note section on print_absolute_relocs,
> > > but it also need to add on walk_relocs to avoid relocations in .note
> > > section.
> > > 
> > > 
> > 
> > Applied to for-next/hardening, thanks!
> > 
> > [1/1] x86, relocs: Ignore relocations in .notes section on walk_relocs
> >       https://git.kernel.org/kees/c/6ba438a29b5d
> 
> Please don't - these are x86 patches, plus it contains an eyesore - see 
> below ...

Dropped.

> 
> Thanks,
> 
> 	Ingo
> 
>  relocs.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> Index: tip/arch/x86/tools/relocs.c
> ===================================================================
> --- tip.orig/arch/x86/tools/relocs.c
> +++ tip/arch/x86/tools/relocs.c
> @@ -752,9 +752,8 @@ static void walk_relocs(int (*process)(s
>  		 * values there are meant for pre-boot consumption (e.g.
>  		 * startup_xen).
>  		 */
> -		if (sec_applies->shdr.sh_type == SHT_NOTE) {
> +		if (sec_applies->shdr.sh_type == SHT_NOTE)
>  			continue;
> -		}

I think the patch was trying to follow the existing code style in the
file. See line 733, for example:

 		if (sec->shdr.sh_type != SHT_REL_TYPE) {
 			continue;
 		}

But yes, agreed, your change is good. :)

-Kees

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-22 23:40             ` Kees Cook
@ 2024-03-23 10:38               ` Borislav Petkov
  2024-03-25 20:23                 ` Kees Cook
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2024-03-23 10:38 UTC (permalink / raw)
  To: Kees Cook
  Cc: tglx, Guixiong Wei, jgross, mingo, dave.hansen, x86, hpa, peterz,
	gregkh, tony.luck, adobriyan, linux-kernel, linux-hardening

On Fri, Mar 22, 2024 at 04:40:11PM -0700, Kees Cook wrote:
> The earlier patch, commit aaa8736370db ("x86, relocs: Ignore relocations
> in .notes section"), landed via my tree. It was sent out on Feb 22nd
> (v1[1]) and got a suggestion from HPA and a Review from Juergen Gross.
> I sent v2 Feb 27th[2] and it sat ignored for two weeks.

s/ignored for two weeks/missed in the avalance of patches/

> Since it was a 10 year old kernel address exposure, I sent it to Linus
> on Mar 12th[3].

So is there some unwritten understanding somewhere which says that you
should take tip patches through your tree?

Maybe I've missed it.

If there isn't, should we agree on something?

Because there clearly is a need for clarification here...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] x86/build: Clean up arch/x86/tools/relocs.c a bit
  2024-03-22 23:41     ` Kees Cook
@ 2024-03-24  3:57       ` Ingo Molnar
  0 siblings, 0 replies; 14+ messages in thread
From: Ingo Molnar @ 2024-03-24  3:57 UTC (permalink / raw)
  To: Kees Cook
  Cc: tglx, Guixiong Wei, jgross, mingo, bp, dave.hansen, x86, hpa,
	peterz, gregkh, tony.luck, adobriyan, linux-kernel,
	linux-hardening


* Kees Cook <keescook@chromium.org> wrote:

> On Fri, Mar 22, 2024 at 09:45:12AM +0100, Ingo Molnar wrote:
> > 
> > * Kees Cook <keescook@chromium.org> wrote:
> > 
> > > On Sun, 17 Mar 2024 23:05:47 +0800, Guixiong Wei wrote:
> > > > The commit aaa8736370db ("x86, relocs: Ignore relocations in
> > > > .notes section") only ignore .note section on print_absolute_relocs,
> > > > but it also need to add on walk_relocs to avoid relocations in .note
> > > > section.
> > > > 
> > > > 
> > > 
> > > Applied to for-next/hardening, thanks!
> > > 
> > > [1/1] x86, relocs: Ignore relocations in .notes section on walk_relocs
> > >       https://git.kernel.org/kees/c/6ba438a29b5d
> > 
> > Please don't - these are x86 patches, plus it contains an eyesore - see 
> > below ...
> 
> Dropped.
> 
> > 
> > Thanks,
> > 
> > 	Ingo
> > 
> >  relocs.c |    3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > Index: tip/arch/x86/tools/relocs.c
> > ===================================================================
> > --- tip.orig/arch/x86/tools/relocs.c
> > +++ tip/arch/x86/tools/relocs.c
> > @@ -752,9 +752,8 @@ static void walk_relocs(int (*process)(s
> >  		 * values there are meant for pre-boot consumption (e.g.
> >  		 * startup_xen).
> >  		 */
> > -		if (sec_applies->shdr.sh_type == SHT_NOTE) {
> > +		if (sec_applies->shdr.sh_type == SHT_NOTE)
> >  			continue;
> > -		}
> 
> I think the patch was trying to follow the existing code style in the
> file. See line 733, for example:
> 
>  		if (sec->shdr.sh_type != SHT_REL_TYPE) {
>  			continue;
>  		}

The 'existing code style' is a hodgepodge of CodingStyle compliant and 
non-compliant bits - and our preference is to not propagate crap ...

Anyway, I cleaned it all up a bit in tip:x86/boot with the patch below.

Thanks,

	Ingo

================>
From: Ingo Molnar <mingo@kernel.org>
Date: Sun, 24 Mar 2024 04:46:10 +0100
Subject: [PATCH] x86/build: Clean up arch/x86/tools/relocs.c a bit

So:

 - Follow Documentation/CodingStyle for:
    - curly braces
    - variable definitions
    - return statements
    - etc.
    - Fix unnecessary linebreaks
    - Don't split user-visible error strings over multiple lines ...

 - It's fine to use vertical alignment to make code more readable,
   but it should be internally consistent for definitions visible
   on a single page ...

 - There's 40+ die() statements that are basically asserts and
   never trigger. Make them single-line, to move them out of
   sight as much as possible.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/tools/relocs.c | 362 ++++++++++++++++++++++++------------------------
 1 file changed, 178 insertions(+), 184 deletions(-)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index e7a44a7f617f..c101bed61940 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -11,41 +11,42 @@
 #define Elf_Shdr		ElfW(Shdr)
 #define Elf_Sym			ElfW(Sym)
 
-static Elf_Ehdr		ehdr;
-static unsigned long	shnum;
-static unsigned int	shstrndx;
-static unsigned int	shsymtabndx;
-static unsigned int	shxsymtabndx;
+static Elf_Ehdr			ehdr;
+static unsigned long		shnum;
+static unsigned int		shstrndx;
+static unsigned int		shsymtabndx;
+static unsigned int		shxsymtabndx;
 
 static int sym_index(Elf_Sym *sym);
 
 struct relocs {
-	uint32_t	*offset;
-	unsigned long	count;
-	unsigned long	size;
+				uint32_t	*offset;
+				unsigned long	count;
+				unsigned long	size;
 };
 
-static struct relocs relocs16;
-static struct relocs relocs32;
+static struct relocs		relocs16;
+static struct relocs		relocs32;
+
 #if ELF_BITS == 64
-static struct relocs relocs32neg;
-static struct relocs relocs64;
-#define FMT PRIu64
+static struct relocs		relocs32neg;
+static struct relocs		relocs64;
+# define FMT PRIu64
 #else
-#define FMT PRIu32
+# define FMT PRIu32
 #endif
 
 struct section {
-	Elf_Shdr       shdr;
-	struct section *link;
-	Elf_Sym        *symtab;
-	Elf32_Word     *xsymtab;
-	Elf_Rel        *reltab;
-	char           *strtab;
+				Elf_Shdr       shdr;
+				struct section *link;
+				Elf_Sym        *symtab;
+				Elf32_Word     *xsymtab;
+				Elf_Rel        *reltab;
+				char           *strtab;
 };
-static struct section *secs;
+static struct section		*secs;
 
-static const char * const sym_regex_kernel[S_NSYMTYPES] = {
+static const char * const	sym_regex_kernel[S_NSYMTYPES] = {
 /*
  * Following symbols have been audited. There values are constant and do
  * not change if bzImage is loaded at a different physical address than
@@ -115,13 +116,13 @@ static const char * const sym_regex_realmode[S_NSYMTYPES] = {
 	"^pa_",
 };
 
-static const char * const *sym_regex;
+static const char * const	*sym_regex;
+
+static regex_t			sym_regex_c[S_NSYMTYPES];
 
-static regex_t sym_regex_c[S_NSYMTYPES];
 static int is_reloc(enum symtype type, const char *sym_name)
 {
-	return sym_regex[type] &&
-		!regexec(&sym_regex_c[type], sym_name, 0, NULL, 0);
+	return sym_regex[type] && !regexec(&sym_regex_c[type], sym_name, 0, NULL, 0);
 }
 
 static void regex_init(int use_real_mode)
@@ -139,8 +140,7 @@ static void regex_init(int use_real_mode)
 		if (!sym_regex[i])
 			continue;
 
-		err = regcomp(&sym_regex_c[i], sym_regex[i],
-			      REG_EXTENDED|REG_NOSUB);
+		err = regcomp(&sym_regex_c[i], sym_regex[i], REG_EXTENDED|REG_NOSUB);
 
 		if (err) {
 			regerror(err, &sym_regex_c[i], errbuf, sizeof(errbuf));
@@ -163,9 +163,10 @@ static const char *sym_type(unsigned type)
 #undef SYM_TYPE
 	};
 	const char *name = "unknown sym type name";
-	if (type < ARRAY_SIZE(type_name)) {
+
+	if (type < ARRAY_SIZE(type_name))
 		name = type_name[type];
-	}
+
 	return name;
 }
 
@@ -179,9 +180,10 @@ static const char *sym_bind(unsigned bind)
 #undef SYM_BIND
 	};
 	const char *name = "unknown sym bind name";
-	if (bind < ARRAY_SIZE(bind_name)) {
+
+	if (bind < ARRAY_SIZE(bind_name))
 		name = bind_name[bind];
-	}
+
 	return name;
 }
 
@@ -196,9 +198,10 @@ static const char *sym_visibility(unsigned visibility)
 #undef SYM_VISIBILITY
 	};
 	const char *name = "unknown sym visibility name";
-	if (visibility < ARRAY_SIZE(visibility_name)) {
+
+	if (visibility < ARRAY_SIZE(visibility_name))
 		name = visibility_name[visibility];
-	}
+
 	return name;
 }
 
@@ -244,9 +247,10 @@ static const char *rel_type(unsigned type)
 #undef REL_TYPE
 	};
 	const char *name = "unknown type rel type name";
-	if (type < ARRAY_SIZE(type_name) && type_name[type]) {
+
+	if (type < ARRAY_SIZE(type_name) && type_name[type])
 		name = type_name[type];
-	}
+
 	return name;
 }
 
@@ -256,15 +260,14 @@ static const char *sec_name(unsigned shndx)
 	const char *name;
 	sec_strtab = secs[shstrndx].strtab;
 	name = "<noname>";
-	if (shndx < shnum) {
+
+	if (shndx < shnum)
 		name = sec_strtab + secs[shndx].shdr.sh_name;
-	}
-	else if (shndx == SHN_ABS) {
+	else if (shndx == SHN_ABS)
 		name = "ABSOLUTE";
-	}
-	else if (shndx == SHN_COMMON) {
+	else if (shndx == SHN_COMMON)
 		name = "COMMON";
-	}
+
 	return name;
 }
 
@@ -272,18 +275,19 @@ static const char *sym_name(const char *sym_strtab, Elf_Sym *sym)
 {
 	const char *name;
 	name = "<noname>";
-	if (sym->st_name) {
+
+	if (sym->st_name)
 		name = sym_strtab + sym->st_name;
-	}
-	else {
+	else
 		name = sec_name(sym_index(sym));
-	}
+
 	return name;
 }
 
 static Elf_Sym *sym_lookup(const char *symname)
 {
 	int i;
+
 	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		long nsyms;
@@ -309,14 +313,15 @@ static Elf_Sym *sym_lookup(const char *symname)
 }
 
 #if BYTE_ORDER == LITTLE_ENDIAN
-#define le16_to_cpu(val) (val)
-#define le32_to_cpu(val) (val)
-#define le64_to_cpu(val) (val)
+# define le16_to_cpu(val)	(val)
+# define le32_to_cpu(val)	(val)
+# define le64_to_cpu(val)	(val)
 #endif
+
 #if BYTE_ORDER == BIG_ENDIAN
-#define le16_to_cpu(val) bswap_16(val)
-#define le32_to_cpu(val) bswap_32(val)
-#define le64_to_cpu(val) bswap_64(val)
+# define le16_to_cpu(val)	bswap_16(val)
+# define le32_to_cpu(val)	bswap_32(val)
+# define le64_to_cpu(val)	bswap_64(val)
 #endif
 
 static uint16_t elf16_to_cpu(uint16_t val)
@@ -337,13 +342,13 @@ static uint64_t elf64_to_cpu(uint64_t val)
 {
         return le64_to_cpu(val);
 }
-#define elf_addr_to_cpu(x)	elf64_to_cpu(x)
-#define elf_off_to_cpu(x)	elf64_to_cpu(x)
-#define elf_xword_to_cpu(x)	elf64_to_cpu(x)
+# define elf_addr_to_cpu(x)	elf64_to_cpu(x)
+# define elf_off_to_cpu(x)	elf64_to_cpu(x)
+# define elf_xword_to_cpu(x)	elf64_to_cpu(x)
 #else
-#define elf_addr_to_cpu(x)	elf32_to_cpu(x)
-#define elf_off_to_cpu(x)	elf32_to_cpu(x)
-#define elf_xword_to_cpu(x)	elf32_to_cpu(x)
+# define elf_addr_to_cpu(x)	elf32_to_cpu(x)
+# define elf_off_to_cpu(x)	elf32_to_cpu(x)
+# define elf_xword_to_cpu(x)	elf32_to_cpu(x)
 #endif
 
 static int sym_index(Elf_Sym *sym)
@@ -365,22 +370,17 @@ static int sym_index(Elf_Sym *sym)
 
 static void read_ehdr(FILE *fp)
 {
-	if (fread(&ehdr, sizeof(ehdr), 1, fp) != 1) {
-		die("Cannot read ELF header: %s\n",
-			strerror(errno));
-	}
-	if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) != 0) {
+	if (fread(&ehdr, sizeof(ehdr), 1, fp) != 1)
+		die("Cannot read ELF header: %s\n", strerror(errno));
+	if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) != 0)
 		die("No ELF magic\n");
-	}
-	if (ehdr.e_ident[EI_CLASS] != ELF_CLASS) {
+	if (ehdr.e_ident[EI_CLASS] != ELF_CLASS)
 		die("Not a %d bit executable\n", ELF_BITS);
-	}
-	if (ehdr.e_ident[EI_DATA] != ELFDATA2LSB) {
+	if (ehdr.e_ident[EI_DATA] != ELFDATA2LSB)
 		die("Not a LSB ELF executable\n");
-	}
-	if (ehdr.e_ident[EI_VERSION] != EV_CURRENT) {
+	if (ehdr.e_ident[EI_VERSION] != EV_CURRENT)
 		die("Unknown ELF version\n");
-	}
+
 	/* Convert the fields to native endian */
 	ehdr.e_type      = elf_half_to_cpu(ehdr.e_type);
 	ehdr.e_machine   = elf_half_to_cpu(ehdr.e_machine);
@@ -439,19 +439,18 @@ static void read_shdrs(FILE *fp)
 	Elf_Shdr shdr;
 
 	secs = calloc(shnum, sizeof(struct section));
-	if (!secs) {
-		die("Unable to allocate %ld section headers\n",
-		    shnum);
-	}
-	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
-		die("Seek to %" FMT " failed: %s\n",
-		    ehdr.e_shoff, strerror(errno));
-	}
+	if (!secs)
+		die("Unable to allocate %ld section headers\n", shnum);
+
+	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
+		die("Seek to %" FMT " failed: %s\n", ehdr.e_shoff, strerror(errno));
+
 	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
+
 		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
-			die("Cannot read ELF section headers %d/%ld: %s\n",
-			    i, shnum, strerror(errno));
+			die("Cannot read ELF section headers %d/%ld: %s\n", i, shnum, strerror(errno));
+
 		sec->shdr.sh_name      = elf_word_to_cpu(shdr.sh_name);
 		sec->shdr.sh_type      = elf_word_to_cpu(shdr.sh_type);
 		sec->shdr.sh_flags     = elf_xword_to_cpu(shdr.sh_flags);
@@ -471,31 +470,28 @@ static void read_shdrs(FILE *fp)
 static void read_strtabs(FILE *fp)
 {
 	int i;
+
 	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
-		if (sec->shdr.sh_type != SHT_STRTAB) {
+
+		if (sec->shdr.sh_type != SHT_STRTAB)
 			continue;
-		}
+
 		sec->strtab = malloc(sec->shdr.sh_size);
-		if (!sec->strtab) {
-			die("malloc of %" FMT " bytes for strtab failed\n",
-			    sec->shdr.sh_size);
-		}
-		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %" FMT " failed: %s\n",
-			    sec->shdr.sh_offset, strerror(errno));
-		}
-		if (fread(sec->strtab, 1, sec->shdr.sh_size, fp)
-		    != sec->shdr.sh_size) {
-			die("Cannot read symbol table: %s\n",
-				strerror(errno));
-		}
+		if (!sec->strtab)
+			die("malloc of %" FMT " bytes for strtab failed\n", sec->shdr.sh_size);
+
+		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0)
+			die("Seek to %" FMT " failed: %s\n", sec->shdr.sh_offset, strerror(errno));
+
+		if (fread(sec->strtab, 1, sec->shdr.sh_size, fp) != sec->shdr.sh_size)
+			die("Cannot read symbol table: %s\n", strerror(errno));
 	}
 }
 
 static void read_symtabs(FILE *fp)
 {
-	int i,j;
+	int i, j;
 
 	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
@@ -504,19 +500,15 @@ static void read_symtabs(FILE *fp)
 		switch (sec->shdr.sh_type) {
 		case SHT_SYMTAB_SHNDX:
 			sec->xsymtab = malloc(sec->shdr.sh_size);
-			if (!sec->xsymtab) {
-				die("malloc of %" FMT " bytes for xsymtab failed\n",
-				    sec->shdr.sh_size);
-			}
-			if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-				die("Seek to %" FMT " failed: %s\n",
-				    sec->shdr.sh_offset, strerror(errno));
-			}
-			if (fread(sec->xsymtab, 1, sec->shdr.sh_size, fp)
-			    != sec->shdr.sh_size) {
-				die("Cannot read extended symbol table: %s\n",
-				    strerror(errno));
-			}
+			if (!sec->xsymtab)
+				die("malloc of %" FMT " bytes for xsymtab failed\n", sec->shdr.sh_size);
+
+			if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0)
+				die("Seek to %" FMT " failed: %s\n", sec->shdr.sh_offset, strerror(errno));
+
+			if (fread(sec->xsymtab, 1, sec->shdr.sh_size, fp) != sec->shdr.sh_size)
+				die("Cannot read extended symbol table: %s\n", strerror(errno));
+
 			shxsymtabndx = i;
 			continue;
 
@@ -524,19 +516,15 @@ static void read_symtabs(FILE *fp)
 			num_syms = sec->shdr.sh_size / sizeof(Elf_Sym);
 
 			sec->symtab = malloc(sec->shdr.sh_size);
-			if (!sec->symtab) {
-				die("malloc of %" FMT " bytes for symtab failed\n",
-				    sec->shdr.sh_size);
-			}
-			if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-				die("Seek to %" FMT " failed: %s\n",
-				    sec->shdr.sh_offset, strerror(errno));
-			}
-			if (fread(sec->symtab, 1, sec->shdr.sh_size, fp)
-			    != sec->shdr.sh_size) {
-				die("Cannot read symbol table: %s\n",
-				    strerror(errno));
-			}
+			if (!sec->symtab)
+				die("malloc of %" FMT " bytes for symtab failed\n", sec->shdr.sh_size);
+
+			if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0)
+				die("Seek to %" FMT " failed: %s\n", sec->shdr.sh_offset, strerror(errno));
+
+			if (fread(sec->symtab, 1, sec->shdr.sh_size, fp) != sec->shdr.sh_size)
+				die("Cannot read symbol table: %s\n", strerror(errno));
+
 			for (j = 0; j < num_syms; j++) {
 				Elf_Sym *sym = &sec->symtab[j];
 
@@ -557,28 +545,27 @@ static void read_symtabs(FILE *fp)
 
 static void read_relocs(FILE *fp)
 {
-	int i,j;
+	int i, j;
+
 	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
-		if (sec->shdr.sh_type != SHT_REL_TYPE) {
+
+		if (sec->shdr.sh_type != SHT_REL_TYPE)
 			continue;
-		}
+
 		sec->reltab = malloc(sec->shdr.sh_size);
-		if (!sec->reltab) {
-			die("malloc of %" FMT " bytes for relocs failed\n",
-			    sec->shdr.sh_size);
-		}
-		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %" FMT " failed: %s\n",
-			    sec->shdr.sh_offset, strerror(errno));
-		}
-		if (fread(sec->reltab, 1, sec->shdr.sh_size, fp)
-		    != sec->shdr.sh_size) {
-			die("Cannot read symbol table: %s\n",
-				strerror(errno));
-		}
+		if (!sec->reltab)
+			die("malloc of %" FMT " bytes for relocs failed\n", sec->shdr.sh_size);
+
+		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0)
+			die("Seek to %" FMT " failed: %s\n", sec->shdr.sh_offset, strerror(errno));
+
+		if (fread(sec->reltab, 1, sec->shdr.sh_size, fp) != sec->shdr.sh_size)
+			die("Cannot read symbol table: %s\n", strerror(errno));
+
 		for (j = 0; j < sec->shdr.sh_size/sizeof(Elf_Rel); j++) {
 			Elf_Rel *rel = &sec->reltab[j];
+
 			rel->r_offset = elf_addr_to_cpu(rel->r_offset);
 			rel->r_info   = elf_xword_to_cpu(rel->r_info);
 #if (SHT_REL_TYPE == SHT_RELA)
@@ -601,23 +588,27 @@ static void print_absolute_symbols(void)
 
 	printf("Absolute symbols\n");
 	printf(" Num:    Value Size  Type       Bind        Visibility  Name\n");
+
 	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		char *sym_strtab;
 		int j;
 
-		if (sec->shdr.sh_type != SHT_SYMTAB) {
+		if (sec->shdr.sh_type != SHT_SYMTAB)
 			continue;
-		}
+
 		sym_strtab = sec->link->strtab;
+
 		for (j = 0; j < sec->shdr.sh_size/sizeof(Elf_Sym); j++) {
 			Elf_Sym *sym;
 			const char *name;
+
 			sym = &sec->symtab[j];
 			name = sym_name(sym_strtab, sym);
-			if (sym->st_shndx != SHN_ABS) {
+
+			if (sym->st_shndx != SHN_ABS)
 				continue;
-			}
+
 			printf(format,
 				j, sym->st_value, sym->st_size,
 				sym_type(ELF_ST_TYPE(sym->st_info)),
@@ -645,34 +636,37 @@ static void print_absolute_relocs(void)
 		char *sym_strtab;
 		Elf_Sym *sh_symtab;
 		int j;
-		if (sec->shdr.sh_type != SHT_REL_TYPE) {
+
+		if (sec->shdr.sh_type != SHT_REL_TYPE)
 			continue;
-		}
+
 		sec_symtab  = sec->link;
 		sec_applies = &secs[sec->shdr.sh_info];
-		if (!(sec_applies->shdr.sh_flags & SHF_ALLOC)) {
+		if (!(sec_applies->shdr.sh_flags & SHF_ALLOC))
 			continue;
-		}
+
 		/*
 		 * Do not perform relocations in .notes section; any
 		 * values there are meant for pre-boot consumption (e.g.
 		 * startup_xen).
 		 */
-		if (sec_applies->shdr.sh_type == SHT_NOTE) {
+		if (sec_applies->shdr.sh_type == SHT_NOTE)
 			continue;
-		}
+
 		sh_symtab  = sec_symtab->symtab;
 		sym_strtab = sec_symtab->link->strtab;
+
 		for (j = 0; j < sec->shdr.sh_size/sizeof(Elf_Rel); j++) {
 			Elf_Rel *rel;
 			Elf_Sym *sym;
 			const char *name;
+
 			rel = &sec->reltab[j];
 			sym = &sh_symtab[ELF_R_SYM(rel->r_info)];
 			name = sym_name(sym_strtab, sym);
-			if (sym->st_shndx != SHN_ABS) {
+
+			if (sym->st_shndx != SHN_ABS)
 				continue;
-			}
 
 			/* Absolute symbols are not relocated if bzImage is
 			 * loaded at a non-compiled address. Display a warning
@@ -691,10 +685,8 @@ static void print_absolute_relocs(void)
 				continue;
 
 			if (!printed) {
-				printf("WARNING: Absolute relocations"
-					" present\n");
-				printf("Offset     Info     Type     Sym.Value "
-					"Sym.Name\n");
+				printf("WARNING: Absolute relocations present\n");
+				printf("Offset     Info     Type     Sym.Value Sym.Name\n");
 				printed = 1;
 			}
 
@@ -718,8 +710,8 @@ static void add_reloc(struct relocs *r, uint32_t offset)
 		void *mem = realloc(r->offset, newsize * sizeof(r->offset[0]));
 
 		if (!mem)
-			die("realloc of %ld entries for relocs failed\n",
-                                newsize);
+			die("realloc of %ld entries for relocs failed\n", newsize);
+
 		r->offset = mem;
 		r->size = newsize;
 	}
@@ -730,6 +722,7 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel,
 			Elf_Sym *sym, const char *symname))
 {
 	int i;
+
 	/* Walk through the relocations */
 	for (i = 0; i < shnum; i++) {
 		char *sym_strtab;
@@ -738,14 +731,13 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel,
 		int j;
 		struct section *sec = &secs[i];
 
-		if (sec->shdr.sh_type != SHT_REL_TYPE) {
+		if (sec->shdr.sh_type != SHT_REL_TYPE)
 			continue;
-		}
+
 		sec_symtab  = sec->link;
 		sec_applies = &secs[sec->shdr.sh_info];
-		if (!(sec_applies->shdr.sh_flags & SHF_ALLOC)) {
+		if (!(sec_applies->shdr.sh_flags & SHF_ALLOC))
 			continue;
-		}
 
 		/*
 		 * Do not perform relocations in .notes sections; any
@@ -757,6 +749,7 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel,
 
 		sh_symtab = sec_symtab->symtab;
 		sym_strtab = sec_symtab->link->strtab;
+
 		for (j = 0; j < sec->shdr.sh_size/sizeof(Elf_Rel); j++) {
 			Elf_Rel *rel = &sec->reltab[j];
 			Elf_Sym *sym = &sh_symtab[ELF_R_SYM(rel->r_info)];
@@ -790,14 +783,16 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel,
  * kernel data and does not require special treatment.
  *
  */
-static int per_cpu_shndx	= -1;
+static int per_cpu_shndx = -1;
 static Elf_Addr per_cpu_load_addr;
 
 static void percpu_init(void)
 {
 	int i;
+
 	for (i = 0; i < shnum; i++) {
 		ElfW(Sym) *sym;
+
 		if (strcmp(sec_name(i), ".data..percpu"))
 			continue;
 
@@ -810,6 +805,7 @@ static void percpu_init(void)
 
 		per_cpu_shndx = i;
 		per_cpu_load_addr = sym->st_value;
+
 		return;
 	}
 }
@@ -880,8 +876,7 @@ static int do_reloc64(struct section *sec, Elf_Rel *rel, ElfW(Sym) *sym,
 		 * Only used by jump labels
 		 */
 		if (is_percpu_sym(sym, symname))
-			die("Invalid R_X86_64_PC64 relocation against per-CPU symbol %s\n",
-			    symname);
+			die("Invalid R_X86_64_PC64 relocation against per-CPU symbol %s\n", symname);
 		break;
 
 	case R_X86_64_32:
@@ -901,8 +896,7 @@ static int do_reloc64(struct section *sec, Elf_Rel *rel, ElfW(Sym) *sym,
 			if (is_reloc(S_ABS, symname))
 				break;
 
-			die("Invalid absolute %s relocation: %s\n",
-			    rel_type(r_type), symname);
+			die("Invalid absolute %s relocation: %s\n", rel_type(r_type), symname);
 			break;
 		}
 
@@ -922,8 +916,7 @@ static int do_reloc64(struct section *sec, Elf_Rel *rel, ElfW(Sym) *sym,
 		break;
 
 	default:
-		die("Unsupported relocation type: %s (%d)\n",
-		    rel_type(r_type), r_type);
+		die("Unsupported relocation type: %s (%d)\n", rel_type(r_type), r_type);
 		break;
 	}
 
@@ -960,8 +953,7 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 			if (is_reloc(S_ABS, symname))
 				break;
 
-			die("Invalid absolute %s relocation: %s\n",
-			    rel_type(r_type), symname);
+			die("Invalid absolute %s relocation: %s\n", rel_type(r_type), symname);
 			break;
 		}
 
@@ -969,16 +961,14 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 		break;
 
 	default:
-		die("Unsupported relocation type: %s (%d)\n",
-		    rel_type(r_type), r_type);
+		die("Unsupported relocation type: %s (%d)\n", rel_type(r_type), r_type);
 		break;
 	}
 
 	return 0;
 }
 
-static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
-			 const char *symname)
+static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym, const char *symname)
 {
 	unsigned r_type = ELF32_R_TYPE(rel->r_info);
 	int shn_abs = (sym->st_shndx == SHN_ABS) && !is_reloc(S_REL, symname);
@@ -1013,9 +1003,7 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 			if (!is_reloc(S_LIN, symname))
 				break;
 		}
-		die("Invalid %s %s relocation: %s\n",
-		    shn_abs ? "absolute" : "relative",
-		    rel_type(r_type), symname);
+		die("Invalid %s %s relocation: %s\n", shn_abs ? "absolute" : "relative", rel_type(r_type), symname);
 		break;
 
 	case R_386_32:
@@ -1036,14 +1024,11 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 				add_reloc(&relocs32, rel->r_offset);
 			break;
 		}
-		die("Invalid %s %s relocation: %s\n",
-		    shn_abs ? "absolute" : "relative",
-		    rel_type(r_type), symname);
+		die("Invalid %s %s relocation: %s\n", shn_abs ? "absolute" : "relative", rel_type(r_type), symname);
 		break;
 
 	default:
-		die("Unsupported relocation type: %s (%d)\n",
-		    rel_type(r_type), r_type);
+		die("Unsupported relocation type: %s (%d)\n", rel_type(r_type), r_type);
 		break;
 	}
 
@@ -1055,7 +1040,10 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 static int cmp_relocs(const void *va, const void *vb)
 {
 	const uint32_t *a, *b;
-	a = va; b = vb;
+
+	a = va;
+	b = vb;
+
 	return (*a == *b)? 0 : (*a > *b)? 1 : -1;
 }
 
@@ -1069,6 +1057,7 @@ static int write32(uint32_t v, FILE *f)
 	unsigned char buf[4];
 
 	put_unaligned_le32(v, buf);
+
 	return fwrite(buf, 1, 4, f) == 4 ? 0 : -1;
 }
 
@@ -1081,8 +1070,7 @@ static void emit_relocs(int as_text, int use_real_mode)
 {
 	int i;
 	int (*write_reloc)(uint32_t, FILE *) = write32;
-	int (*do_reloc)(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
-			const char *symname);
+	int (*do_reloc)(struct section *sec, Elf_Rel *rel, Elf_Sym *sym, const char *symname);
 
 #if ELF_BITS == 64
 	if (!use_real_mode)
@@ -1169,6 +1157,7 @@ static int do_reloc_info(struct section *sec, Elf_Rel *rel, ElfW(Sym) *sym,
 		rel_type(ELF_R_TYPE(rel->r_info)),
 		symname,
 		sec_name(sym_index(sym)));
+
 	return 0;
 }
 
@@ -1194,19 +1183,24 @@ void process(FILE *fp, int use_real_mode, int as_text,
 	read_strtabs(fp);
 	read_symtabs(fp);
 	read_relocs(fp);
+
 	if (ELF_BITS == 64)
 		percpu_init();
+
 	if (show_absolute_syms) {
 		print_absolute_symbols();
 		return;
 	}
+
 	if (show_absolute_relocs) {
 		print_absolute_relocs();
 		return;
 	}
+
 	if (show_reloc_info) {
 		print_reloc_info();
 		return;
 	}
+
 	emit_relocs(as_text, use_real_mode);
 }

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs
  2024-03-23 10:38               ` Borislav Petkov
@ 2024-03-25 20:23                 ` Kees Cook
  0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2024-03-25 20:23 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: tglx, Guixiong Wei, jgross, mingo, dave.hansen, x86, hpa, peterz,
	gregkh, tony.luck, adobriyan, linux-kernel, linux-hardening

On Sat, Mar 23, 2024 at 11:38:27AM +0100, Borislav Petkov wrote:
> On Fri, Mar 22, 2024 at 04:40:11PM -0700, Kees Cook wrote:
> > The earlier patch, commit aaa8736370db ("x86, relocs: Ignore relocations
> > in .notes section"), landed via my tree. It was sent out on Feb 22nd
> > (v1[1]) and got a suggestion from HPA and a Review from Juergen Gross.
> > I sent v2 Feb 27th[2] and it sat ignored for two weeks.
> 
> s/ignored for two weeks/missed in the avalance of patches/
> 
> > Since it was a 10 year old kernel address exposure, I sent it to Linus
> > on Mar 12th[3].
> 
> So is there some unwritten understanding somewhere which says that you
> should take tip patches through your tree?
> 
> Maybe I've missed it.
> 
> If there isn't, should we agree on something?
> 
> Because there clearly is a need for clarification here...

Yeah, happy to figure this out. How should I handle x86 patches that
maintainers haven't responded to when they have security bug fix
implications? For all the security hardening stuff I usually just ping
every few weeks, but those don't usually tend to be urgent.

In this case, I felt like since it was a trivial fix, HPA had already
implied it was a sensible change, and Juergen had reviewed it, it seemed
like it wouldn't be disruptive to take it, given the timing of the merge
window, etc.

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-03-25 20:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-17 15:05 [PATCH] x86, relocs: Ignore relocations in .notes section on walk_relocs Guixiong Wei
2024-03-18 21:40 ` Kees Cook
2024-03-18 21:56   ` Borislav Petkov
2024-03-18 23:45     ` Kees Cook
2024-03-19  8:16       ` Borislav Petkov
2024-03-19 16:56         ` Kees Cook
2024-03-22 19:46           ` Borislav Petkov
2024-03-22 23:40             ` Kees Cook
2024-03-23 10:38               ` Borislav Petkov
2024-03-25 20:23                 ` Kees Cook
2024-03-22  8:45   ` Ingo Molnar
2024-03-22 23:41     ` Kees Cook
2024-03-24  3:57       ` [PATCH] x86/build: Clean up arch/x86/tools/relocs.c a bit Ingo Molnar
2024-03-22  8:56 ` [tip: x86/boot] x86/boot: Ignore relocations in .notes sections in walk_relocs() too tip-bot2 for Guixiong Wei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).