diff mbox series

[v3,10/10] fdt: Add device tree file type

Message ID 20181114172739.51640-11-agraf@suse.de
State New
Headers show
Series Add RISC-V support | expand

Commit Message

Alexander Graf Nov. 14, 2018, 5:27 p.m. UTC 
We now have signature check logic in grub which allows us to treat
files differently depending on their file type.

Mark a loaded device tree as such and treat it like an overlayed ACPI
table. Both describe hardware, so I suppose their threat level is the
same.

Signed-off-by: Alexander Graf <agraf@suse.de>

---
 grub-core/commands/efi/shim_lock.c | 1 +
 grub-core/loader/efi/fdt.c         | 2 +-
 include/grub/file.h                | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.12.3


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Comments

Alistair Francis Nov. 16, 2018, 10:07 p.m. UTC | #1 
On Wed, 2018-11-14 at 18:27 +0100, Alexander Graf wrote:
> We now have signature check logic in grub which allows us to treat

> files differently depending on their file type.

> 

> Mark a loaded device tree as such and treat it like an overlayed ACPI

> table. Both describe hardware, so I suppose their threat level is the

> same.

> 

> Signed-off-by: Alexander Graf <agraf@suse.de>


Reviewed-by: Alistair Francis <alistair.francis@wdc.com>


Alistair

> ---

>  grub-core/commands/efi/shim_lock.c | 1 +

>  grub-core/loader/efi/fdt.c         | 2 +-

>  include/grub/file.h                | 2 ++

>  3 files changed, 4 insertions(+), 1 deletion(-)

> 

> diff --git a/grub-core/commands/efi/shim_lock.c b/grub-

> core/commands/efi/shim_lock.c

> index 01246b0fc..90dccb0c7 100644

> --- a/grub-core/commands/efi/shim_lock.c

> +++ b/grub-core/commands/efi/shim_lock.c

> @@ -81,6 +81,7 @@ shim_lock_init (grub_file_t io, enum grub_file_type

> type,

>        /* Fall through. */

>  

>      case GRUB_FILE_TYPE_ACPI_TABLE:

> +    case GRUB_FILE_TYPE_DEVICE_TREE:

>        *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH;

>  

>        return GRUB_ERR_NONE;

> diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c

> index a4c6e8036..d8ebe648e 100644

> --- a/grub-core/loader/efi/fdt.c

> +++ b/grub-core/loader/efi/fdt.c

> @@ -123,7 +123,7 @@ grub_cmd_devicetree (grub_command_t cmd

> __attribute__ ((unused)),

>        return GRUB_ERR_NONE;

>      }

>  

> -  dtb = grub_file_open (argv[0]);

> +  dtb = grub_file_open (argv[0], GRUB_FILE_TYPE_DEVICE_TREE);

>    if (!dtb)

>      goto out;

>  

> diff --git a/include/grub/file.h b/include/grub/file.h

> index 19dda67f6..b8fb13017 100644

> --- a/include/grub/file.h

> +++ b/include/grub/file.h

> @@ -93,6 +93,8 @@ enum grub_file_type

>      GRUB_FILE_TYPE_FILE_ID,

>      /* File holding ACPI table.  */

>      GRUB_FILE_TYPE_ACPI_TABLE,

> +    /* File holding Device Tree.  */

> +    GRUB_FILE_TYPE_DEVICE_TREE,

>      /* File we intend show to user.  */

>      GRUB_FILE_TYPE_CAT,

>      GRUB_FILE_TYPE_HEXCAT,

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Andreas Schwab Nov. 19, 2018, 10:11 a.m. UTC | #2 
On Nov 14 2018, Alexander Graf <agraf@suse.de> wrote:

> diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c

> index a4c6e8036..d8ebe648e 100644

> --- a/grub-core/loader/efi/fdt.c

> +++ b/grub-core/loader/efi/fdt.c

> @@ -123,7 +123,7 @@ grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)),

>        return GRUB_ERR_NONE;

>      }

>  

> -  dtb = grub_file_open (argv[0]);

> +  dtb = grub_file_open (argv[0], GRUB_FILE_TYPE_DEVICE_TREE);

>    if (!dtb)

>      goto out;

>  


Looks like this has been obsoleted by commit dfb1742aab?

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Leif Lindholm Nov. 19, 2018, 5:56 p.m. UTC | #3 
On Mon, Nov 19, 2018 at 11:11:09AM +0100, Andreas Schwab wrote:
> On Nov 14 2018, Alexander Graf <agraf@suse.de> wrote:

> 

> > diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c

> > index a4c6e8036..d8ebe648e 100644

> > --- a/grub-core/loader/efi/fdt.c

> > +++ b/grub-core/loader/efi/fdt.c

> > @@ -123,7 +123,7 @@ grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)),

> >        return GRUB_ERR_NONE;

> >      }

> >  

> > -  dtb = grub_file_open (argv[0]);

> > +  dtb = grub_file_open (argv[0], GRUB_FILE_TYPE_DEVICE_TREE);

> >    if (!dtb)

> >      goto out;

> >  

> 

> Looks like this has been obsoleted by commit dfb1742aab?


Actually, that one does not do anything sensible in
grub-core/commands/efi/shim_lock.c
so that hunk would be worth considering including.

Treating DT and ACPI equally sounds sensible to me.

/
    Leif

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Daniel Kiper Nov. 21, 2018, 4:27 p.m. UTC | #4 
On Wed, Nov 14, 2018 at 06:27:39PM +0100, Alexander Graf wrote:
> We now have signature check logic in grub which allows us to treat

> files differently depending on their file type.

>

> Mark a loaded device tree as such and treat it like an overlayed ACPI

> table. Both describe hardware, so I suppose their threat level is the

> same.

>

> Signed-off-by: Alexander Graf <agraf@suse.de>

> ---

>  grub-core/commands/efi/shim_lock.c | 1 +

>  grub-core/loader/efi/fdt.c         | 2 +-

>  include/grub/file.h                | 2 ++

>  3 files changed, 4 insertions(+), 1 deletion(-)

>

> diff --git a/grub-core/commands/efi/shim_lock.c b/grub-core/commands/efi/shim_lock.c

> index 01246b0fc..90dccb0c7 100644

> --- a/grub-core/commands/efi/shim_lock.c

> +++ b/grub-core/commands/efi/shim_lock.c

> @@ -81,6 +81,7 @@ shim_lock_init (grub_file_t io, enum grub_file_type type,

>        /* Fall through. */

>

>      case GRUB_FILE_TYPE_ACPI_TABLE:

> +    case GRUB_FILE_TYPE_DEVICE_TREE:

>        *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH;

>

>        return GRUB_ERR_NONE;

> diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c

> index a4c6e8036..d8ebe648e 100644

> --- a/grub-core/loader/efi/fdt.c

> +++ b/grub-core/loader/efi/fdt.c

> @@ -123,7 +123,7 @@ grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)),

>        return GRUB_ERR_NONE;

>      }

>

> -  dtb = grub_file_open (argv[0]);

> +  dtb = grub_file_open (argv[0], GRUB_FILE_TYPE_DEVICE_TREE);

>    if (!dtb)

>      goto out;

>

> diff --git a/include/grub/file.h b/include/grub/file.h

> index 19dda67f6..b8fb13017 100644

> --- a/include/grub/file.h

> +++ b/include/grub/file.h

> @@ -93,6 +93,8 @@ enum grub_file_type

>      GRUB_FILE_TYPE_FILE_ID,

>      /* File holding ACPI table.  */

>      GRUB_FILE_TYPE_ACPI_TABLE,

> +    /* File holding Device Tree.  */

> +    GRUB_FILE_TYPE_DEVICE_TREE,

>      /* File we intend show to user.  */

>      GRUB_FILE_TYPE_CAT,

>      GRUB_FILE_TYPE_HEXCAT,


You have to rebase this patch set on latest master. It has
GRUB_FILE_TYPE_DEVICE_TREE_IMAGE instead of GRUB_FILE_TYPE_DEVICE_TREE.
Please use it. If you wish you can move it behind GRUB_FILE_TYPE_ACPI_TABLE
in grub_file_type.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Daniel Kiper Nov. 21, 2018, 4:36 p.m. UTC | #5 
On Mon, Nov 19, 2018 at 05:56:36PM +0000, Leif Lindholm wrote:
> On Mon, Nov 19, 2018 at 11:11:09AM +0100, Andreas Schwab wrote:

> > On Nov 14 2018, Alexander Graf <agraf@suse.de> wrote:

> > 

> > > diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c

> > > index a4c6e8036..d8ebe648e 100644

> > > --- a/grub-core/loader/efi/fdt.c

> > > +++ b/grub-core/loader/efi/fdt.c

> > > @@ -123,7 +123,7 @@ grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)),

> > >        return GRUB_ERR_NONE;

> > >      }

> > >  

> > > -  dtb = grub_file_open (argv[0]);

> > > +  dtb = grub_file_open (argv[0], GRUB_FILE_TYPE_DEVICE_TREE);

> > >    if (!dtb)

> > >      goto out;

> > >  

> > 

> > Looks like this has been obsoleted by commit dfb1742aab?

> 

> Actually, that one does not do anything sensible in

> grub-core/commands/efi/shim_lock.c

> so that hunk would be worth considering including.

> 

> Treating DT and ACPI equally sounds sensible to me.


Yep, that make sense for me too.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
diff mbox series

Patch

diff --git a/grub-core/commands/efi/shim_lock.c b/grub-core/commands/efi/shim_lock.c
index 01246b0fc..90dccb0c7 100644
--- a/grub-core/commands/efi/shim_lock.c
+++ b/grub-core/commands/efi/shim_lock.c
@@ -81,6 +81,7 @@  shim_lock_init (grub_file_t io, enum grub_file_type type,
       /* Fall through. */
 
     case GRUB_FILE_TYPE_ACPI_TABLE:
+    case GRUB_FILE_TYPE_DEVICE_TREE:
       *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH;
 
       return GRUB_ERR_NONE;
diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c
index a4c6e8036..d8ebe648e 100644
--- a/grub-core/loader/efi/fdt.c
+++ b/grub-core/loader/efi/fdt.c
@@ -123,7 +123,7 @@  grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)),
       return GRUB_ERR_NONE;
     }
 
-  dtb = grub_file_open (argv[0]);
+  dtb = grub_file_open (argv[0], GRUB_FILE_TYPE_DEVICE_TREE);
   if (!dtb)
     goto out;
 
diff --git a/include/grub/file.h b/include/grub/file.h
index 19dda67f6..b8fb13017 100644
--- a/include/grub/file.h
+++ b/include/grub/file.h
@@ -93,6 +93,8 @@  enum grub_file_type
     GRUB_FILE_TYPE_FILE_ID,
     /* File holding ACPI table.  */
     GRUB_FILE_TYPE_ACPI_TABLE,
+    /* File holding Device Tree.  */
+    GRUB_FILE_TYPE_DEVICE_TREE,
     /* File we intend show to user.  */
     GRUB_FILE_TYPE_CAT,
     GRUB_FILE_TYPE_HEXCAT,