Re: [PATCH] loader/efi/fdt: Ensure FDT blob is 8-byte aligned
Mikle Lappo <[email protected]> Thu, 26 Feb 2026 10:25:25 +0100
| Newsgroups | org.gnu.grub-devel |
|---|---|
| Message-ID | <CAEnBdDr-vzMfwhZtfw2C8Y8q=7Ne5X39JkEev2ZEbW16cPLvGQ@mail.gmail.com> |
--===============0015359737784534889== Content-Type: multipart/alternative; boundary="000000000000935292064bb6b5b3" --000000000000935292064bb6b5b3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Good morning Heinrich, thanks for pointing this out, I overlooked it. I enabled GRUB2 logs and I also see that DTB is loaded aligned. devicetree <target_name>.dtb kern/disk.c:196:disk: Opening `tftp,128.0.0.1'... disk/efi/efidisk.c:482:efidisk: opening tftp kern/disk.c:281:disk: Opening `tftp,128.0.0.1' failed. kern/disk.c:299:disk: Closing `tftp'. tpm/grub_tpm.c:95:tpm: extending pcr#15 from file type 44 ??mmands/efi/tpm.c:281:tpm: log_event, pcr =3D 15, size =3D 0x43b3, ? kern/verifiers.c:88:verify: file: <target_name>.dtb type: 44 commands/efi/tpm.c:281:tpm: log_event, pcr =3D 9, size =3D 0x43b3, <target_name>.dtb loader/efi/linux.c:689:linux: filename <target_name>.dtb, size 17331 loader/efi/linux.c:649:loader: load_dtbloader/efi/linux.c:654:loader: Loading device tree to 0x6fbbd600 Still I get FDT_ERR_ALIGNMENT in fdt_check_header when booting... Unfortunately my Grub2 comes as binary from BSP and I can't recompile nor really debug it. I'll update my CR if I find anything or close it if not. Best regards, Mikhail On Thu, Feb 26, 2026 at 9:31=E2=80=AFAM Heinrich Schuchardt < [email protected]> wrote: > On 2/26/26 09:14, Mikle Lappo wrote: > > Add maintainers into CC > > > > On Wed, Feb 25, 2026 at 2:07=E2=80=AFPM <[email protected] > > <mailto:[email protected]>> wrote: > > > > From: Mikhail Lappo <[email protected] <mailto: > [email protected]>> > > > > libfdt requires the FDT blob to be 8-byte aligned. The alignment is > > validated in fdt_check_header(), and misaligned blobs may be reject= ed > > or cause failures on architectures that enforce strict alignment. > > > > Currently the blob is allocated with grub_malloc(), which does not > > guarantee 8-byte alignment. Replace it with grub_memalign(8, size) > > to ensure the required alignment. > > > > Signed-off-by: Mikhail Lappo <[email protected] > > <mailto:[email protected]>> > > --- > > grub-core/loader/efi/fdt.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.= c > > index e510b3491..f25d362d3 100644 > > --- a/grub-core/loader/efi/fdt.c > > +++ b/grub-core/loader/efi/fdt.c > > @@ -38,6 +38,7 @@ static void *fdt; > > #define FDT_ADDR_SIZE_EXTRA ((2 * grub_fdt_prop_entry_size > > (sizeof(grub_uint32_t))) + \ > > sizeof (FDT_ADDR_CELLS_STRING) + \ > > sizeof (FDT_SIZE_CELLS_STRING)) > > +#define FDT_BLOB_ALIGNMENT 8 > > > > static const struct grub_arg_option options_fdtdump[] =3D { > > {"prop", 'p', 0, N_("Get property."), N_("prop"), > > ARG_TYPE_STRING}, > > @@ -145,7 +146,7 @@ grub_cmd_devicetree (grub_command_t cmd > > __attribute__ ((unused)), > > goto out; > > > > size =3D grub_file_size (dtb); > > - blob =3D grub_malloc (size); > > + blob =3D grub_memalign (FDT_BLOB_ALIGNMENT, size); > > if (!blob) > > goto out; > > > > -- > > 2.44.0 > > > > Isn't #define GRUB_MM_ALIGN_LOG2 setting a minimum alignment of 16 bytes > on 32-bit and 32 bytes on 64-bit systems? > > Please, have a look at the code starting here: > > grub-core/kern/mm.c:478: align =3D (align >> GRUB_MM_ALIGN_LOG2); > > Best regards > > Heinrich > --000000000000935292064bb6b5b3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr">Good morning Heinrich,<div><br></div><div>thanks for point= ing this out, I overlooked it.</div><div>I enabled GRUB2 logs and I also se= e that DTB is loaded aligned.</div><div><br></div><div>devicetree <targe= t_name>.dtb<br>kern/disk.c:196:disk: Opening `tftp,128.0.0.1'...<br>= disk/efi/efidisk.c:482:efidisk: opening tftp<br>kern/disk.c:281:disk: Openi= ng `tftp,128.0.0.1' failed.<br>kern/disk.c:299:disk: Closing `tftp'= .<br>tpm/grub_tpm.c:95:tpm: extending pcr#15 from file type 44<br>??mmands/= efi/tpm.c:281:tpm: log_event, pcr =3D 15, size =3D 0x43b3, ?<br>kern/verifi= ers.c:88:verify: file: <target_name>.dtb type: 44<br>commands/efi/tpm= .c:281:tpm: log_event, pcr =3D 9, size =3D 0x43b3, <target_name>.dtb<= br>loader/efi/linux.c:689:linux: filename <target_name>.dtb, size 173= 31<br>loader/efi/linux.c:649:loader: load_dtbloader/efi/linux.c:654:loader:= Loading<br>device tree to 0x6fbbd600<br></div><div><br></div><div>Still I = get FDT_ERR_ALIGNMENT in fdt_check_header when booting...</div><div>Unfortu= nately my Grub2 comes as binary from BSP and I can't recompile nor real= ly debug it.</div><div><br></div><div>I'll update my CR if I find anyth= ing or close it if not.</div><div><br></div><div>Best regards,</div><div>Mi= khail</div></div><br><div class=3D"gmail_quote gmail_quote_container"><div = dir=3D"ltr" class=3D"gmail_attr">On Thu, Feb 26, 2026 at 9:31=E2=80=AFAM He= inrich Schuchardt <<a href=3D"mailto:[email protected]">= [email protected]</a>> wrote:<br></div><blockquote class= =3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left-width:1px;bo= rder-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">= On 2/26/26 09:14, Mikle Lappo wrote:<br> > Add maintainers into CC<br> > <br> > On Wed, Feb 25, 2026 at 2:07=E2=80=AFPM <<a href=3D"mailto:miklelap= [email protected]" target=3D"_blank">[email protected]</a> <br> > <mailto:<a href=3D"mailto:[email protected]" target=3D"_blank">m= [email protected]</a>>> wrote:<br> > <br> >=C2=A0 =C2=A0 =C2=A0From: Mikhail Lappo <<a href=3D"mailto:miklelapp= [email protected]" target=3D"_blank">[email protected]</a> <mailto:<a href= =3D"mailto:[email protected]" target=3D"_blank">[email protected]</a>= >><br> > <br> >=C2=A0 =C2=A0 =C2=A0libfdt requires the FDT blob to be 8-byte aligned. = The alignment is<br> >=C2=A0 =C2=A0 =C2=A0validated in fdt_check_header(), and misaligned blo= bs may be rejected<br> >=C2=A0 =C2=A0 =C2=A0or cause failures on architectures that enforce str= ict alignment.<br> > <br> >=C2=A0 =C2=A0 =C2=A0Currently the blob is allocated with grub_malloc(),= which does not<br> >=C2=A0 =C2=A0 =C2=A0guarantee 8-byte alignment. Replace it with grub_me= malign(8, size)<br> >=C2=A0 =C2=A0 =C2=A0to ensure the required alignment.<br> > <br> >=C2=A0 =C2=A0 =C2=A0Signed-off-by: Mikhail Lappo <<a href=3D"mailto:= [email protected]" target=3D"_blank">[email protected]</a><br> >=C2=A0 =C2=A0 =C2=A0<mailto:<a href=3D"mailto:[email protected]" = target=3D"_blank">[email protected]</a>>><br> >=C2=A0 =C2=A0 =C2=A0---<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0grub-core/loader/efi/fdt.c | 3 ++-<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A01 file changed, 2 insertions(+), 1 deletion(= -)<br> > <br> >=C2=A0 =C2=A0 =C2=A0diff --git a/grub-core/loader/efi/fdt.c b/grub-core= /loader/efi/fdt.c<br> >=C2=A0 =C2=A0 =C2=A0index e510b3491..f25d362d3 100644<br> >=C2=A0 =C2=A0 =C2=A0--- a/grub-core/loader/efi/fdt.c<br> >=C2=A0 =C2=A0 =C2=A0+++ b/grub-core/loader/efi/fdt.c<br> >=C2=A0 =C2=A0 =C2=A0@@ -38,6 +38,7 @@ static void *fdt;<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0#define FDT_ADDR_SIZE_EXTRA ((2 * grub_fdt_p= rop_entry_size<br> >=C2=A0 =C2=A0 =C2=A0(sizeof(grub_uint32_t))) + \<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeof (FDT_ADDR_CE= LLS_STRING) + \<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeof (FDT_SIZE_CE= LLS_STRING))<br> >=C2=A0 =C2=A0 =C2=A0+#define FDT_BLOB_ALIGNMENT 8<br> > <br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0static const struct grub_arg_option options_= fdtdump[] =3D {<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{"prop",=C2=A0 =C2=A0 =C2= =A0'p', 0, N_("Get property."), N_("prop"),<br> >=C2=A0 =C2=A0 =C2=A0ARG_TYPE_STRING},<br> >=C2=A0 =C2=A0 =C2=A0@@ -145,7 +146,7 @@ grub_cmd_devicetree (grub_comma= nd_t cmd<br> >=C2=A0 =C2=A0 =C2=A0__attribute__ ((unused)),<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto out;<br> > <br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0size =3D grub_file_size (dtb);<br> >=C2=A0 =C2=A0 =C2=A0-=C2=A0 blob =3D grub_malloc (size);<br> >=C2=A0 =C2=A0 =C2=A0+=C2=A0 blob =3D grub_memalign (FDT_BLOB_ALIGNMENT,= size);<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!blob)<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto out;<br> > <br> >=C2=A0 =C2=A0 =C2=A0-- <br> >=C2=A0 =C2=A0 =C2=A02.44.0<br> > <br> <br> Isn't #define GRUB_MM_ALIGN_LOG2 setting a minimum alignment of 16 byte= s <br> on 32-bit and 32 bytes on 64-bit systems?<br> <br> Please, have a look at the code starting here:<br> <br> grub-core/kern/mm.c:478:=C2=A0 align =3D (align >> GRUB_MM_ALIGN_LOG2= );<br> <br> Best regards<br> <br> Heinrich<br> </blockquote></div> --000000000000935292064bb6b5b3-- --===============0015359737784534889== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KR3J1Yi1kZXZl bCBtYWlsaW5nIGxpc3QKR3J1Yi1kZXZlbEBnbnUub3JnCmh0dHBzOi8vbGlzdHMuZ251Lm9yZy9t YWlsbWFuL2xpc3RpbmZvL2dydWItZGV2ZWwK --===============0015359737784534889==--