Re: [PATCH 2/6] ieee1275/powerpc: enables device mapper discovery
On Mon, May 08, 2023 at 01:58:36PM +, Avnish Chouhan wrote:
> From: Diego Domingos
>
> 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
> ---
> 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 0f5d54e9f2d..cc849d9c94c 100644
> --- a/grub-core/osdep/linux/ofpath.c
> +++ b/grub-core/osdep/linux/ofpath.c
> @@ -37,6 +37,7 @@
> #include
> #include
> #include
> +#include
>
> #ifdef __sparc__
> typedef enum
> @@ -755,13 +756,74 @@ strip_trailing_digits (const char *p)
> return new;
> }
>
> +static char *
> +get_slave_from_dm(const char * device){
> + char *curr_device, *tmp;
> + char *directory;
> + char *ret = NULL;
> +
> + directory = grub_strdup (device);
> + tmp = get_basename(directory);
> + curr_device = grub_strdup (tmp);
> + *tmp = '\0';
> +
> + /* Recursively check for slaves devices so we can find the root device */
> + while ((curr_device[0] == 'd') && (curr_device[1] == 'm') &&
> (curr_device[2] == '-')){
> +DIR *dp;
> +struct dirent *ep;
> +char* device_path;
> +
> +device_path = grub_xasprintf ("/sys/block/%s/slaves", curr_device);
> +dp = opendir(device_path);
> +free(device_path);
> +
> +if (dp != NULL)
> + {
> +ep = readdir (dp);
> +while (ep != NULL)
> + {
> + /* avoid some system directories */
> +if (!strcmp(ep->d_name,"."))
> + goto next_dir;
> +if (!strcmp(ep->d_name,".."))
> + goto next_dir;
> +
> + free (curr_device);
> + free (ret);
> + curr_device = grub_strdup (ep->d_name);
> + ret = grub_xasprintf ("%s%s", directory, curr_device);
> + break;
> +
> +next_dir:
> +ep = readdir (dp);
> +continue;
> + }
> +closedir (dp);
> + }
> +else
> + grub_util_warn (_("cannot open directory `%s'"), device_path);
This will lead to UAF (Use After Free) for the device_path pointer.
Thanks,
Micahel
> + }
> +
> + free (directory);
> + free (curr_device);
> +
> + return ret;
> +}
> +
> char *
> grub_util_devname_to_ofpath (const char *sys_devname)
> {
> - char *name_buf, *device, *devnode, *devicenode, *ofpath;
> + char *name_buf, *device, *devnode, *devicenode, *ofpath, *realname;
>
> name_buf = xrealpath (sys_devname);
>
> + realname = get_slave_from_dm (name_buf);
> + if (realname)
> +{
> + free (name_buf);
> + name_buf = realname;
> +}
> +
> device = get_basename (name_buf);
> devnode = strip_trailing_digits (name_buf);
> devicenode = strip_trailing_digits (device);
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 7/7] verifiers: Verify after decompression
On Wed, Mar 13, 2024 at 03:07:48PM +, Ross Lagerwall via Grub-devel wrote:
> It is convenient and common to have binaries stored in gzip archives
> (e.g. xen.gz). Verification should be run after decompression rather
> than before so reorder the file filter list as appropriate.
The proposed change would result in the disruption of the tpm and pgp
clients within the verifier framework. Specifically, the tpm pcr
prediction software relies on the integrity of raw files rather than
decompressed ones. Additionally, pgp detached signatures are designed to
target original files, thus necessitating the current structure to
maintain functionality.
Thanks,
Michael
>
> Signed-off-by: Ross Lagerwall
> ---
> include/grub/file.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/grub/file.h b/include/grub/file.h
> index a5bf3a792d6f..a1ef3582bc7b 100644
> --- a/include/grub/file.h
> +++ b/include/grub/file.h
> @@ -182,10 +182,10 @@ extern grub_disk_read_hook_t
> EXPORT_VAR(grub_file_progress_hook);
> /* Filters with lower ID are executed first. */
> typedef enum grub_file_filter_id
>{
> -GRUB_FILE_FILTER_VERIFY,
> GRUB_FILE_FILTER_GZIO,
> GRUB_FILE_FILTER_XZIO,
> GRUB_FILE_FILTER_LZOPIO,
> +GRUB_FILE_FILTER_VERIFY,
> GRUB_FILE_FILTER_MAX,
> GRUB_FILE_FILTER_COMPRESSION_FIRST = GRUB_FILE_FILTER_GZIO,
> GRUB_FILE_FILTER_COMPRESSION_LAST = GRUB_FILE_FILTER_LZOPIO,
> --
> 2.43.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/7] multiboot2: Add load type header and support for the PE binary type
On 14.03.2024 15:24, Ross Lagerwall wrote: > On Thu, Mar 14, 2024 at 1:37 PM Jan Beulich wrote: >> On 14.03.2024 10:30, Ross Lagerwall wrote: >>> On Thu, Mar 14, 2024 at 7:24 AM Jan Beulich wrote: On 13.03.2024 16:07, Ross Lagerwall wrote: > In addition to the existing address and ELF load types, specify a new > optional PE binary load type. This new type is a useful addition since > PE binaries can be signed and verified (i.e. used with Secure Boot). And the consideration to have ELF signable (by whatever extension to the ELF spec) went nowhere? >>> >>> I'm not sure if you're referring to some ongoing work to create signable >>> ELFs that I'm not aware of. >> >> Something must have been invented already to make Linux modules signable. > > Linux module signatures operate outside of the ELF container. In fact, > AFAIK the actual signed content could be anything. The file format is: > > * Content (i.e. ELF binary) > * Signature of content in PKCS7 format > * Signature info, including signature length > * Magic marker: "~Module signature appended~\n" > > This kind of arrangement does indeed work but it is fragile. Since the > signature is on the entire contents and tools that understand ELF don't > parse the signature, any transformation of the binary (e.g. to > strip out debuginfo) will cause the signature to be lost / invalidated. This looks extremely poor to me, so ... > Nevertheless, this could still be an option for Xen if this is > deemed to be a preferred solution by others. It would be good to hear > some opinions on this. ... I'd rather not see this used for Xen. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/7] multiboot2: Add load type header and support for the PE binary type
On Thu, Mar 14, 2024 at 1:37 PM Jan Beulich wrote: > > On 14.03.2024 10:30, Ross Lagerwall wrote: > > On Thu, Mar 14, 2024 at 7:24 AM Jan Beulich wrote: > >> > >> On 13.03.2024 16:07, Ross Lagerwall wrote: > >>> In addition to the existing address and ELF load types, specify a new > >>> optional PE binary load type. This new type is a useful addition since > >>> PE binaries can be signed and verified (i.e. used with Secure Boot). > >> > >> And the consideration to have ELF signable (by whatever extension to > >> the ELF spec) went nowhere? > >> > > > > I'm not sure if you're referring to some ongoing work to create signable > > ELFs that I'm not aware of. > > Something must have been invented already to make Linux modules signable. Linux module signatures operate outside of the ELF container. In fact, AFAIK the actual signed content could be anything. The file format is: * Content (i.e. ELF binary) * Signature of content in PKCS7 format * Signature info, including signature length * Magic marker: "~Module signature appended~\n" This kind of arrangement does indeed work but it is fragile. Since the signature is on the entire contents and tools that understand ELF don't parse the signature, any transformation of the binary (e.g. to strip out debuginfo) will cause the signature to be lost / invalidated. Nevertheless, this could still be an option for Xen if this is deemed to be a preferred solution by others. It would be good to hear some opinions on this. > > > I didn't choose that route because: > > > > * Signed PE binaries are the current standard for Secure Boot. > > > > * Having signed ELF binaries would mean that code to handle them needs > > to be added to Shim which contravenes its goals of being small and > > simple to verify. > > Both true, but neither goes entirely without saying, I suppose. > > > * I could be wrong on this but to my knowledge, the ELF format is not > > being actively updated nor is the standard owned/maintained by a > > specific group which makes updating it difficult. > > And PE/COFF isn't under control of a public entity / group afaik, which > may be viewed as no better, if not worse. Agreed, I guess my point was that PE/COFF doesn't need to be updated to support signing because it already supports it. Thanks, Ross ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/7] multiboot2: Add load type header and support for the PE binary type
On 14.03.2024 10:30, Ross Lagerwall wrote: > On Thu, Mar 14, 2024 at 7:24 AM Jan Beulich wrote: >> >> On 13.03.2024 16:07, Ross Lagerwall wrote: >>> In addition to the existing address and ELF load types, specify a new >>> optional PE binary load type. This new type is a useful addition since >>> PE binaries can be signed and verified (i.e. used with Secure Boot). >> >> And the consideration to have ELF signable (by whatever extension to >> the ELF spec) went nowhere? >> > > I'm not sure if you're referring to some ongoing work to create signable > ELFs that I'm not aware of. Something must have been invented already to make Linux modules signable. > I didn't choose that route because: > > * Signed PE binaries are the current standard for Secure Boot. > > * Having signed ELF binaries would mean that code to handle them needs > to be added to Shim which contravenes its goals of being small and > simple to verify. Both true, but neither goes entirely without saying, I suppose. > * I could be wrong on this but to my knowledge, the ELF format is not > being actively updated nor is the standard owned/maintained by a > specific group which makes updating it difficult. And PE/COFF isn't under control of a public entity / group afaik, which may be viewed as no better, if not worse. > * Tools would need to be updated/developed to add support for signing > ELF binaries and inspecting the signatures. As above, yes indeed. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/7] multiboot2: Add load type header and support for the PE binary type
On Thu, Mar 14, 2024 at 7:24 AM Jan Beulich wrote: > > On 13.03.2024 16:07, Ross Lagerwall wrote: > > In addition to the existing address and ELF load types, specify a new > > optional PE binary load type. This new type is a useful addition since > > PE binaries can be signed and verified (i.e. used with Secure Boot). > > And the consideration to have ELF signable (by whatever extension to > the ELF spec) went nowhere? > I'm not sure if you're referring to some ongoing work to create signable ELFs that I'm not aware of. I didn't choose that route because: * Signed PE binaries are the current standard for Secure Boot. * Having signed ELF binaries would mean that code to handle them needs to be added to Shim which contravenes its goals of being small and simple to verify. * I could be wrong on this but to my knowledge, the ELF format is not being actively updated nor is the standard owned/maintained by a specific group which makes updating it difficult. * Tools would need to be updated/developed to add support for signing ELF binaries and inspecting the signatures. I am open to suggestions of course but I'm not sure what benefits there would be to going the ELF route. Ross ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Support dropin files for Linux kernel parameters
Ping?
On 01/03/2024, 15:43, "Simon Rowe" wrote:
Kernel parameters actually cover a range of purposes, including
userspace like systemd. They also need setting for a variety of
reasons:
* as distro defaults
* to provide configuration for a package
* for an admin to set desired behaviour
Having these all combined in a single line (like GRUB_CMDLINE_LINUX)
is unwieldy, it is hard to make changes without impacting another
usecase.
Add optional support for dropin files in the directories:
* /usr/lib/kernel.d/
* /etc/kernel.d/
where the contents of each file with the '.conf' suffix is evaluated
(excluding comments) and appended to any other kernel parameters
defined via GRUB_CMDLINE_LINUX etc. Files in /etc/kernel.d/ completely
replace those of the same name in /usr/lib/kernel.d/. This allows a
distro or installer to set parameters but then for an admin to
override them.
Signed-off-by: Simon Rowe
---
util/grub-mkconfig_lib.in | 18 ++
util/grub.d/10_linux.in | 8 +---
util/grub.d/20_linux_xen.in | 8 +---
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
index 08953287c..7aaa747f1 100644
--- a/util/grub-mkconfig_lib.in
+++ b/util/grub-mkconfig_lib.in
@@ -348,3 +348,21 @@ grub_add_tab () {
sed -e "s/^/$grub_tab/"
}
+kernel_params_from_files () {
+ # Read Linux kernel parameters from dropin files.
+
+ file_bases=""
+
+ for f in /etc/kernel.d/*.conf /usr/lib/kernel.d/*.conf; do
+[ -r $f ] || continue
+file_bases="$file_bases $(basename $f)"
+ done
+
+ for b in $(echo $file_bases | tr ' ' '\n' | sort -u); do
+if [ -r /etc/kernel.d/$b ]; then
+ grep -v '^#' /etc/kernel.d/$b | tr '\n' ' '
+elif [ -r /usr/lib/kernel.d/$b ]; then
+ grep -v '^#' /usr/lib/kernel.d/$b | tr '\n' ' '
+fi
+ done
+}
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index cc393be7e..fba3775f9 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -275,6 +275,8 @@ for linux in ${reverse_sorted_list}; do
fi
fi
+ extra_kernel_params=$(kernel_params_from_files)
+
# The GRUB_DISABLE_SUBMENU option used to be different than others since it
was
# mentioned in the documentation that has to be set to 'y' instead of 'true'
to
# enable it. This caused a lot of confusion to users that set the option to
'y',
@@ -285,7 +287,7 @@ for linux in ${reverse_sorted_list}; do
if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" != xtrue ];
then
linux_entry "${OS}" "${version}" simple \
-"${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
+"${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}
${extra_kernel_params}"
submenu_indentation="$grub_tab"
@@ -298,10 +300,10 @@ for linux in ${reverse_sorted_list}; do
fi
linux_entry "${OS}" "${version}" advanced \
- "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
+ "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}
${extra_kernel_params}"
if [ "x${GRUB_DISABLE_RECOVERY}" != "xtrue" ]; then
linux_entry "${OS}" "${version}" recovery \
-"${GRUB_CMDLINE_LINUX_RECOVERY} ${GRUB_CMDLINE_LINUX}"
+"${GRUB_CMDLINE_LINUX_RECOVERY} ${GRUB_CMDLINE_LINUX}
${extra_kernel_params}"
fi
done
diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index 94dd8be13..089f6de43 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -336,6 +336,8 @@ for current_xen in ${reverse_sorted_xen_list}; do
fi
fi
+ extra_kernel_params=$(kernel_params_from_files)
+
# The GRUB_DISABLE_SUBMENU option used to be different than others
since it was
# mentioned in the documentation that has to be set to 'y' instead of
'true' to
# enable it. This caused a lot of confusion to users that set the
option to 'y',
@@ -346,7 +348,7 @@ for current_xen in ${reverse_sorted_xen_list}; do
if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" !=
xtrue ]; then
linux_entry "${OS}" "${version}" "${xen_version}" simple \
- "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
"${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT}"
+ "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
"${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT} ${extra_kernel_params}"
submenu_indentation="$grub_tab$grub_tab"
@@ -360,10 +362,10 @@ for current_xen in ${reverse_sorted_xen_list}; do
fi
linux_entry "${OS}" "${version}" "${xen_version}" advanced \
- "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
"${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT}"
+ "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
"${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT} ${extra_kernel_params}"
if [ "x${GRUB_DISABLE_RECOVERY}" != "xtrue" ]; then
linux_entry
Re: [PATCH 1/7] multiboot2: Add load type header and support for the PE binary type
Please take this discussion to a separate thread. In this thread it's pure offtopic On Thu, Mar 14, 2024 at 10:25 AM Jan Beulich via Grub-devel wrote: > > On 13.03.2024 16:07, Ross Lagerwall wrote: > > In addition to the existing address and ELF load types, specify a new > > optional PE binary load type. This new type is a useful addition since > > PE binaries can be signed and verified (i.e. used with Secure Boot). > > And the consideration to have ELF signable (by whatever extension to > the ELF spec) went nowhere? > > Jan > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel -- Regards Vladimir 'phcoder' Serbinenko ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/7] multiboot2: Add load type header and support for the PE binary type
Hi, I saw this on the list and have concerns: Original Message On 14 Mar 2024, 6:24 pm, Jan Beulich via Grub-devel < grub-devel@gnu.org> wrote: On 13.03.2024 16:07, Ross Lagerwall wrote: >> In addition to the existing address and ELF load types, specify a new >> optional PE binary load type. This new type is a useful addition since >> PE binaries can be signed and verified (i.e. used with Secure Boot). > And the consideration to have ELF signable (by whatever extension to the ELF > spec) went nowhere? Jan If the purpose of signing binaries is to prevent their execution unless they are signed by their owner, this is MALWARE unless the end user can replace the keys with one of their choosing. Adding a field to elf to provide this feature is IMHO asking for trouble because the key is stored elsewhere and there is nothing to prevent abuse of this field to deny users their freedom to run code, (ie by not providing them the key or a guaranteed mechanism for providing their own). On that note, why is it such a useful feature to restrict the freedom to run code in grub? If grub selects malware to execute, the user must have chosen to run it - or grub itself is compromised? Do you think that locking binaries down is the future for users to ensure their own security or it is acceptable for 3rd parties to hide platform keys to lock all systems down, even by binary? I'm not convinced. Damien Zammit GNU/Hurd hacker___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/7] multiboot2: Add load type header and support for the PE binary type
On 13.03.2024 16:07, Ross Lagerwall wrote: > In addition to the existing address and ELF load types, specify a new > optional PE binary load type. This new type is a useful addition since > PE binaries can be signed and verified (i.e. used with Secure Boot). And the consideration to have ELF signable (by whatever extension to the ELF spec) went nowhere? Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
