Re: [PATCH v3 2/6] ieee1275/powerpc: enables device mapper discovery
On 2024-06-25 10:01, Michael Chang wrote:
On Thu, Jun 20, 2024 at 03:14:59PM GMT, avnish wrote:
Hi Vladimir,
We have implemented this code to enable the ieee1275 hint for grub. We
had
scenarios (in SLES) like the disk had PReP partition followed by an
LVM and,
inside this LVM , the boot partition. So, we implemented this code to
make
grub able to generate the hint. Without this code, we were unable to
find
the hint for the boot process. And, in some scenarios, due the amount
of
disks in the machine, the process could take a lot to finish.
As Vladimir pointed out, the correct search hint should be
lvmid/,
so grub will try to assemble the root logical volume first in the
process of looking up the root file system. This way, time won't be
wasted on trying those OpenFirmware raw disks, as they are not the
target.
The problem here is that bringing up the logical volume itself can be
very slow, and grub lacks measures like an LVM filter, which would help
to restrict which devices are scanned and improve performance. In the
end, the patch doesn't help either way.
In my opinion, this patch is more about updating the system bootlist
than achieving the proclaimed boot time enhancement.
Thanks,
Michael
Hi Vladimir and Michael,
Thank you so much for helping us on this patch!
Based on the inputs received. We would like to re analyze the fix done
by us. We'll check the fix whether this is really required for the
problem we have observed earlier. And based on our analysis, we'll
update on it.
For NVMeOF patch series, we would like to drop off this patch from patch
series (We'll remove this patch in next version).
Vladimir and Michael, please share your reviews on the other patches in
the NVMeOf series. We'll do the required changes as per suggestions on
the other patches.
Thank you!
Regards,
Avnish Chouhan
Shall we limit this function call specific to PowerPC? Something like
this
below:
#ifdef __powerpc__
realname = get_slave_from_dm (name_buf);
if (realname)
{
free (name_buf);
name_buf = realname;
}
#endif
Please suggest us.
Thank you so much!
Regards,
Avnish Chouhan
On 2024-06-07 15:05, grub-devel-requ...@gnu.org wrote:
> Send Grub-devel mailing list submissions to
>grub-devel@gnu.org
> --
>
> Message: 4
> Date: Fri, 7 Jun 2024 11:34:45 +0200
> From: "Vladimir 'phcoder' Serbinenko"
> To: avnish
> Cc: The development of GNU GRUB , Daniel Kiper
>, brk...@linux.ibm.com,
>meghanaprak...@in.ibm.com, Diego Domingos
> Subject: Re: [PATCH v3 2/6] ieee1275/powerpc: enables device mapper
>discovery
> Message-ID:
>
> Content-Type: text/plain; charset="utf-8"
>
> Le ven. 7 juin 2024, 10:34, avnish a écrit :
>
> > On 2024-06-06 21:04, Vladimir 'phcoder' Serbinenko wrote:
> > > 2 problems: * How does dm device ends up on ofpathname? It should be
> > > handled by grub internal logic in most cases and not end up in
> > > of-specific paths * Why not use existing devmapper functions already
> > > present in codebase? Le jeu. 6 juin 2024,
> > > > 2 problems:
> > > * How does dm device ends up on ofpathname? It should be handled by
> > > grub internal logic in most cases and not end up in of-specific paths
> > > * Why not use existing devmapper functions already present in
> > > codebase?
> > >
> >
> > Hi Vladimir,
> > Thank you so much for your response!
> >
> > We have observed that whenever we are dealing with the devices like
> > "/dev/dm-*", the ofpath returns null.
> >
> It's expected behavior
>
> > To resolve this, as no such required functions has been implemented to
> > handle this kind of case. We have done changes based on the
> > requirement
> > that will look into /sys/block/dm-* devices and search slave
> > devices recursively inside slaves directory to find the root disk.
>
> Installing on e.g. LVM is invalid. Only 3 cases can your approach work:
> 1) multipath
> 2) maybe RAID1 as well in some cases
> 3) if ofw assembles raid somehow
> Which one do you have? We need more details.
> This patch enables invalid configs like installing on LVM
>
> >
> > Regards,
> > Avnish Chouhan
> >
> > > Le jeu. 6 juin 2024, 14:40, Avnish Chouhan a
> > > écrit :
> > >
> > >> This patch enables the device mapper discovery on ofpath.c.
> > >> Currently,
> > >> when we are dealing with a device like /dev/dm-* the ofpath returns
> > >> null
> > >> since there is no function implemented to handle this case.
> > >>
> > >> This patch implements a function that will look into /sys/block/dm-*
> > >> devices and search recursively inside slaves directory to find the
> > >> root
> > >> disk.
> > >>
> > >> Signed-off-by: Diego Domingos
> > >> Signed-off-by: Avnish Chouhan
> > >> ---
> > >> grub-core/osdep/linux/ofpath.c | 64
> > >> +-
> > >> 1 file changed, 63 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/grub-core/osdep/linux/ofpath.c
> > >> b/grub-core/osdep/linux/ofpath.c
> > >> index
Re: [PATCH v9 00/22] Automatic Disk Unlock with TPM2
On Tue, Jun 25, 2024 at 02:42:31PM +0800, Gary Lin wrote:
> On Mon, Jun 24, 2024 at 07:28:14PM +0200, Daniel Kiper wrote:
> > On Thu, Mar 07, 2024 at 04:59:05PM +0800, Gary Lin via Grub-devel wrote:
> > > On Thu, Feb 08, 2024 at 08:58:43PM +0100, Daniel Kiper wrote:
> > > > Hey,
> > > >
> > > --8<--
> > > >
> > > > And I have attached the Coverity report. All issues reported there have
> > > > to be fixed. If you cannot fix an issue you have to explain why you
> > > > cannot do that and what is potential impact on the code stability,
> > > > security, etc.
> > > >
> > > I have went through all the coverity issues. There are 6 issues in the
> > > TPM2 stack and the utility:
> > >
> > > CID 435775, CID 435771, CID 435770, CID 435769, CID 435767, CID 435761
> > >
> > > Those will be fixed in the next version.
> > >
> > > The 9 issues are from libtasn1 and gnulib.
> > >
> > > There are two memory corruption issue: CID 435762 and CID 435766, and
> > > I've filed the issues in libtasn1 upstream.
> > >
> > > - CID 435762
> > > https://gitlab.com/gnutls/libtasn1/-/issues/49
> > > This is a valid case. However, the only exploitable function,
> > > _asn1_insert_tag_der(), is disabled in grub2 patch, so the severity is
> > > low. I have a quick fix but upstream would like to fix it in another
> > > way.
> > > - CID 435766
> > > https://gitlab.com/gnutls/libtasn1/-/issues/50
> > > IMO, this is false positive, but I need libtasn1 upstream to confirm
> > > that.
> > >
> > > Then, the remaining 7 Integer handling issues are involved with the macros
> > > from gnulib, and I believe those are false positive.
> > >
> > > The following are my analyses:
> > >
> > >
> > > *** CID 435774: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
> > > /grub-core/lib/libtasn1/lib/decoding.c: 481 in asn1_get_object_id_der()
> > > 475*/
> > > 476 if (leading != 0 && der[len_len + k] == 0x80)
> > > 477 return ASN1_DER_ERROR;
> > > 478 leading = 0;
> > > 479
> > > 480 /* check for wrap around */
> > > >>> CID 435774: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
> > > >>> "val < 1 ? 0 : val) - 1 < 0) ? ~1 ? 0 : val) + 1 << 62UL
> > > >>> /* sizeof (+val) * 8 - 2 */) - 1) * 2 + 1) : ((1 ? 0 : val) + 0)) >>
> > > >>> 7)" is always false regardless of the values of its operands. This
> > > >>> occurs as the second operand of "?:".
> > > 481 if (INT_LEFT_SHIFT_OVERFLOW (val, 7))
> > > 482 return ASN1_DER_ERROR;
> > > 483
> > > 484 val = val << 7;
> > > 485 val |= der[len_len + k] & 0x7F;
> > > 486
> > >
> > > Here are the related macros:
> > >
> > > #define EXPR_SIGNED(e) (_GL_INT_NEGATE_CONVERT (e, 1) < 0)
> > >
> > > #define _GL_INT_CONVERT(e, v) ((1 ? 0 : (e)) + (v))
> > >
> > > #define _GL_SIGNED_INT_MAXIMUM(e) \
> > > (((_GL_INT_CONVERT (e, 1) << (TYPE_WIDTH (+ (e)) - 2)) - 1) * 2 + 1)
> > >
> > > #define _GL_INT_MINIMUM(e) \
> > > (EXPR_SIGNED (e) \
> > >? ~ _GL_SIGNED_INT_MAXIMUM (e) \
> > >: _GL_INT_CONVERT (e, 0))
> > >
> > > #define INT_LEFT_SHIFT_OVERFLOW(a, b) \
> > > INT_LEFT_SHIFT_RANGE_OVERFLOW (a, b, \
> > > _GL_INT_MINIMUM (a), _GL_INT_MAXIMUM (a))
> > >
> > > #define INT_LEFT_SHIFT_RANGE_OVERFLOW(a, b, min, max) \
> > > ((a) < 0 \
> > >? (a) < (min) >> (b) \
> > >: (max) >> (b) < (a))
> > >
> > > The statement in question is expanded "(a) < (min) >> (b)" of
> > > INT_LEFT_SHIFT_RANGE_OVERFLOW. Since 'val' is 'uint64_t', '(a) < 0' is
> > > always
> > > false, so the result of that statement doen't matter.
> >
> > Something is missing and/or requires clarification/expansion here.
> > AFAICT at least your description completely ignores "(max) >> (b) < (a)"
> > expression result.
> >
> Because Coverity only complained "(a) < (min) >> (b)".
> "(max) >> (b) < (a)" does the right thing to check whether 'a' is
> overflowed after the left shift.
OK, then something like above should be added to the CID description then.
> > >
> > > *** CID 435773: Integer handling issues (NO_EFFECT)
> > > /grub-core/lib/libtasn1/lib/decoding.c: 439 in asn1_get_object_id_der()
> > > 433 return ASN1_DER_ERROR;
> > > 434
> > > 435 val0 = 0;
> > > 436
> > > 437 for (k = 0; k < len; k++)
> > > 438 {
> > > >>> CID 435773: Integer handling issues (NO_EFFECT)
> > > >>> This less-than-zero comparison of an unsigned value is never
> > > >>> true. "(1 ? 0UL : val0) - 1UL < 0UL".
>
Re: [PATCH v4 06/10] nx: set page permissions for loaded modules.
On Wed, Jun 12, 2024 at 04:57:09PM +0100, Mate Kukri wrote:
> For NX, we need to set write and executable permissions on the sections
> of grub modules when we load them.
>
> On sections with SHF_ALLOC set, which is typically everything except
> .modname and the symbol and string tables, this patch clears the Read
> Only flag on sections that have the ELF flag SHF_WRITE set, and clears
> the No eXecute flag on sections with SHF_EXECINSTR set. In all other
> cases it sets both flags.
>
> Original-Author: Peter Jones
> Original-Author: Robbie Harwood
> Original-Author: Laszlo Ersek
> Signed-off-by: Mate Kukri
> ---
> grub-core/kern/dl.c | 104 ++--
> include/grub/dl.h | 46
> 2 files changed, 137 insertions(+), 13 deletions(-)
>
> diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
> index 8338f7436..3341d78d6 100644
> --- a/grub-core/kern/dl.c
> +++ b/grub-core/kern/dl.c
> @@ -616,25 +616,97 @@ grub_dl_relocate_symbols (grub_dl_t mod, void *ehdr)
> grub_dl_segment_t seg;
> grub_err_t err;
>
> - /* Find the target segment. */
> - for (seg = mod->segment; seg; seg = seg->next)
> - if (seg->section == s->sh_info)
> - break;
> + seg = grub_dl_find_segment(mod, s->sh_info);
> +if (!seg)
> + continue;
This hunk is suspicious...
> - if (seg)
> - {
> - if (!mod->symtab)
> - return grub_error (GRUB_ERR_BAD_MODULE, "relocation without
> symbol table");
> + if (!mod->symtab)
> + return grub_error (GRUB_ERR_BAD_MODULE, "relocation without symbol
> table");
>
> - err = grub_arch_dl_relocate_symbols (mod, ehdr, s, seg);
> - if (err)
> - return err;
> - }
> + err = grub_arch_dl_relocate_symbols (mod, ehdr, s, seg);
> + if (err)
> + return err;
>}
>
>return GRUB_ERR_NONE;
> }
>
> +/* Only define this on EFI to save space in core */
> +#ifdef GRUB_MACHINE_EFI
> +static grub_err_t
> +grub_dl_set_mem_attrs (grub_dl_t mod, void *ehdr)
> +{
> + unsigned i;
s/unsigned/unsigned int/
> + const Elf_Shdr *s;
> + const Elf_Ehdr *e = ehdr;
> + grub_err_t err;
> +#if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv)
> + grub_size_t arch_addralign = grub_arch_dl_min_alignment ();
> + grub_addr_t tgaddr;
> + grub_size_t tgsz;
> +#endif
> +
> + for (i = 0, s = (const Elf_Shdr *)((const char *) e + e->e_shoff);
> + i < e->e_shnum;
> + i++, s = (const Elf_Shdr *)((const char *) s + e->e_shentsize))
> +{
> + grub_dl_segment_t seg;
> + grub_uint64_t set_attrs = GRUB_MEM_ATTR_R;
> + grub_uint64_t clear_attrs = GRUB_MEM_ATTR_W|GRUB_MEM_ATTR_X;
> +
> + seg = grub_dl_find_segment(mod, i);
> + if (!seg)
> + continue;
> +
> + if (seg->size == 0 || !(s->sh_flags & SHF_ALLOC))
> + continue;
> +
> + if (s->sh_flags & SHF_WRITE)
> + {
> + set_attrs |= GRUB_MEM_ATTR_W;
> + clear_attrs &= ~GRUB_MEM_ATTR_W;
> + }
> +
> + if (s->sh_flags & SHF_EXECINSTR)
> + {
> + set_attrs |= GRUB_MEM_ATTR_X;
> + clear_attrs &= ~GRUB_MEM_ATTR_X;
> + }
> +
> + err = grub_update_mem_attrs ((grub_addr_t)(seg->addr), seg->size,
> +set_attrs, clear_attrs);
> + if (err != GRUB_ERR_NONE)
> + return err;
> +}
> +
> +#if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv)
> + tgaddr = grub_min((grub_addr_t)mod->tramp, (grub_addr_t)mod->got);
> + tgsz = grub_max((grub_addr_t)mod->trampptr, (grub_addr_t)mod->gotptr) -
> tgaddr;
Wrong coding style for function calls and casts.
> + if (tgsz)
> +{
> + tgsz = ALIGN_UP(tgsz, arch_addralign);
> +
> + if (tgaddr < (grub_addr_t)mod->base ||
> + tgsz > (grub_addr_t)-1 - tgaddr ||
> + tgaddr + tgsz > (grub_addr_t)mod->base + mod->sz)
Wrong casts coding style...
> + return grub_error (GRUB_ERR_BUG,
> +"BUG: trying to protect pages outside of module "
> +"allocation (\"%s\"): module base %p, size 0x%"
> +PRIxGRUB_SIZE "; tramp/GOT base 0x%" PRIxGRUB_ADDR
> +", size 0x%" PRIxGRUB_SIZE,
> +mod->name, mod->base, mod->sz, tgaddr, tgsz);
> + err = grub_update_mem_attrs (tgaddr, tgsz,
> GRUB_MEM_ATTR_R|GRUB_MEM_ATTR_X,
> +GRUB_MEM_ATTR_W);
Missing space before and after "|" (I saw similar mistakes in the other
places too). And you do not need to wrap this line.
> + if (err != GRUB_ERR_NONE)
> + return err;
> +}
> +#endif
> +
> + return GRUB_ERR_NONE;
> +}
> +#endif
> +
> /* Load a module from core memory. */
> grub_dl_t
> grub_dl_load_core_noinit (void *addr, grub_size_t size)
> @@ -668,6 +740,7 @@ grub_dl_load_core_noinit (void *addr, grub_size_t size)
>mod->ref_count = 1;
>
>grub_dprintf ("modules", "relocating t
Re: [PATCH v4 07/10] nx: set the nx compatible flag in EFI grub images
On Wed, Jun 12, 2024 at 04:57:10PM +0100, Mate Kukri wrote: > For NX, we need the grub binary to announce that it is compatible with s/grub/GRUB/ > the NX feature. This implies that when loading the executable grub Ditto. May I ask you to use correct project name? > image, several attributes are true: > > - the binary doesn't need an executable stack > - the binary doesn't need sections to be both executable and writable > - the binary knows how to use the EFI Memory Attributes protocol on code > it is loading. > > This patch > - adds a definition for the PE DLL Characteristics flag GRUB_PE32_NX_COMPAT > - changes grub-mkimage to set that flag. > > Original-Author: Peter Jones > Signed-off-by: Mate Kukri If you fix nits mentioned above then you can add Reviewed-by: Daniel Kiper ... Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4 00/10] UEFI NX support and NX Linux loader using shim loader protocol
On Wed, Jun 12, 2024 at 04:57:03PM +0100, Mate Kukri wrote: > Currently the patchset consists of: > - Reworked Fedora NX patches to make GRUB itself work under NX. > - Julian Andres Klode's loader framework patch (used in Debian and Ubuntu for > the downstream loader). > - Implemented shim loader protocol support using the above loader framework. > - Added patch to disallow using the legacy Linux loader when NX is required. When I apply the patches git complains in the following way: Applying: modules: make .module_license read-only Applying: modules: strip .llvm_addrsig sections and similar. Applying: modules: Don't allocate space for non-allocable sections. Applying: modules: load module sections at page-aligned addresses Applying: nx: add memory attribute get/set API Applying: nx: set page permissions for loaded modules. .git/rebase-apply/patch:137: space before tab in indent. ) warning: 1 line adds whitespace errors. Applying: nx: set the nx compatible flag in EFI grub images Applying: efi: Provide wrappers for load_image, start_image, unload_image .git/rebase-apply/patch:174: trailing whitespace. grub_efi_status_t warning: 1 line adds whitespace errors. Applying: efi: Use shim's loader protocol for EFI image verification and loading Applying: efi: Disallow fallback to legacy Linux loader when shim says NX is required. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 1/2] mkimage: create new ELF Note for SBAT
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
Co-authored-by: Daniel Axtens
---
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"
+#define GRUB_SBAT_NOTE_TYPE 0x73626174 /* "sbat" */
+
+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);
+ struct grub_sbat_note *note_ptr = (struct grub_sbat_note *)footer;
+
+ note_ptr->header.n_namesz = grub_host_to_target32 (si
[PATCH v2 0/2] Secure Boot Advanced Targeting (SBAT) support on powerpc
In powerpc, PE format Binary are not supported and can't use shim (https://github.com/rhboot/shim/blob/main/SBAT.md). However, ELF binary are supported. So, we created new ELF note for SBAT in ELF binary which store the SBAT data and SBAT verifier will be there in firmware to read SBAT data from ELF note and validate it. this patch series consists of 2 parts: 1) Patch 1: create new ELF Note for SBAT 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. 2) Patch 2: adding sbat metadata into sbat ELF Note The SBAT metadata, which is read from .csv file and transformed into an ELF note, is made into an image using the -s option. (The rest of this cover letter concerns testing the entire end-to-end setup - SBAT.) You can experiement with this using entirely free software. You need the following trees: https://github.com/SudhakarKuppusamy1/qemu branch sbat https://github.com/SudhakarKuppusamy1/SLOF branch sbat https://github.com/SudhakarKuppusamy1/grub branch sbat You also need: - the SBAT metadata (.csv file) - the SBAT Variable (.csv file) Both should followed the SBAT specification (https://github.com/rhboot/shim/blob/main/SBAT.md) Example: https://github.com/SudhakarKuppusamy1/testing/sbat sbat_metadata.csv sbat_var.csv Lastly you will need a working a ppc64(le) vm. sample vm: https://github.com/SudhakarKuppusamy1/testing/vm pseries-ubuntu-20.04.6.qcow2 Then: - build qemu (./configure --target-list=ppc64-softmmu && make). You need qemu-system-ppc64. - use xxd (ex: xxd -i sbat_var.csv sbat_var.h) to convert the SBAT Variable for verifying grub into a header file, and copy it in to SLOF/lib/libcrypto/sbat_var.h. It must create variables sbat_var_csv and sbat_var_csv_len. - build SLOF for qemu (make qemu) - verify that you can boot your VM with new SLOF and stock grub. To boot with new SLOF, pass -bios ./SLOF/boot_rom.bin . It should boot with new slof in secure boot mode. sudo ./build/qemu-system-ppc64 -m 8192 -M pseries-2.12,accel=kvm,cap-ail-mode-3=off,secure-boot=on -nographic -vga none -smp 4 -hdd pseries-ubuntu-20.04.6.qcow2 -bios ./boot_rom.bin - Build grub in your VM. - Build the SBAT metadata into grub.The following incantation should give you a working but non-portable grub, assuming you have grub installed on /dev/sda2: GRUB_MODULES="all_video boot btrfs cat configfile echo ext2 fat font gfxmenu gfxterm gzio halt hfsplus http iso9660 jpeg loadenv loopback linux lvm mdraid09 mdraid1x minicmd net normal part_apple part_msdos part_gpt password_pbkdf2 png reboot regexp search search_fs_uuid search_fs_file search_label serial sleep syslinuxcfg test tftp video xfs" sudo ./grub-install --modules "$GRUB_MODULES" -d ./grub-core/ -v "/dev/sda2" --sbat=./sbat_metadata.csv dd if=/boot/grub/powerpc-ieee1275/core.elf of=/dev/sda2 Sudhakar Kuppusamy (2): mkimage: create new ELF Note for SBAT mkimage: adding sbat metadata into sbat ELF Note on powerpc include/grub/util/mkimage.h | 4 +-- util/grub-mkimagexx.c | 51 +++-- util/mkimage.c | 17 ++--- 3 files changed, 64 insertions(+), 8 deletions(-) -- 2.39.3 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 2/2] mkimage: adding sbat metadata into sbat ELF Note on powerpc
The SBAT metadata, which is read from .csv file and transformed into an ELF
note,
is made into an image using the -s option.
Signed-off-by: Sudhakar Kuppusamy
Co-authored-by: Daniel Axtens
---
util/mkimage.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/util/mkimage.c b/util/mkimage.c
index e67f7ce73..bafe190b0 100644
--- a/util/mkimage.c
+++ b/util/mkimage.c
@@ -943,8 +943,9 @@ grub_install_generate_image (const char *dir, const char
*prefix,
total_module_size += dtb_size + sizeof (struct grub_module_header);
}
- if (sbat_path != NULL && image_target->id != IMAGE_EFI)
-grub_util_error (_(".sbat section can be embedded into EFI images only"));
+ if (sbat_path != NULL && (image_target->id != IMAGE_EFI && image_target->id
!= IMAGE_PPC))
+grub_util_error (_(".sbat section can be embedded into EFI images/"
+ "sbat ELF Note can be added into powerpc-ieee1275
images only"));
if (disable_shim_lock)
total_module_size += sizeof (struct grub_module_header);
@@ -1814,6 +1815,13 @@ grub_install_generate_image (const char *dir, const char
*prefix,
{
grub_uint64_t target_addr;
char *sbat = NULL;
+ if (sbat_path != NULL)
+ {
+ sbat_size = grub_util_get_image_size (sbat_path);
+ sbat = xmalloc (sbat_size);
+ grub_util_load_image (sbat_path, sbat);
+ layout.sbat_size = sbat_size;
+ }
if (image_target->id == IMAGE_LOONGSON_ELF)
{
if (comp == GRUB_COMPRESSION_NONE)
--
2.39.3
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
