Re: [PATCH v5 1/2] Warn if MBR gap is small and user uses advanced modules

[Vladimir 'phcoder' Serbinenko]

Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>

On Fri, Nov 13, 2020 at 9:27 PM Vladimir 'phcoder' Serbinenko
<phcoder@gmail.com> wrote:
>
> From 515ee70c0a289624f71f43cbd09be2a1b929f574 Mon Sep 17 00:00:00 2001
> From: Vladimir Serbinenko <phcoder@gmail.com>
> Date: Mon, 27 Apr 2020 17:50:04 +0200
> Subject: [PATCH 1/2] Warn if MBR gap is small and user uses advanced modules
>
> We don't want to support small MBR gap in pair with anything but
> the simplest config of biosdisk+part_msdos+simple filesystem. In this
> path "simple filesystems" are all current filesystems except zfs and
> btrfs.
> ---
>  grub-core/partmap/gpt.c     |  9 ++++++++-
>  grub-core/partmap/msdos.c   |  7 ++++++-
>  include/grub/partition.h    |  4 +++-
>  include/grub/util/install.h |  7 +++++--
>  util/grub-install-common.c  | 24 ++++++++++++++++++++++++
>  util/grub-install.c         | 10 +++++++---
>  util/grub-setup.c           |  2 +-
>  util/setup.c                |  5 +++--
>  8 files changed, 57 insertions(+), 11 deletions(-)
>
> diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> index 72a2e37cd..20cbcc7ff 100644
> --- a/grub-core/partmap/gpt.c
> +++ b/grub-core/partmap/gpt.c
> @@ -25,6 +25,9 @@
>  #include <grub/msdos_partition.h>
>  #include <grub/gpt_partition.h>
>  #include <grub/i18n.h>
> +#ifdef GRUB_UTIL
> +#include <grub/emu/misc.h>
> +#endif
>
>  GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -169,7 +172,8 @@ static grub_err_t
>  gpt_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
>   unsigned int max_nsectors,
>   grub_embed_type_t embed_type,
> - grub_disk_addr_t **sectors)
> + grub_disk_addr_t **sectors,
> + int warn_short)
>  {
>    struct gpt_partition_map_embed_ctx ctx = {
>      .start = 0,
> @@ -191,6 +195,9 @@ gpt_partition_map_embed (struct grub_disk *disk,
> unsigned int *nsectors,
>          N_("this GPT partition label contains no BIOS Boot Partition;"
>     " embedding won't be possible"));
>
> +  if (ctx.len < GRUB_MIN_RECOMMENDED_MBRGAP) {
> +    grub_util_warn("Your BIOS Boot Partition is under 1 MiB, please
> increase its size.");
> +  }
>    if (ctx.len < *nsectors)
>      return grub_error (GRUB_ERR_OUT_OF_RANGE,
>          N_("your BIOS Boot Partition is too small;"
> diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
> index ee3f24982..1483413b2 100644
> --- a/grub-core/partmap/msdos.c
> +++ b/grub-core/partmap/msdos.c
> @@ -236,7 +236,8 @@ static grub_err_t
>  pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
>   unsigned int max_nsectors,
>   grub_embed_type_t embed_type,
> - grub_disk_addr_t **sectors)
> + grub_disk_addr_t **sectors,
> + int warn_short)
>  {
>    grub_disk_addr_t end = ~0ULL;
>    struct grub_msdos_partition_mbr mbr;
> @@ -390,6 +391,10 @@ pc_partition_map_embed (struct grub_disk *disk,
> unsigned int *nsectors,
>        return GRUB_ERR_NONE;
>      }
>
> +  if (end < GRUB_MIN_RECOMMENDED_MBRGAP && warn_short) {
> +    grub_util_warn("You have a short MBR gap and use advanced config.
> Please increase post-MBR gap");
> +  }
> +
>    if (end <= 1)
>      return grub_error (GRUB_ERR_FILE_NOT_FOUND,
>          N_("this msdos-style partition label has no "
> diff --git a/include/grub/partition.h b/include/grub/partition.h
> index 7adb7ec6e..adc50d680 100644
> --- a/include/grub/partition.h
> +++ b/include/grub/partition.h
> @@ -52,10 +52,12 @@ struct grub_partition_map
>   grub_partition_iterate_hook_t hook, void *hook_data);
>  #ifdef GRUB_UTIL
>    /* Determine sectors available for embedding.  */
> +#define GRUB_MIN_RECOMMENDED_MBRGAP 1900
>    grub_err_t (*embed) (struct grub_disk *disk, unsigned int *nsectors,
>          unsigned int max_nsectors,
>          grub_embed_type_t embed_type,
> -        grub_disk_addr_t **sectors);
> +        grub_disk_addr_t **sectors,
> +        int warn_short);
>  #endif
>  };
>  typedef struct grub_partition_map *grub_partition_map_t;
> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> index 2631b1074..640c71d28 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -193,13 +193,13 @@ grub_util_bios_setup (const char *dir,
>         const char *boot_file, const char *core_file,
>         const char *dest, int force,
>         int fs_probe, int allow_floppy,
> -       int add_rs_codes);
> +       int add_rs_codes, int warn_short_mbr_gap);
>  void
>  grub_util_sparc_setup (const char *dir,
>          const char *boot_file, const char *core_file,
>          const char *dest, int force,
>          int fs_probe, int allow_floppy,
> -        int add_rs_codes);
> +        int add_rs_codes, int warn_short_mbr_gap);
>
>  char *
>  grub_install_get_image_targets_string (void);
> @@ -265,4 +265,7 @@ grub_util_get_target_name (const struct
> grub_install_image_target_desc *t);
>  extern char *grub_install_copy_buffer;
>  #define GRUB_INSTALL_COPY_BUFFER_SIZE 1048576
>
> +int
> +grub_install_is_short_mbrgap_supported(void);
> +
>  #endif
> diff --git a/util/grub-install-common.c b/util/grub-install-common.c
> index 277eaf4e2..06cb74646 100644
> --- a/util/grub-install-common.c
> +++ b/util/grub-install-common.c
> @@ -234,6 +234,30 @@ char *grub_install_source_directory = NULL;
>  char *grub_install_locale_directory = NULL;
>  char *grub_install_themes_directory = NULL;
>
> +int
> +grub_install_is_short_mbrgap_supported()
> +{
> +  int i, j;
> +  static const char *whitelist[] =
> +    {
> +     "part_msdos", "biosdisk", "affs", "afs", "bfs", "archelp",
> +     "cpio", "cpio_be", "newc", "odc", "ext2", "fat", "exfat",
> +     "f2fs", "fshelp", "hfs", "hfsplus",
> +     "iso9660", "jfs", "minix", "minix2", "minix3", "minix_be",
> +     "minix2_be", "minix2_be", "nilfs2", "ntfs", "ntfscomp",
> +     "reiserfs", "romfs", "sfs", "tar", "udf",
> +     "ufs1", "ufs1_be", "ufs2", "xfs"
> +    };
> +  for (i = 0; i < modules.n_entries; i++) {
> +    for (j = 0; j < ARRAY_SIZE (whitelist); j++)
> +      if (strcmp(modules.entries[i], whitelist[j]) == 0)
> + break;
> +    if (j == ARRAY_SIZE (whitelist))
> +      return 0;
> +  }
> +  return 1;
> +}
> +
>  void
>  grub_install_push_module (const char *val)
>  {
> diff --git a/util/grub-install.c b/util/grub-install.c
> index a35a2e2e8..c123319c3 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -1718,10 +1718,14 @@ main (int argc, char *argv[])
>   install_device);
>
>   /*  Now perform the installation.  */
> - if (install_bootsector)
> + if (install_bootsector) {
> +   int warn_short_mbr_gap = !grub_install_is_short_mbrgap_supported();
> +
>     grub_util_bios_setup (platdir, "boot.img", "core.img",
>   install_drive, force,
> - fs_probe, allow_floppy, add_rs_codes);
> + fs_probe, allow_floppy, add_rs_codes,
> + warn_short_mbr_gap);
> + }
>   break;
>        }
>      case GRUB_INSTALL_PLATFORM_SPARC64_IEEE1275:
> @@ -1748,7 +1752,7 @@ main (int argc, char *argv[])
>     grub_util_sparc_setup (platdir, "boot.img", "core.img",
>   install_drive, force,
>   fs_probe, allow_floppy,
> - 0 /* unused */ );
> + 0 /* unused */, 0 /* unused */ );
>   break;
>        }
>
> diff --git a/util/grub-setup.c b/util/grub-setup.c
> index 42b98ad3c..1783224dd 100644
> --- a/util/grub-setup.c
> +++ b/util/grub-setup.c
> @@ -315,7 +315,7 @@ main (int argc, char *argv[])
>      arguments.core_file ? : DEFAULT_CORE_FILE,
>      dest_dev, arguments.force,
>      arguments.fs_probe, arguments.allow_floppy,
> -    arguments.add_rs_codes);
> +    arguments.add_rs_codes, 0);
>
>    /* Free resources.  */
>    grub_fini_all ();
> diff --git a/util/setup.c b/util/setup.c
> index 3be88aae1..da5f2c07f 100644
> --- a/util/setup.c
> +++ b/util/setup.c
> @@ -254,7 +254,8 @@ SETUP (const char *dir,
>         const char *boot_file, const char *core_file,
>         const char *dest, int force,
>         int fs_probe, int allow_floppy,
> -       int add_rs_codes __attribute__ ((unused))) /* unused on sparc64 */
> +       int add_rs_codes __attribute__ ((unused)), /* unused on sparc64 */
> +       int warn_small)
>  {
>    char *core_path;
>    char *boot_img, *core_img, *boot_path;
> @@ -530,7 +531,7 @@ SETUP (const char *dir,
>   GRUB_EMBED_PCBIOS, &sectors);
>      else if (ctx.dest_partmap)
>        err = ctx.dest_partmap->embed (dest_dev->disk, &nsec, maxsec,
> -      GRUB_EMBED_PCBIOS, &sectors);
> +      GRUB_EMBED_PCBIOS, &sectors, warn_small);
>      else
>        err = fs->fs_embed (dest_dev, &nsec, maxsec,
>     GRUB_EMBED_PCBIOS, &sectors);
> --
> 2.20.1
>
>
> --
> Regards
> Vladimir 'phcoder' Serbinenko



-- 
Regards
Vladimir 'phcoder' Serbinenko