Re: [PATCH] minix: avoid mistakenly probing ext2 filesystems

[Daniel Kiper]

On Fri, Mar 12, 2021 at 12:05:08PM -0600, Derek Foreman wrote:
> From: Daniel Drake <drake@endlessm.com>
>
> ext2 (and ext3, ext4) filesystems write the number of free inodes to
> location 0x410.
>
> On a minix filesystem, that same location is used for the minix superblock
> magic number.
>
> If the number of free inodes on an ext2 filesystem is equal to any
> of the four minix superblock magic values plus any multiple of 65536,
> grub's minix filesystem code will probe it as a minix filesystem.
>
> In the case of an OS using ext2 as the root filesystem, since there will
> ordinarily be some amount of file creation and deletion on every bootup,
> it effectively means that this situation has a 1:16384 chance of being hit
> on every reboot.
>
> This will cause grub's filesystem probing code to mistakenly identify an
> ext2 filesystem as minix. This can be seen by e.g. "search --label"
> incorrectly indicating that no such ext2 partition with matching label
> exists, whereas in fact it does.
>
> After spotting the rough cause of the issue I was facing here, I borrowed
> much of the diagnosis/explanation from meierfra who found and investigated
> the same issue in util-linux in 2010:
>
> https://bugs.launchpad.net/ubuntu/+source/util-linux/+bug/518582
>
> This was fixed in util-linux by having the minix code check for the
> ext2 magic. Do the same here.
>
> Signed-off-by: Daniel Drake <drake@endlessm.com>
> Reviewed-by: Derek Foreman <derek@endlessos.org>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

> ---
>
> This bug fix was previously sent to the grub-devel list once before:
>  https://lists.gnu.org/archive/html/grub-devel/2020-05/msg00205.html
> but received no response, so I'm bring it up again.
>
> If my understanding is correct, the bytes in question overlap with the
> "maximum file system size" field in the minix superblock, which will
> never contain the ext2 magic byte pattern, so there shouldn't be any
> unintended side effects.
>
>  grub-core/fs/minix.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/fs/minix.c b/grub-core/fs/minix.c
> index d0d08363c..db0a83feb 100644
> --- a/grub-core/fs/minix.c
> +++ b/grub-core/fs/minix.c
> @@ -38,6 +38,8 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  #define GRUB_MINIX_MAGIC_30  0x138F
>  #endif
>
> +#define      EXT2_MAGIC              0xEF53
> +
>  #define GRUB_MINIX_INODE_DIR_BLOCKS  7
>  #define GRUB_MINIX_LOG2_BSIZE        1
>  #define GRUB_MINIX_ROOT_INODE        1
> @@ -466,7 +468,22 @@ grub_minix_find_file (struct grub_minix_data *data, 
> const char *path)
>  static struct grub_minix_data *
>  grub_minix_mount (grub_disk_t disk)
>  {
> -  struct grub_minix_data *data;
> +  struct grub_minix_data *data = NULL;
> +  grub_uint16_t ext2_marker;
> +
> +  grub_disk_read (disk, 1 * 2, 56, sizeof (ext2_marker),
> +                  &ext2_marker);
> +  if (grub_errno)
> +    goto fail;
> +
> +  /* ext2 filesystems can sometimes be mistakenly identified
> +   * as minix, e.g. due to the number of free ext2 inodes being
> +   * written to the same location where the minix superblock
> +   * magic is found.
> +   * Avoid such situations by skipping any filesystems that
> +   * have the ext2 superblock magic. */
> +  if (ext2_marker == grub_cpu_to_le16_compile_time (EXT2_MAGIC))
> +    goto fail;
>
>    data = grub_malloc (sizeof (struct grub_minix_data));
>    if (!data)
> --
> 2.26.2
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel