From: Eric W. Biederman <ebiederm@xmission.com>
To: kexec@lists.infradead.org
Subject: [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add]
Date: Wed, 18 May 2022 16:59:37 -0500 [thread overview]
Message-ID: <87ee0q7b92.fsf@email.froward.int.ebiederm.org> (raw)
In-Reply-To: <20220518181828.645877-1-naveen.n.rao@linux.vnet.ibm.com> (Naveen N. Rao's message of "Wed, 18 May 2022 23:48:28 +0530")
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> Since commit d1bcae833b32f1 ("ELF: Don't generate unused section
> symbols") [1], binutils (v2.36+) started dropping section symbols that
> it thought were unused. This isn't an issue in general, but with
> kexec_file.c, gcc is placing kexec_arch_apply_relocations[_add] into a
> separate .text.unlikely section and the section symbol ".text.unlikely"
> is being dropped. Due to this, recordmcount is unable to find a non-weak
> symbol in .text.unlikely to generate a relocation record against.
>
> Address this by dropping the weak attribute from these functions:
> - arch_kexec_apply_relocations() is not overridden by any architecture
> today, so just drop the weak attribute.
> - arch_kexec_apply_relocations_add() is only overridden by x86 and s390.
> Retain the function prototype for those and move the weak
> implementation into the header as a static inline for other
> architectures.
>
> [1] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1
Any chance you can also get machine_kexec_post_load,
crash_free_reserved_phys_range, arch_kexec_protect_protect_crashkres,
arch_kexec_unprotect_crashkres, arch_kexec_kernel_image_probe,
arch_kexec_kernel_image_probe, arch_kimage_file_post_load_cleanup,
arch_kexec_kernel_verify_sig, and arch_kexec_locate_mem_hole as well.
That is everything in kexec that uses a __weak symbol. If we can't
count on them working we might as well just get rid of the rest
preemptively.
Could you also address Andrews concerns by using a Kconfig symbol that
the architectures that implement the symbol can select.
I don't want to ask too much of a volunteer but if you are willing
addressing both of those would be a great help.
Eric
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> include/linux/kexec.h | 28 ++++++++++++++++++++++++----
> kernel/kexec_file.c | 19 +------------------
> 2 files changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 58d1b58a971e34..e656f981f43a73 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -193,10 +193,6 @@ void *kexec_purgatory_get_symbol_addr(struct kimage *image, const char *name);
> int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> unsigned long buf_len);
> void *arch_kexec_kernel_image_load(struct kimage *image);
> -int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> - Elf_Shdr *section,
> - const Elf_Shdr *relsec,
> - const Elf_Shdr *symtab);
> int arch_kexec_apply_relocations(struct purgatory_info *pi,
> Elf_Shdr *section,
> const Elf_Shdr *relsec,
> @@ -229,6 +225,30 @@ extern int crash_exclude_mem_range(struct crash_mem *mem,
> unsigned long long mend);
> extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
> void **addr, unsigned long *sz);
> +
> +#if defined(CONFIG_X86_64) || defined(CONFIG_S390)
> +int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> + Elf_Shdr *section,
> + const Elf_Shdr *relsec,
> + const Elf_Shdr *symtab);
> +#else
> +/*
> + * arch_kexec_apply_relocations_add - apply relocations of type RELA
> + * @pi: Purgatory to be relocated.
> + * @section: Section relocations applying to.
> + * @relsec: Section containing RELAs.
> + * @symtab: Corresponding symtab.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +static inline int
> +arch_kexec_apply_relocations_add(struct purgatory_info *pi, Elf_Shdr *section,
> + const Elf_Shdr *relsec, const Elf_Shdr *symtab)
> +{
> + pr_err("RELA relocation unsupported.\n");
> + return -ENOEXEC;
> +}
> +#endif /* CONFIG_X86_64 || CONFIG_S390 */
> #endif /* CONFIG_KEXEC_FILE */
>
> #ifdef CONFIG_KEXEC_ELF
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 8347fc158d2b96..6bae253b4d315e 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -108,23 +108,6 @@ int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> }
> #endif
>
> -/*
> - * arch_kexec_apply_relocations_add - apply relocations of type RELA
> - * @pi: Purgatory to be relocated.
> - * @section: Section relocations applying to.
> - * @relsec: Section containing RELAs.
> - * @symtab: Corresponding symtab.
> - *
> - * Return: 0 on success, negative errno on error.
> - */
> -int __weak
> -arch_kexec_apply_relocations_add(struct purgatory_info *pi, Elf_Shdr *section,
> - const Elf_Shdr *relsec, const Elf_Shdr *symtab)
> -{
> - pr_err("RELA relocation unsupported.\n");
> - return -ENOEXEC;
> -}
> -
> /*
> * arch_kexec_apply_relocations - apply relocations of type REL
> * @pi: Purgatory to be relocated.
> @@ -134,7 +117,7 @@ arch_kexec_apply_relocations_add(struct purgatory_info *pi, Elf_Shdr *section,
> *
> * Return: 0 on success, negative errno on error.
> */
> -int __weak
> +int
> arch_kexec_apply_relocations(struct purgatory_info *pi, Elf_Shdr *section,
> const Elf_Shdr *relsec, const Elf_Shdr *symtab)
> {
>
> base-commit: ef1302160bfb19f804451d0e919266703501c875
WARNING: multiple messages have this Message-ID (diff)
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
kexec@lists.infradead.org
Subject: Re: [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add]
Date: Wed, 18 May 2022 16:59:37 -0500 [thread overview]
Message-ID: <87ee0q7b92.fsf@email.froward.int.ebiederm.org> (raw)
In-Reply-To: <20220518181828.645877-1-naveen.n.rao@linux.vnet.ibm.com> (Naveen N. Rao's message of "Wed, 18 May 2022 23:48:28 +0530")
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> Since commit d1bcae833b32f1 ("ELF: Don't generate unused section
> symbols") [1], binutils (v2.36+) started dropping section symbols that
> it thought were unused. This isn't an issue in general, but with
> kexec_file.c, gcc is placing kexec_arch_apply_relocations[_add] into a
> separate .text.unlikely section and the section symbol ".text.unlikely"
> is being dropped. Due to this, recordmcount is unable to find a non-weak
> symbol in .text.unlikely to generate a relocation record against.
>
> Address this by dropping the weak attribute from these functions:
> - arch_kexec_apply_relocations() is not overridden by any architecture
> today, so just drop the weak attribute.
> - arch_kexec_apply_relocations_add() is only overridden by x86 and s390.
> Retain the function prototype for those and move the weak
> implementation into the header as a static inline for other
> architectures.
>
> [1] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1
Any chance you can also get machine_kexec_post_load,
crash_free_reserved_phys_range, arch_kexec_protect_protect_crashkres,
arch_kexec_unprotect_crashkres, arch_kexec_kernel_image_probe,
arch_kexec_kernel_image_probe, arch_kimage_file_post_load_cleanup,
arch_kexec_kernel_verify_sig, and arch_kexec_locate_mem_hole as well.
That is everything in kexec that uses a __weak symbol. If we can't
count on them working we might as well just get rid of the rest
preemptively.
Could you also address Andrews concerns by using a Kconfig symbol that
the architectures that implement the symbol can select.
I don't want to ask too much of a volunteer but if you are willing
addressing both of those would be a great help.
Eric
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> include/linux/kexec.h | 28 ++++++++++++++++++++++++----
> kernel/kexec_file.c | 19 +------------------
> 2 files changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 58d1b58a971e34..e656f981f43a73 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -193,10 +193,6 @@ void *kexec_purgatory_get_symbol_addr(struct kimage *image, const char *name);
> int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> unsigned long buf_len);
> void *arch_kexec_kernel_image_load(struct kimage *image);
> -int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> - Elf_Shdr *section,
> - const Elf_Shdr *relsec,
> - const Elf_Shdr *symtab);
> int arch_kexec_apply_relocations(struct purgatory_info *pi,
> Elf_Shdr *section,
> const Elf_Shdr *relsec,
> @@ -229,6 +225,30 @@ extern int crash_exclude_mem_range(struct crash_mem *mem,
> unsigned long long mend);
> extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
> void **addr, unsigned long *sz);
> +
> +#if defined(CONFIG_X86_64) || defined(CONFIG_S390)
> +int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> + Elf_Shdr *section,
> + const Elf_Shdr *relsec,
> + const Elf_Shdr *symtab);
> +#else
> +/*
> + * arch_kexec_apply_relocations_add - apply relocations of type RELA
> + * @pi: Purgatory to be relocated.
> + * @section: Section relocations applying to.
> + * @relsec: Section containing RELAs.
> + * @symtab: Corresponding symtab.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +static inline int
> +arch_kexec_apply_relocations_add(struct purgatory_info *pi, Elf_Shdr *section,
> + const Elf_Shdr *relsec, const Elf_Shdr *symtab)
> +{
> + pr_err("RELA relocation unsupported.\n");
> + return -ENOEXEC;
> +}
> +#endif /* CONFIG_X86_64 || CONFIG_S390 */
> #endif /* CONFIG_KEXEC_FILE */
>
> #ifdef CONFIG_KEXEC_ELF
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 8347fc158d2b96..6bae253b4d315e 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -108,23 +108,6 @@ int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> }
> #endif
>
> -/*
> - * arch_kexec_apply_relocations_add - apply relocations of type RELA
> - * @pi: Purgatory to be relocated.
> - * @section: Section relocations applying to.
> - * @relsec: Section containing RELAs.
> - * @symtab: Corresponding symtab.
> - *
> - * Return: 0 on success, negative errno on error.
> - */
> -int __weak
> -arch_kexec_apply_relocations_add(struct purgatory_info *pi, Elf_Shdr *section,
> - const Elf_Shdr *relsec, const Elf_Shdr *symtab)
> -{
> - pr_err("RELA relocation unsupported.\n");
> - return -ENOEXEC;
> -}
> -
> /*
> * arch_kexec_apply_relocations - apply relocations of type REL
> * @pi: Purgatory to be relocated.
> @@ -134,7 +117,7 @@ arch_kexec_apply_relocations_add(struct purgatory_info *pi, Elf_Shdr *section,
> *
> * Return: 0 on success, negative errno on error.
> */
> -int __weak
> +int
> arch_kexec_apply_relocations(struct purgatory_info *pi, Elf_Shdr *section,
> const Elf_Shdr *relsec, const Elf_Shdr *symtab)
> {
>
> base-commit: ef1302160bfb19f804451d0e919266703501c875
WARNING: multiple messages have this Message-ID (diff)
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
<linuxppc-dev@lists.ozlabs.org>, <linux-kernel@vger.kernel.org>,
kexec@lists.infradead.org,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add]
Date: Wed, 18 May 2022 16:59:37 -0500 [thread overview]
Message-ID: <87ee0q7b92.fsf@email.froward.int.ebiederm.org> (raw)
In-Reply-To: <20220518181828.645877-1-naveen.n.rao@linux.vnet.ibm.com> (Naveen N. Rao's message of "Wed, 18 May 2022 23:48:28 +0530")
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> Since commit d1bcae833b32f1 ("ELF: Don't generate unused section
> symbols") [1], binutils (v2.36+) started dropping section symbols that
> it thought were unused. This isn't an issue in general, but with
> kexec_file.c, gcc is placing kexec_arch_apply_relocations[_add] into a
> separate .text.unlikely section and the section symbol ".text.unlikely"
> is being dropped. Due to this, recordmcount is unable to find a non-weak
> symbol in .text.unlikely to generate a relocation record against.
>
> Address this by dropping the weak attribute from these functions:
> - arch_kexec_apply_relocations() is not overridden by any architecture
> today, so just drop the weak attribute.
> - arch_kexec_apply_relocations_add() is only overridden by x86 and s390.
> Retain the function prototype for those and move the weak
> implementation into the header as a static inline for other
> architectures.
>
> [1] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1
Any chance you can also get machine_kexec_post_load,
crash_free_reserved_phys_range, arch_kexec_protect_protect_crashkres,
arch_kexec_unprotect_crashkres, arch_kexec_kernel_image_probe,
arch_kexec_kernel_image_probe, arch_kimage_file_post_load_cleanup,
arch_kexec_kernel_verify_sig, and arch_kexec_locate_mem_hole as well.
That is everything in kexec that uses a __weak symbol. If we can't
count on them working we might as well just get rid of the rest
preemptively.
Could you also address Andrews concerns by using a Kconfig symbol that
the architectures that implement the symbol can select.
I don't want to ask too much of a volunteer but if you are willing
addressing both of those would be a great help.
Eric
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> include/linux/kexec.h | 28 ++++++++++++++++++++++++----
> kernel/kexec_file.c | 19 +------------------
> 2 files changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 58d1b58a971e34..e656f981f43a73 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -193,10 +193,6 @@ void *kexec_purgatory_get_symbol_addr(struct kimage *image, const char *name);
> int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> unsigned long buf_len);
> void *arch_kexec_kernel_image_load(struct kimage *image);
> -int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> - Elf_Shdr *section,
> - const Elf_Shdr *relsec,
> - const Elf_Shdr *symtab);
> int arch_kexec_apply_relocations(struct purgatory_info *pi,
> Elf_Shdr *section,
> const Elf_Shdr *relsec,
> @@ -229,6 +225,30 @@ extern int crash_exclude_mem_range(struct crash_mem *mem,
> unsigned long long mend);
> extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
> void **addr, unsigned long *sz);
> +
> +#if defined(CONFIG_X86_64) || defined(CONFIG_S390)
> +int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> + Elf_Shdr *section,
> + const Elf_Shdr *relsec,
> + const Elf_Shdr *symtab);
> +#else
> +/*
> + * arch_kexec_apply_relocations_add - apply relocations of type RELA
> + * @pi: Purgatory to be relocated.
> + * @section: Section relocations applying to.
> + * @relsec: Section containing RELAs.
> + * @symtab: Corresponding symtab.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +static inline int
> +arch_kexec_apply_relocations_add(struct purgatory_info *pi, Elf_Shdr *section,
> + const Elf_Shdr *relsec, const Elf_Shdr *symtab)
> +{
> + pr_err("RELA relocation unsupported.\n");
> + return -ENOEXEC;
> +}
> +#endif /* CONFIG_X86_64 || CONFIG_S390 */
> #endif /* CONFIG_KEXEC_FILE */
>
> #ifdef CONFIG_KEXEC_ELF
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 8347fc158d2b96..6bae253b4d315e 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -108,23 +108,6 @@ int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> }
> #endif
>
> -/*
> - * arch_kexec_apply_relocations_add - apply relocations of type RELA
> - * @pi: Purgatory to be relocated.
> - * @section: Section relocations applying to.
> - * @relsec: Section containing RELAs.
> - * @symtab: Corresponding symtab.
> - *
> - * Return: 0 on success, negative errno on error.
> - */
> -int __weak
> -arch_kexec_apply_relocations_add(struct purgatory_info *pi, Elf_Shdr *section,
> - const Elf_Shdr *relsec, const Elf_Shdr *symtab)
> -{
> - pr_err("RELA relocation unsupported.\n");
> - return -ENOEXEC;
> -}
> -
> /*
> * arch_kexec_apply_relocations - apply relocations of type REL
> * @pi: Purgatory to be relocated.
> @@ -134,7 +117,7 @@ arch_kexec_apply_relocations_add(struct purgatory_info *pi, Elf_Shdr *section,
> *
> * Return: 0 on success, negative errno on error.
> */
> -int __weak
> +int
> arch_kexec_apply_relocations(struct purgatory_info *pi, Elf_Shdr *section,
> const Elf_Shdr *relsec, const Elf_Shdr *symtab)
> {
>
> base-commit: ef1302160bfb19f804451d0e919266703501c875
next prev parent reply other threads:[~2022-05-18 21:59 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-18 18:18 [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add] Naveen N. Rao
2022-05-18 18:18 ` Naveen N. Rao
2022-05-18 18:18 ` Naveen N. Rao
2022-05-18 20:47 ` Andrew Morton
2022-05-18 20:47 ` Andrew Morton
2022-05-18 20:47 ` Andrew Morton
2022-05-19 9:13 ` Naveen N. Rao
2022-05-19 9:13 ` Naveen N. Rao
2022-05-19 9:13 ` Naveen N. Rao
2022-05-18 21:59 ` Eric W. Biederman [this message]
2022-05-18 21:59 ` Eric W. Biederman
2022-05-18 21:59 ` Eric W. Biederman
2022-05-19 2:58 ` Baoquan He
2022-05-19 2:58 ` Baoquan He
2022-05-19 2:58 ` Baoquan He
2022-05-19 9:28 ` Naveen N. Rao
2022-05-19 9:28 ` Naveen N. Rao
2022-05-19 9:28 ` Naveen N. Rao
2022-05-19 17:59 ` Eric W. Biederman
2022-05-19 17:59 ` Eric W. Biederman
2022-05-19 17:59 ` Eric W. Biederman
2022-05-20 10:46 ` Baoquan He
2022-05-20 10:46 ` Baoquan He
2022-05-20 10:46 ` Baoquan He
2022-05-20 19:25 ` Eric W. Biederman
2022-05-20 19:25 ` Eric W. Biederman
2022-05-20 19:25 ` Eric W. Biederman
2022-05-25 19:56 ` Andrew Morton
2022-05-25 19:56 ` Andrew Morton
2022-05-25 19:56 ` Andrew Morton
2022-05-26 11:00 ` Naveen N. Rao
2022-05-26 11:00 ` Naveen N. Rao
2022-05-26 11:00 ` Naveen N. Rao
2022-05-19 5:41 ` Michael Ellerman
2022-05-19 5:41 ` Michael Ellerman
2022-05-19 5:41 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ee0q7b92.fsf@email.froward.int.ebiederm.org \
--to=ebiederm@xmission.com \
--cc=kexec@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.