On Wed, 3 Nov 2021 14:22:07 -0400
Robbie Harwood <rharw...@redhat.com> wrote:
> From: Peter Jones <pjo...@redhat.com>
>
> Add a grub_dprintf() call during platform init and during module loading
> that tells us the virtual addresses of the .text and .data sections of
> grub-core/kernel.exec and any modules it loads.
>
> Specifically, it displays them in the gdb "add-symbol-file" syntax, with
> the presumption that there's a variable $grubdir that reflects the path
> to any such binaries.
>
> Signed-off-by: Peter Jones <pjo...@redhat.com>
> [rharw...@redhat.com: remove custom function, adjust commit message]
> Signed-off-by: Robbie Harwood <rharw...@redhat.com>
> ---
> grub-core/kern/dl.c | 50 +++++++++++++++++++++++++++++++++++++++
> grub-core/kern/efi/efi.c | 4 ++--
> grub-core/kern/efi/init.c | 26 +++++++++++++++++++-
> include/grub/efi/efi.h | 2 +-
> 4 files changed, 78 insertions(+), 4 deletions(-)
>
> diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
> index 6a52de168..a5def4ea3 100644
> --- a/grub-core/kern/dl.c
> +++ b/grub-core/kern/dl.c
> @@ -447,6 +447,23 @@ grub_dl_find_section (Elf_Ehdr *e, const char *name)
> return s;
> return NULL;
> }
> +static long
> +grub_dl_find_section_index (Elf_Ehdr *e, const char *name)
> +{
> + Elf_Shdr *s;
> + const char *str;
> + unsigned i;
> +
> + s = (Elf_Shdr *) ((char *) e + e->e_shoff + e->e_shstrndx *
> e->e_shentsize);
> + str = (char *) e + s->sh_offset;
> +
> + for (i = 0, s = (Elf_Shdr *) ((char *) e + e->e_shoff);
> + i < e->e_shnum;
> + i++, s = (Elf_Shdr *) ((char *) s + e->e_shentsize))
> + if (grub_strcmp (str + s->sh_name, name) == 0)
> + return (long)i;
> + return -1;
> +}
>
> /* Me, Vladimir Serbinenko, hereby I add this module check as per new
> GNU module policy. Note that this license check is informative only.
> @@ -607,6 +624,37 @@ grub_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
>
> return GRUB_ERR_NONE;
> }
> +static void
> +grub_dl_print_gdb_info (grub_dl_t mod, Elf_Ehdr *e)
> +{
> + void *text, *data = NULL;
> + long idx;
> +
> + idx = grub_dl_find_section_index (e, ".text");
> + if (idx < 0)
> + return;
> +
> + text = grub_dl_get_section_addr (mod, idx);
> + if (!text)
> + return;
> +
> + idx = grub_dl_find_section_index (e, ".data");
> + if (idx >= 0)
> + data = grub_dl_get_section_addr (mod, idx);
> +
> + if (data)
> + grub_dprintf ("gdb", "add-symbol-file \\\n"
> + "/usr/lib/debug/usr/lib/grub/%s-%s/%s.debug "
> + "\\\n %p -s .data %p\n",
> + GRUB_TARGET_CPU, GRUB_PLATFORM,
> + mod->name, text, data);
> + else
> + grub_dprintf ("gdb", "add-symbol-file \\\n"
> + "/usr/lib/debug/usr/lib/grub/%s-%s/%s.debug "
> + "\\\n%p\n",
> + GRUB_TARGET_CPU, GRUB_PLATFORM,
> + mod->name, text);
> +}
>
> /* Load a module from core memory. */
> grub_dl_t
> @@ -666,6 +714,8 @@ grub_dl_load_core_noinit (void *addr, grub_size_t size)
> grub_dprintf ("modules", "module name: %s\n", mod->name);
> grub_dprintf ("modules", "init function: %p\n", mod->init);
>
> + grub_dl_print_gdb_info (mod, e);
> +
> if (grub_dl_add (mod))
> {
> grub_dl_unload (mod);
> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> index 8cff7be02..f12fd5893 100644
> --- a/grub-core/kern/efi/efi.c
> +++ b/grub-core/kern/efi/efi.c
> @@ -291,7 +291,7 @@ grub_efi_get_variable (const char *var, const
> grub_efi_guid_t *guid,
> /* Search the mods section from the PE32/PE32+ image. This code uses
> a PE32 header, but should work with PE32+ as well. */
> grub_addr_t
> -grub_efi_modules_addr (void)
> +grub_efi_section_addr (const char *section_name)
> {
> grub_efi_loaded_image_t *image;
> struct grub_pe32_header *header;
> @@ -316,7 +316,7 @@ grub_efi_modules_addr (void)
> i < coff_header->num_sections;
> i++, section++)
> {
> - if (grub_strcmp (section->name, "mods") == 0)
> + if (grub_strcmp (section->name, section_name) == 0)
> break;
> }
>
> diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
> index 7facacf09..35f787915 100644
> --- a/grub-core/kern/efi/init.c
> +++ b/grub-core/kern/efi/init.c
> @@ -82,10 +82,33 @@ stack_protector_init (void)
>
> grub_addr_t grub_modbase;
>
> +static void
> +grub_efi_print_gdb_info (void)
> +{
> + grub_addr_t text;
> + grub_addr_t data;
> +
> + text = grub_efi_section_addr (".text");
> + if (!text)
> + return;
> +
> + data = grub_efi_section_addr (".data");
> + if (data)
> + grub_dprintf ("gdb",
> + "add-symbol-file /usr/lib/debug/usr/lib/grub/%s-%s/"
> + "kernel.exec %p -s .data %p\n",
> + GRUB_TARGET_CPU, GRUB_PLATFORM, (void *)text, (void *)data);
As far as I can tell, this is an incorrect value for data. The value
for the data segment that gets displayed is the location of the UEFI
data segment (PE32+ format). But the location you want is of the ELF
data segment in kernel.exec. The UEFI data segment is a combination of
all the segments marked as data followed by bss from the ELF kernel.exec
file (rodata, rodata.str1.1, data, module_license, and bss from what
I'm seeing).
Does your patch actually work for you? Function symbols will be fine,
but any data variables should be not right (eg. grub_dl_head).
Currently, GRUB doesn't have enough information to reconstruct where
ELF data segments and BSS map into the PE32+ data segment. It needs the
name, size and alignment of the ELF segments to figure it out.
I see a couple solutions.
1. Have a function in gdb_grub that figures everything out based on the
PE32+ data segment offset. I could use objdump to get the info from
kernel.exec and then load the symbol file itself. It would be called
like "grub_add_symbols <text offset> <data offset>".
2. Have grub-mkimage insert the required data into the UEFI binary so
GRUB can figure it out and print the add-symbol-file line as you're
wanting to do in this patch. Perhaps it could go into a new section
in the UEFI binary or patch it into a global variable (I'd prefer the
former).
Option 1 seems more hackish, but probably easier (I've got a working
POC). Option 2 using another PE32+ section, seems like a cleaner
approach, but (maybe) more work.
Glenn
> + else
> + grub_dprintf ("gdb",
> + "add-symbol-file /usr/lib/debug/usr/lib/grub/%s-%s/"
> + "kernel.exec %p\n",
> + GRUB_TARGET_CPU, GRUB_PLATFORM, (void *)text);
> +}
> +
> void
> grub_efi_init (void)
> {
> - grub_modbase = grub_efi_modules_addr ();
> + grub_modbase = grub_efi_section_addr ("mods");
> /* First of all, initialize the console so that GRUB can display
> messages. */
> grub_console_init ();
> @@ -108,6 +131,7 @@ grub_efi_init (void)
> efi_call_4 (grub_efi_system_table->boot_services->set_watchdog_timer,
> 0, 0, 0, NULL);
>
> + grub_efi_print_gdb_info ();
> grub_efidisk_init ();
> }
>
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index 83d958f99..73b754177 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -105,7 +105,7 @@ grub_err_t grub_arch_efi_linux_boot_image(grub_addr_t
> addr, grub_size_t size,
> char *args);
> #endif
>
> -grub_addr_t grub_efi_modules_addr (void);
> +grub_addr_t grub_efi_section_addr (const char *section);
>
> void grub_efi_mm_init (void);
> void grub_efi_mm_fini (void);
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
