Re: [PATCH] Generalize MD_MAX_DISKS to GRUB_MDRAID_MAX_DISKS
On Tue, Jun 13, 2023 at 12:33:36PM -0700, Kees Cook wrote: > On Tue, Jun 13, 2023 at 02:54:48PM +0200, Julian Andres Klode wrote: > > Move the constant from getroot.c to disk.h and then reuse > > it place of the hardcoded 1024 limit in diskfilter. > > > > Fixes: 2a5e3c1f2 (disk/diskfilter: Don't make a RAID array with more than > > 1024 disks) > > Cc: Daniel Axtens > > Cc: Kees Cook > > Yup, looks good to me. Thanks! > > Reviewed-by: Kees Cook Reviewed-by: Daniel Kiper Thank you for fixing this issue. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Generalize MD_MAX_DISKS to GRUB_MDRAID_MAX_DISKS
On Tue, Jun 13, 2023 at 02:54:48PM +0200, Julian Andres Klode wrote:
> Move the constant from getroot.c to disk.h and then reuse
> it place of the hardcoded 1024 limit in diskfilter.
>
> Fixes: 2a5e3c1f2 (disk/diskfilter: Don't make a RAID array with more than
> 1024 disks)
> Cc: Daniel Axtens
> Cc: Kees Cook
Yup, looks good to me. Thanks!
Reviewed-by: Kees Cook
-Kees
> ---
> grub-core/disk/diskfilter.c | 4 ++--
> grub-core/osdep/linux/getroot.c | 12 ++--
> include/grub/disk.h | 10 ++
> 3 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
> index 1c568927b..61a311efd 100644
> --- a/grub-core/disk/diskfilter.c
> +++ b/grub-core/disk/diskfilter.c
> @@ -1046,8 +1046,8 @@ grub_diskfilter_make_raid (grub_size_t uuidlen, char
> *uuid, int nmemb,
>struct grub_diskfilter_pv *pv;
>grub_err_t err;
>
> - /* We choose not to support more than 1024 disks. */
> - if (nmemb < 1 || nmemb > 1024)
> + /* We choose not to support more than the specified number of disks. */
> + if (nmemb < 1 || nmemb > GRUB_MDRAID_MAX_DISKS)
> {
>grub_free (uuid);
>return NULL;
> diff --git a/grub-core/osdep/linux/getroot.c b/grub-core/osdep/linux/getroot.c
> index 9cf037ec2..7dd775d2a 100644
> --- a/grub-core/osdep/linux/getroot.c
> +++ b/grub-core/osdep/linux/getroot.c
> @@ -44,6 +44,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -130,15 +131,6 @@ struct mountinfo_entry
>char fstype[ESCAPED_PATH_MAX + 1], device[ESCAPED_PATH_MAX + 1];
> };
>
> -/*
> - * GET_DISK_INFO nr_disks (total count) does not map to disk.number,
> - * which is an internal kernel index. Instead, do what mdadm does
> - * and keep scanning until we find enough valid disks. The limit is
> - * copied from there, which notes that it is sufficiently high given
> - * that the on-disk metadata for v1.x can only support 1920.
> - */
> -#define MD_MAX_DISKS 4096
> -
> static char **
> grub_util_raid_getmembers (const char *name, int bootable)
> {
> @@ -176,7 +168,7 @@ grub_util_raid_getmembers (const char *name, int bootable)
>devicelist = xcalloc (info.nr_disks + 1, sizeof (char *));
>
>remaining = info.nr_disks;
> - for (i = 0, j = 0; i < MD_MAX_DISKS && remaining > 0; i++)
> + for (i = 0, j = 0; i < GRUB_MDRAID_MAX_DISKS && remaining > 0; i++)
> {
>disk.number = i;
>ret = ioctl (fd, GET_DISK_INFO, );
> diff --git a/include/grub/disk.h b/include/grub/disk.h
> index 071b2f7df..cbd05e9c7 100644
> --- a/include/grub/disk.h
> +++ b/include/grub/disk.h
> @@ -172,6 +172,16 @@ typedef struct grub_disk_memberlist
> *grub_disk_memberlist_t;
> /* The maximum number of disk caches. */
> #define GRUB_DISK_CACHE_NUM 1021
>
> +/* The maximum number of disks in an mdraid device.
> + *
> + * GET_DISK_INFO nr_disks (total count) does not map to disk.number,
> + * which is an internal kernel index. Instead, do what mdadm does
> + * and keep scanning until we find enough valid disks. The limit is
> + * copied from there, which notes that it is sufficiently high given
> + * that the on-disk metadata for v1.x can only support 1920.
> + */
> +#define GRUB_MDRAID_MAX_DISKS4096
> +
> /* The size of a disk cache in 512B units. Must be at least as big as the
> largest supported sector size, currently 16K. */
> #define GRUB_DISK_CACHE_BITS 6
> --
> 2.40.1
>
--
Kees Cook
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] Generalize MD_MAX_DISKS to GRUB_MDRAID_MAX_DISKS
Move the constant from getroot.c to disk.h and then reuse
it place of the hardcoded 1024 limit in diskfilter.
Fixes: 2a5e3c1f2 (disk/diskfilter: Don't make a RAID array with more than 1024
disks)
Cc: Daniel Axtens
Cc: Kees Cook
---
grub-core/disk/diskfilter.c | 4 ++--
grub-core/osdep/linux/getroot.c | 12 ++--
include/grub/disk.h | 10 ++
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 1c568927b..61a311efd 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -1046,8 +1046,8 @@ grub_diskfilter_make_raid (grub_size_t uuidlen, char
*uuid, int nmemb,
struct grub_diskfilter_pv *pv;
grub_err_t err;
- /* We choose not to support more than 1024 disks. */
- if (nmemb < 1 || nmemb > 1024)
+ /* We choose not to support more than the specified number of disks. */
+ if (nmemb < 1 || nmemb > GRUB_MDRAID_MAX_DISKS)
{
grub_free (uuid);
return NULL;
diff --git a/grub-core/osdep/linux/getroot.c b/grub-core/osdep/linux/getroot.c
index 9cf037ec2..7dd775d2a 100644
--- a/grub-core/osdep/linux/getroot.c
+++ b/grub-core/osdep/linux/getroot.c
@@ -44,6 +44,7 @@
#include
#include
+#include
#include
#include
#include
@@ -130,15 +131,6 @@ struct mountinfo_entry
char fstype[ESCAPED_PATH_MAX + 1], device[ESCAPED_PATH_MAX + 1];
};
-/*
- * GET_DISK_INFO nr_disks (total count) does not map to disk.number,
- * which is an internal kernel index. Instead, do what mdadm does
- * and keep scanning until we find enough valid disks. The limit is
- * copied from there, which notes that it is sufficiently high given
- * that the on-disk metadata for v1.x can only support 1920.
- */
-#define MD_MAX_DISKS 4096
-
static char **
grub_util_raid_getmembers (const char *name, int bootable)
{
@@ -176,7 +168,7 @@ grub_util_raid_getmembers (const char *name, int bootable)
devicelist = xcalloc (info.nr_disks + 1, sizeof (char *));
remaining = info.nr_disks;
- for (i = 0, j = 0; i < MD_MAX_DISKS && remaining > 0; i++)
+ for (i = 0, j = 0; i < GRUB_MDRAID_MAX_DISKS && remaining > 0; i++)
{
disk.number = i;
ret = ioctl (fd, GET_DISK_INFO, );
diff --git a/include/grub/disk.h b/include/grub/disk.h
index 071b2f7df..cbd05e9c7 100644
--- a/include/grub/disk.h
+++ b/include/grub/disk.h
@@ -172,6 +172,16 @@ typedef struct grub_disk_memberlist
*grub_disk_memberlist_t;
/* The maximum number of disk caches. */
#define GRUB_DISK_CACHE_NUM1021
+/* The maximum number of disks in an mdraid device.
+ *
+ * GET_DISK_INFO nr_disks (total count) does not map to disk.number,
+ * which is an internal kernel index. Instead, do what mdadm does
+ * and keep scanning until we find enough valid disks. The limit is
+ * copied from there, which notes that it is sufficiently high given
+ * that the on-disk metadata for v1.x can only support 1920.
+ */
+#define GRUB_MDRAID_MAX_DISKS 4096
+
/* The size of a disk cache in 512B units. Must be at least as big as the
largest supported sector size, currently 16K. */
#define GRUB_DISK_CACHE_BITS 6
--
2.40.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
