On Fri, Sep 13, 2024 at 04:57:58PM +0530, Sudhakar Kuppusamy wrote:
> In order to store the SBAT data, we create a new ELF note. The string
> "Secure-Boot-Advanced-Targeting",
> zero-padded to 4 byte alignment, shall be entered in the name field. The
> string "sbat"'s ASCII values,
> 0x41536967, should be entered in the type field.
>
> Signed-off-by: Sudhakar Kuppusamy <sudha...@linux.ibm.com>
> Co-authored-by: Daniel Axtens <d...@axtens.net>
s/Co-authored-by/Signed-off-by/
... and AIUI Daniel's SOB should be first...
> ---
> include/grub/util/mkimage.h | 4 +--
> util/grub-mkimagexx.c | 51 +++++++++++++++++++++++++++++++++++--
> util/mkimage.c | 5 ++--
> 3 files changed, 54 insertions(+), 6 deletions(-)
>
> diff --git a/include/grub/util/mkimage.h b/include/grub/util/mkimage.h
> index 3819a6744..9d74f82c5 100644
> --- a/include/grub/util/mkimage.h
> +++ b/include/grub/util/mkimage.h
> @@ -51,12 +51,12 @@ grub_mkimage_load_image64 (const char *kernel_path,
> const struct grub_install_image_target_desc
> *image_target);
> void
> grub_mkimage_generate_elf32 (const struct grub_install_image_target_desc
> *image_target,
> - int note, char **core_img, size_t *core_size,
> + int note, char *sbat, char **core_img, size_t
> *core_size,
> Elf32_Addr target_addr,
> struct grub_mkimage_layout *layout);
> void
> grub_mkimage_generate_elf64 (const struct grub_install_image_target_desc
> *image_target,
> - int note, char **core_img, size_t *core_size,
> + int note, char *sbat, char **core_img, size_t
> *core_size,
> Elf64_Addr target_addr,
> struct grub_mkimage_layout *layout);
>
> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> index e50b29533..99f6ab05e 100644
> --- a/util/grub-mkimagexx.c
> +++ b/util/grub-mkimagexx.c
> @@ -107,6 +107,14 @@ struct section_metadata
> const char *strtab;
> };
>
> +#define GRUB_SBAT_NOTE_NAME "Secure-Boot-Advanced-Targeting"
Why cannot be it in line with PE section name, i.e. ".sbat"?
> +#define GRUB_SBAT_NOTE_TYPE 0x73626174 /* "sbat" */
I think SBAT would be more appropriate here...
> +struct grub_sbat_note {
> + Elf32_Nhdr header;
> + char name[ALIGN_UP(sizeof(GRUB_SBAT_NOTE_NAME), 4)];
> +};
> +
> static int
> is_relocatable (const struct grub_install_image_target_desc *image_target)
> {
> @@ -208,7 +216,7 @@ grub_arm_reloc_jump24 (grub_uint32_t *target, Elf32_Addr
> sym_addr)
>
> void
> SUFFIX (grub_mkimage_generate_elf) (const struct
> grub_install_image_target_desc *image_target,
> - int note, char **core_img, size_t
> *core_size,
> + int note, char *sbat, char **core_img,
> size_t *core_size,
> Elf_Addr target_addr,
> struct grub_mkimage_layout *layout)
> {
> @@ -217,10 +225,17 @@ SUFFIX (grub_mkimage_generate_elf) (const struct
> grub_install_image_target_desc
> Elf_Ehdr *ehdr;
> Elf_Phdr *phdr;
> Elf_Shdr *shdr;
> - int header_size, footer_size = 0;
> + int header_size, footer_size = 0, footer_offset = 0;
> int phnum = 1;
> int shnum = 4;
> int string_size = sizeof (".text") + sizeof ("mods") + 1;
> + char *footer;
> +
> + if (sbat)
> + {
> + phnum++;
> + footer_size += ALIGN_UP (sizeof (struct grub_sbat_note) +
> layout->sbat_size, 4);
> + }
>
> if (image_target->id != IMAGE_LOONGSON_ELF)
> phnum += 2;
> @@ -248,6 +263,7 @@ SUFFIX (grub_mkimage_generate_elf) (const struct
> grub_install_image_target_desc
> ehdr = (void *) elf_img;
> phdr = (void *) (elf_img + sizeof (*ehdr));
> shdr = (void *) (elf_img + sizeof (*ehdr) + phnum * sizeof (*phdr));
> + footer = elf_img + program_size + header_size;
> memcpy (ehdr->e_ident, ELFMAG, SELFMAG);
> ehdr->e_ident[EI_CLASS] = ELFCLASSXX;
> if (!image_target->bigendian)
> @@ -420,6 +436,8 @@ SUFFIX (grub_mkimage_generate_elf) (const struct
> grub_install_image_target_desc
> phdr->p_filesz = grub_host_to_target32 (XEN_NOTE_SIZE);
> phdr->p_memsz = 0;
> phdr->p_offset = grub_host_to_target32 (header_size + program_size);
> + footer = ptr;
> + footer_offset = XEN_NOTE_SIZE;
> }
>
> if (image_target->id == IMAGE_XEN_PVH)
> @@ -453,6 +471,8 @@ SUFFIX (grub_mkimage_generate_elf) (const struct
> grub_install_image_target_desc
> phdr->p_filesz = grub_host_to_target32 (XEN_PVH_NOTE_SIZE);
> phdr->p_memsz = 0;
> phdr->p_offset = grub_host_to_target32 (header_size + program_size);
> + footer = ptr;
> + footer_offset = XEN_PVH_NOTE_SIZE;
> }
>
> if (note)
> @@ -483,6 +503,33 @@ SUFFIX (grub_mkimage_generate_elf) (const struct
> grub_install_image_target_desc
> phdr->p_filesz = grub_host_to_target32 (note_size);
> phdr->p_memsz = 0;
> phdr->p_offset = grub_host_to_target32 (header_size + program_size);
> + footer = (elf_img + program_size + header_size + note_size);
> + footer_offset += note_size;
> + }
> +
> + if (sbat)
> + {
> + int note_size = ALIGN_UP(sizeof (struct grub_sbat_note) +
> layout->sbat_size, 4);
Missing space before "(". I can see similar mistakes in other places.
Please fix them too.
> + struct grub_sbat_note *note_ptr = (struct grub_sbat_note *)footer;
Missing space after ")". And same as above, casts formatting are broken
in other places too.
> +
> + note_ptr->header.n_namesz = grub_host_to_target32 (sizeof
> (GRUB_SBAT_NOTE_NAME));
> + note_ptr->header.n_descsz = grub_host_to_target32
> (ALIGN_UP(layout->sbat_size, 4));
> + note_ptr->header.n_type = grub_host_to_target32 (GRUB_SBAT_NOTE_TYPE);
> + memcpy (note_ptr->name, GRUB_SBAT_NOTE_NAME, sizeof
> (GRUB_SBAT_NOTE_NAME));
> + memcpy ((char *)(note_ptr + 1), sbat, layout->sbat_size);
> +
> + phdr++;
> + phdr->p_type = grub_host_to_target32 (PT_NOTE);
> + phdr->p_flags = grub_host_to_target32 (PF_R);
> + phdr->p_align = grub_host_to_target32 (image_target->voidp_sizeof);
> + phdr->p_vaddr = 0;
> + phdr->p_paddr = 0;
> + phdr->p_filesz = grub_host_to_target32 (note_size);
> + phdr->p_memsz = 0;
> + phdr->p_offset = grub_host_to_target32 (header_size + program_size +
> footer_offset);
> +
> + footer += note_size;
> + footer_offset += note_size;
You can drop these two lines.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
