All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>
To: grub-devel@gnu.org
Subject: Re: [PATCH] Improve ext2 driver to allow embedding of the boot loader code.
Date: Thu, 09 Jan 2014 23:52:28 +0100	[thread overview]
Message-ID: <52CF282C.4070408@gmail.com> (raw)
In-Reply-To: <1389301657-8236-1-git-send-email-tilmann@bubecks.de>

[-- Attachment #1: Type: text/plain, Size: 11425 bytes --]

On 09.01.2014 22:07, Dr. Tilmann Bubeck wrote:
> This patch extends the ext2 driver to allow embedding core.img into the
> filesystem. This allows the long needed installation of GRUB into a partition
> without the need to use (unsafe) block lists.
> 
> It is realized using the ioctl(EXT4_IOC_SWAP_BOOT) introduced into
> Linux 3.10. This ioctl basically swaps the data blocks of the associated
> file with the EXT2_BOOT_LOADER_INO inode. After using the ioctl the code
> of core.img is stored in the BOOT_LOADER incode of the filesystem and it
> is not accessible to file system tools anymore. This ensures, that the boot
> code is safe and installation into a partition is possible.
> 
I've already commented on this design here: it not only doesn't
guarantee any kind of blockstability that we need but guarantees quite
the opposite in several scenarios (detailed in previous mails).
> The patchs needs 0001-Refactor-grub_read_mountinfo-out-of-grub_find_root_d.patch
> to work correctly.
> 
> Signed-off-by: Dr. Tilmann Bubeck <tilmann@bubecks.de>
> ---
>  grub-core/fs/ext2.c | 212 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  util/setup.c        |  11 ++-
>  2 files changed, 215 insertions(+), 8 deletions(-)
> 
> diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
> index 5f7a2b9..643969e 100644
> --- a/grub-core/fs/ext2.c
> +++ b/grub-core/fs/ext2.c
> @@ -133,6 +133,14 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  
>  #define EXT4_EXTENTS_FLAG		0x80000
>  
> +/*
> + * Special inodes numbers
> + */
> +#define EXT2_ROOT_INO            2      /* Root inode */
> +#define EXT2_BOOT_LOADER_INO     5      /* Boot loader inode */
> +
> +#define EXT4_IOC_SWAP_BOOT		_IO('f', 17)
> +
>  /* The ext2 superblock.  */
>  struct grub_ext2_sblock
>  {
> @@ -393,10 +401,10 @@ grub_ext4_find_leaf (struct grub_ext2_data *data,
>  }
>  
>  static grub_disk_addr_t
> -grub_ext2_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
> +grub_ext2_read_block2 (struct grub_ext2_data *data,
> +		       struct grub_ext2_inode *inode,
> +		       grub_disk_addr_t fileblock)
>  {
> -  struct grub_ext2_data *data = node->data;
> -  struct grub_ext2_inode *inode = &node->inode;
>    unsigned int blksz = EXT2_BLOCK_SIZE (data);
>    grub_disk_addr_t blksz_quarter = blksz / 4;
>    int log2_blksz = LOG2_EXT2_BLOCK_SIZE (data);
> @@ -497,6 +505,12 @@ indirect:
>    return grub_le_to_cpu32 (indir);
>  }
>  
> +static grub_disk_addr_t
> +grub_ext2_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
> +{
> +  return grub_ext2_read_block2(node->data, &node->inode, fileblock);
> +}
> +
>  /* Read LEN bytes from the file described by DATA starting with byte
>     POS.  Return the amount of read bytes in READ.  */
>  static grub_ssize_t
> @@ -607,12 +621,12 @@ grub_ext2_mount (grub_disk_t disk)
>    data->disk = disk;
>  
>    data->diropen.data = data;
> -  data->diropen.ino = 2;
> +  data->diropen.ino = EXT2_ROOT_INO;
>    data->diropen.inode_read = 1;
>  
>    data->inode = &data->diropen.inode;
>  
> -  grub_ext2_read_inode (data, 2, data->inode);
> +  grub_ext2_read_inode (data, EXT2_ROOT_INO, data->inode);
>    if (grub_errno)
>      goto fail;
>  
> @@ -977,6 +991,193 @@ grub_ext2_mtime (grub_device_t device, grub_int32_t *tm)
>  
>  }
>  
> +#ifdef GRUB_UTIL
> +
> +#include <stdlib.h>
> +#include <sys/ioctl.h>
> +#include <include/grub/emu/misc.h>
> +#include <include/grub/emu/getroot.h>
> +#include <include/grub/emu/hostfile.h>
> +
> +/* Read the addresses of all sectors occupied by the file with the
> +   given inode from the filesystem.  Return the number of sectors in
> +   "nsector" and the addresses in "sectors".  "sectors" is allocated
> +   in this function and must be freed by the caller after usage.
> +   A sector in sectors has size GRUB_DISK_SECTOR_SIZE. */
> +static grub_err_t
> +grub_ext2_read_sectorlist(grub_device_t device,
> +			 int ino,
> +			 unsigned int *nsectors,
> +			 grub_disk_addr_t **sectors)
> +{
> +  struct grub_ext2_data *data;
> +  struct grub_ext2_inode inode;
> +  grub_off_t size;
> +  grub_err_t err;
> +  grub_disk_addr_t fileblock;
> +  int i;
> +  grub_disk_addr_t fileblock_count;
> +  int sectors_per_block;
> +
> +  data = grub_ext2_mount (device->disk);
> +  if (! data) {
> +    return grub_error (grub_errno, N_("Unable to mount device"));
> +  }
> +
> +  err = grub_ext2_read_inode (data, ino, &inode);
> +  if (err)
> +    return grub_error (err, N_("Unable to read inode #%d"), ino);
> +
> +  size = grub_le_to_cpu32 (inode.size);
> +  size |= ((grub_off_t) grub_le_to_cpu32 (inode.size_high)) << 32;
> +
> +  fileblock_count = size / EXT2_BLOCK_SIZE(data);
> +  if ( size % EXT2_BLOCK_SIZE(data) != 0 ) fileblock_count++;
> +
> +  sectors_per_block = EXT2_BLOCK_SIZE(data) / GRUB_DISK_SECTOR_SIZE;
> +  *sectors = grub_malloc (fileblock_count * sectors_per_block
> +			  * sizeof (**sectors));
> +  *nsectors = 0;
> +  for ( fileblock = 0; fileblock < fileblock_count; fileblock++ ) {
> +    (*sectors)[*nsectors] = grub_ext2_read_block2 (data, &inode, fileblock)
> +	                    * sectors_per_block;
> +    for ( i = 1; i < sectors_per_block; i++) {
> +      (*sectors)[(*nsectors) + i] = (*sectors)[*nsectors] + i;
> +    }
> +    (*nsectors) += sectors_per_block;
> +  }
> +
> +  return GRUB_ERR_NONE;
> +}
> +
> +static grub_err_t
> +grub_ext2_embed (grub_device_t device,
> +		  unsigned int *nsectors,
> +		  unsigned int max_nsectors,
> +		  grub_embed_type_t embed_type,
> +		  grub_disk_addr_t **sectors)
> +{
> +  unsigned i;
> +  grub_err_t err;
> +  char *mountpoint;
> +  struct mountinfo_entry *entries;
> +  grub_util_fd_t out;
> +  char *core_name;
> +  char diskbuffer[GRUB_DISK_SECTOR_SIZE];
> +  unsigned int nsectors_wanted;
> +  char *device_name;
> +  int ioctl_err;
> +
> +  if (embed_type != GRUB_EMBED_PCBIOS)
> +    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> +		       N_("ext2 currently supports only PC-BIOS embedding"));
> +
> +  if ( device->disk->partition)
> +      device_name = grub_xasprintf ("%s,%s", device->disk->name,
> +			   grub_partition_get_name(device->disk->partition));
> +  else
> +      device_name = grub_xasprintf ("%s", device->disk->name);
> +
> +  nsectors_wanted = *nsectors;
> +
> +  grub_util_info (N_("Embedding %d/%d sectors into ext2 filesystem of %s"),
> +		 nsectors_wanted, max_nsectors, device_name);
> +
> +  /* [1] Check, if the existing boot loader inode exists and has the
> +         wanted size: */
> +  err = grub_ext2_read_sectorlist (device, EXT2_BOOT_LOADER_INO,
> +				   nsectors, sectors);
> +  if (!err && *nsectors >= nsectors_wanted && *nsectors <= max_nsectors) {
> +      grub_util_info(N_("Reusing existing boot loader inode"
> +			" offering %d sectors"),
> +		   *nsectors);
> +    grub_free (device_name);
> +    return GRUB_ERR_NONE;       /* YES! Everything is fine. */
> +  }
> +
> +  /* [2] We have to create a new boot loader inode. */
> +
> +  /* [2.1] Check if device is mounted and get mountpoint: */
> +  mountpoint = NULL;
> +  entries = grub_read_mountinfo ();
> +  if ( entries ) {
> +    for ( i = 0; entries[i].enc_root[0] != 0; i++) {
> +      char *grub_dev_of_mount;
> +      grub_errno = GRUB_ERR_NONE;    /* Clear errno set previously */
> +      grub_dev_of_mount = grub_util_get_grub_dev (entries[i].device);
> +      if ( grub_dev_of_mount ) {
> +	if ( grub_strcmp (grub_dev_of_mount, device_name) == 0 ) {
> +	  mountpoint = grub_strdup (entries[i].enc_path);
> +	  break;
> +	}
> +      }
> +    }
> +    free (entries);
> +  }
> +
> +  grub_errno = GRUB_ERR_NONE;    /* Clear errno set previously */
> +
> +  if (!mountpoint) {
> +    /* We were unable to find the mountpoint for the device of the
> +       filesystem. Maybe it is not mounted? */
> +    return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
> +		       N_("We are unable to find the mountpoint of %s"),
> +		       device_name);
> +
> +    /** ToDo: Try to mount the filesystem to a temporary location. */
> +  }
> +
> +  grub_free (device_name);
> +
> +  /* [2.2] Create a new (temporary) file, which then gets the boot loader: */
> +  core_name = grub_util_path_concat (2, mountpoint, ".core.img");
> +  free (mountpoint);
> +  out = grub_util_fd_open (core_name, GRUB_UTIL_FD_O_WRONLY
> +			   | GRUB_UTIL_FD_O_CREATTRUNC);
> +  if (!GRUB_UTIL_FD_IS_VALID (out)) {
> +    return grub_error (GRUB_ERR_BAD_FS,
> +		       N_("cannot create `%s': %s"), core_name,
> +		       grub_util_fd_strerror ());
> +  }
> +  for ( i = 0; i < max_nsectors; i++ ) {
> +      grub_util_fd_write (out, diskbuffer, GRUB_DISK_SECTOR_SIZE);
> +  }
> +  grub_util_fd_sync (out);
> +
> +  /* [2.3] Make that file to the new boot loader inode by swapping the
> +           file of "out" with the boot loader inode: */
> +  ioctl_err = ioctl (out, EXT4_IOC_SWAP_BOOT);
> +  if ( ioctl_err ) {
> +    err = grub_error (GRUB_ERR_BAD_FS,
> +		      N_("Error in ioctl(EXT4_IOC_SWAP_BOOT);"
> +			 "you need Linux >= 3.10: %s"),
> +		      grub_util_fd_strerror ());
> +    grub_util_fd_close (out);
> +    grub_util_unlink (core_name);
> +    return err;
> +  }
> +  grub_util_fd_close (out);
> +
> +  /* [2.4] Unlink the core file, now containing the previous boot loader. */
> +  grub_util_unlink (core_name);
> +
> +  /* [2.5] Invalidate disk cache and read block list again: */
> +  grub_disk_cache_invalidate_all ();
> +
> +  err = grub_ext2_read_sectorlist (device, EXT2_BOOT_LOADER_INO,
> +				   nsectors, sectors);
> +  if ( err ) {
> +    return grub_error (GRUB_ERR_BAD_FS,
> +		       N_("unable to read boot loader inode"));
> +  }
> +
> +  grub_util_info (N_("Created new boot loader inode offering %d sectors"),
> +		     *nsectors);
> +
> +  return GRUB_ERR_NONE;
> +}
> +#endif
> +
>  
>  \f
>  static struct grub_fs grub_ext2_fs =
> @@ -990,6 +1191,7 @@ static struct grub_fs grub_ext2_fs =
>      .uuid = grub_ext2_uuid,
>      .mtime = grub_ext2_mtime,
>  #ifdef GRUB_UTIL
> +    .embed = grub_ext2_embed,
>      .reserved_first_sector = 1,
>      .blocklist_install = 1,
>  #endif
> diff --git a/util/setup.c b/util/setup.c
> index 9fb91a8..3f4a007 100644
> --- a/util/setup.c
> +++ b/util/setup.c
> @@ -509,8 +509,10 @@ SETUP (const char *dir,
>      if (!err && nsec < core_sectors)
>        {
>  	err = grub_error (GRUB_ERR_OUT_OF_RANGE,
> -			  N_("Your embedding area is unusually small.  "
> -			     "core.img won't fit in it."));
> +			  N_("Your embedding area is unusually small "
> +			     "(only %d sectors). "
> +			     "core.img won't fit in it (needs %d sectors)."),
> +			  nsec, core_sectors);
>        }
>      
>      if (err)
> @@ -583,10 +585,13 @@ SETUP (const char *dir,
>        }
>  
>      /* Write the core image onto the disk.  */
> -    for (i = 0; i < nsec; i++)
> +    for (i = 0; i < nsec; i++) {
> +	grub_util_info ("writing core.img/%d to sector %" PRIuGRUB_UINT64_T, i,
> +			sectors[i]);
>        grub_disk_write (dest_dev->disk, sectors[i], 0,
>  		       GRUB_DISK_SECTOR_SIZE,
>  		       core_img + i * GRUB_DISK_SECTOR_SIZE);
> +    }
>  
>      grub_free (sectors);
>  
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 274 bytes --]

  reply	other threads:[~2014-01-09 22:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-09 21:07 [PATCH] Improve ext2 driver to allow embedding of the boot loader code Dr. Tilmann Bubeck
2014-01-09 22:52 ` Vladimir 'φ-coder/phcoder' Serbinenko [this message]
2014-01-10  7:49   ` Dr. Tilmann Bubeck
2014-01-21  8:14     ` Vladimir 'φ-coder/phcoder' Serbinenko
2014-01-21  8:28       ` Andrey Borzenkov
2014-01-21  8:32         ` Vladimir 'φ-coder/phcoder' Serbinenko
2014-01-21  8:41           ` Andrey Borzenkov
2014-01-21 11:03             ` Vladimir 'φ-coder/phcoder' Serbinenko
2014-01-21 19:55               ` Dr. Tilmann Bubeck
2014-02-01  7:23                 ` Dr. Tilmann Bubeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52CF282C.4070408@gmail.com \
    --to=phcoder@gmail.com \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.