| Message ID | 20240317150547.24910-1-weiguixiong@bytedance.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 76e9762d66373354b45c33b60e9a53ef2a3c5ff2 |
| Headers | show |
| Series | x86, relocs: Ignore relocations in .notes section on walk_relocs | expand |
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,
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.
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?
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.
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 <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;
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++) {
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/
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
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.
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.
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++) {
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(+)