On 18.02.2016 16:00, Leif Lindholm wrote:
> Returned from the OpenProtocol operation, the grub_efi_block_io_media
> structure contains the io_align field, specifying the minimum alignment
> required for buffers used in any data transfers with the device.
>
> Make grub_efidisk_readwrite() allocate a temporary buffer, aligned to
> this boundary, if the buffer passed to it does not already meet the
> requirements.
>
> Also sanity check the io_align field in grub_efidisk_open() for
> power-of-two-ness and bail if invalid.
>
> Reported-by: Jeremy Linton <jeremy.lin...@arm.com>
> ---
>
> v3 - Fixed up types and added a check on the io_align field, as per
> Andrei's comments.
>
> grub-core/disk/efi/efidisk.c | 53
> ++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 1c00e3e..c1afbe4 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -463,6 +463,7 @@ grub_efidisk_open (const char *name, struct grub_disk
> *disk)
> int num;
> struct grub_efidisk_data *d = 0;
> grub_efi_block_io_media_t *m;
> + unsigned long i = 0;
>
> grub_dprintf ("efidisk", "opening %s\n", name);
>
> @@ -491,10 +492,21 @@ grub_efidisk_open (const char *name, struct grub_disk
> *disk)
>
> disk->id = ((num << GRUB_CHAR_BIT) | name[0]);
> m = d->block_io->media;
> + /* Ensure required buffer alignment is a power of two (or is zero). */
> + do
> + {
> + if ((m->io_align & (1UL << i)) && (m->io_align & ~(1UL << i)))
> + return grub_error (GRUB_ERR_IO, "invalid buffer alignment %d",
> + m->io_align);
> + }
> + while (++i < (sizeof (grub_addr_t) * 8));
> +
if (m->io_align & (m->io_align - 1)) { ...error handling... }
> /* FIXME: Probably it is better to store the block size in the disk,
> and total sectors should be replaced with total blocks. */
> - grub_dprintf ("efidisk", "m = %p, last block = %llx, block size = %x\n",
> - m, (unsigned long long) m->last_block, m->block_size);
> + grub_dprintf ("efidisk",
> + "m = %p, last block = %llx, block size = %x, io align = %x\n",
> + m, (unsigned long long) m->last_block, m->block_size,
> + m->io_align);
> disk->total_sectors = m->last_block + 1;
> /* Don't increase this value due to bug in some EFI. */
> disk->max_agglomerate = 0xa0000 >> (GRUB_DISK_CACHE_BITS +
> GRUB_DISK_SECTOR_BITS);
> @@ -524,15 +536,42 @@ grub_efidisk_readwrite (struct grub_disk *disk,
> grub_disk_addr_t sector,
> {
> struct grub_efidisk_data *d;
> grub_efi_block_io_t *bio;
> + grub_efi_status_t status;
> + grub_size_t io_align, num_bytes;
> + char *aligned_buf;
>
> d = disk->data;
> bio = d->block_io;
>
> - return efi_call_5 ((wr ? bio->write_blocks : bio->read_blocks), bio,
> - bio->media->media_id,
> - (grub_efi_uint64_t) sector,
> - (grub_efi_uintn_t) size << disk->log_sector_size,
> - buf);
> + /* Set alignment to 1 if 0 specified */
> + io_align = bio->media->io_align ? bio->media->io_align : 1;
> + num_bytes = size << disk->log_sector_size;
> +
> + if ((grub_addr_t) buf & (io_align - 1))
> + {
> + aligned_buf = grub_memalign (io_align, num_bytes);
> + if (! aligned_buf)
> + return GRUB_EFI_OUT_OF_RESOURCES;
Rather than constantly allocating and deallocating perhaps we should
allocate maximum size and then always use the same buffer? See
max_agglomerate. We already limit to at most 640KiB per read on EFI due
to EFI bugs.
> + if (wr)
> + grub_memcpy (aligned_buf, buf, num_bytes);
> + }
> + else
> + {
> + aligned_buf = buf;
> + }
> +
> + status = efi_call_5 ((wr ? bio->write_blocks : bio->read_blocks), bio,
> + bio->media->media_id, (grub_efi_uint64_t) sector,
> + (grub_efi_uintn_t) num_bytes, aligned_buf);
> +
> + if ((grub_addr_t) buf & (io_align - 1))
> + {
> + if (!wr)
> + grub_memcpy (buf, aligned_buf, num_bytes);
> + grub_free (aligned_buf);
> + }
> +
> + return status;
> }
>
> static grub_err_t
>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
