[U-Boot] [PATCH 0/4] efi_loader: fix EFI_BLOCK_IO_PROTOCOL

* provide parameter checks for ReadBlocks and WriteBlocks * correct return code for Reset * indented debug output * add EFI_BLOCK_IO_PROTOCOL and other protocols to HTML documentation
Heinrich Schuchardt (4): efi_loader: parameter checks BLOCK_IO_PROTOCOL efi_loader: use EFI_PRINT() instead of debug() efi_loader: EFI_BLOCK_IO_PROTOCOL.Reset() doc: UEFI API documentation
doc/api/efi.rst | 33 ++++++++++++++++++++ lib/efi_loader/efi_console.c | 11 ++++--- lib/efi_loader/efi_disk.c | 60 +++++++++++++++++++++++++++++++++--- lib/efi_loader/efi_gop.c | 2 +- lib/efi_loader/efi_net.c | 9 ++++-- 5 files changed, 101 insertions(+), 14 deletions(-)
-- 2.23.0.rc1

Check parameters of ReadBlocks() and WriteBlocks().
If the buffer size is not a multiple of the block size, we have to return EFI_BAD_BUFFER_SIZE.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_disk.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 7a6b06821a..0bf436ac66 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -74,7 +74,7 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
/* We only support full block access */ if (buffer_size & (blksz - 1)) - return EFI_DEVICE_ERROR; + return EFI_BAD_BUFFER_SIZE;
if (direction == EFI_DISK_READ) n = blk_dread(desc, lba, blocks, buffer); @@ -99,6 +99,20 @@ static efi_status_t EFIAPI efi_disk_read_blocks(struct efi_block_io *this, void *real_buffer = buffer; efi_status_t r;
+ if (!this) + return EFI_INVALID_PARAMETER; + /* TODO: check for media changes */ + if (media_id != this->media->media_id) + return EFI_MEDIA_CHANGED; + if (!this->media->media_present) + return EFI_NO_MEDIA; + /* media->io_align is a power of 2 */ + if ((uintptr_t)buffer & (this->media->io_align - 1)) + return EFI_INVALID_PARAMETER; + if (lba * this->media->block_size + buffer_size > + this->media->last_block * this->media->block_size) + return EFI_INVALID_PARAMETER; + #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER if (buffer_size > EFI_LOADER_BOUNCE_BUFFER_SIZE) { r = efi_disk_read_blocks(this, media_id, lba, @@ -134,6 +148,22 @@ static efi_status_t EFIAPI efi_disk_write_blocks(struct efi_block_io *this, void *real_buffer = buffer; efi_status_t r;
+ if (!this) + return EFI_INVALID_PARAMETER; + if (this->media->read_only) + return EFI_WRITE_PROTECTED; + /* TODO: check for media changes */ + if (media_id != this->media->media_id) + return EFI_MEDIA_CHANGED; + if (!this->media->media_present) + return EFI_NO_MEDIA; + /* media->io_align is a power of 2 */ + if ((uintptr_t)buffer & (this->media->io_align - 1)) + return EFI_INVALID_PARAMETER; + if (lba * this->media->block_size + buffer_size > + this->media->last_block * this->media->block_size) + return EFI_INVALID_PARAMETER; + #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER if (buffer_size > EFI_LOADER_BOUNCE_BUFFER_SIZE) { r = efi_disk_write_blocks(this, media_id, lba, @@ -288,6 +318,11 @@ static efi_status_t efi_disk_add_dev( /* Fill in EFI IO Media info (for read/write callbacks) */ diskobj->media.removable_media = desc->removable; diskobj->media.media_present = 1; + /* + * MediaID is just an arbitrary counter. + * We have to change it if the medium is removed or changed. + */ + diskobj->media.media_id = 1; diskobj->media.block_size = desc->blksz; diskobj->media.io_align = desc->blksz; diskobj->media.last_block = desc->lba - offset; -- 2.23.0.rc1

EFI_PRINT() offers indention of debug messages. Adjust the debug messages of the BLOCK_IO_PROTOCOL.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_disk.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 0bf436ac66..4d4937c8ba 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -69,8 +69,8 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this, blocks = buffer_size / blksz; lba += diskobj->offset;
- debug("EFI: %s:%d blocks=%x lba=%llx blksz=%x dir=%d\n", __func__, - __LINE__, blocks, lba, blksz, direction); + EFI_PRINT("blocks=%x lba=%llx blksz=%x dir=%d\n", + blocks, lba, blksz, direction);
/* We only support full block access */ if (buffer_size & (blksz - 1)) @@ -84,7 +84,7 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this, /* We don't do interrupts, so check for timers cooperatively */ efi_timer_check();
- debug("EFI: %s:%d n=%lx blocks=%x\n", __func__, __LINE__, n, blocks); + EFI_PRINT("n=%lx blocks=%x\n", n, blocks);
if (n != blocks) return EFI_DEVICE_ERROR; -- 2.23.0.rc1

We cannot do anything in EFI_BLOCK_IO_PROTOCOL.Reset() but this does not justify to return an error.
Let EFI_BLOCK_IO_PROTOCOL.Reset() return EFI_SUCCESS.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_disk.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 4d4937c8ba..9007a5f77f 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -41,11 +41,26 @@ struct efi_disk_obj { struct blk_desc *desc; };
+/** + * efi_disk_reset() - reset block device + * + * This function implements the Reset service of the EFI_BLOCK_IO_PROTOCOL. + * + * As U-Boot's block devices do not have a reset function simply return + * EFI_SUCCESS. + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * @this: pointer to the BLOCK_IO_PROTOCOL + * @extended_verification: extended verification + * Return: status code + */ static efi_status_t EFIAPI efi_disk_reset(struct efi_block_io *this, char extended_verification) { EFI_ENTRY("%p, %x", this, extended_verification); - return EFI_EXIT(EFI_DEVICE_ERROR); + return EFI_EXIT(EFI_SUCCESS); }
enum efi_disk_direction { -- 2.23.0.rc1

Add some more files to the UEFI API documentation.
Correct some Sphinx comments.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- doc/api/efi.rst | 33 +++++++++++++++++++++++++++++++++ lib/efi_loader/efi_console.c | 11 ++++++----- lib/efi_loader/efi_gop.c | 2 +- lib/efi_loader/efi_net.c | 9 ++++++--- 4 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/doc/api/efi.rst b/doc/api/efi.rst index 39e2dbae0b..2ca344932e 100644 --- a/doc/api/efi.rst +++ b/doc/api/efi.rst @@ -103,3 +103,36 @@ Block device driver
.. kernel-doc:: lib/efi_driver/efi_block_device.c :internal: + +Protocols +--------- + +Block IO protocol +~~~~~~~~~~~~~~~~~ + +.. kernel-doc:: lib/efi_loader/efi_disk.c + :internal: + +File protocol +~~~~~~~~~~~~~ + +.. kernel-doc:: lib/efi_loader/efi_file.c + :internal: + +Graphical output protocol +~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. kernel-doc:: lib/efi_loader/efi_gop.c + :internal: + +Network protocols +~~~~~~~~~~~~~~~~~ + +.. kernel-doc:: lib/efi_loader/efi_net.c + :internal: + +Text IO protocols +~~~~~~~~~~~~~~~~~ + +.. kernel-doc:: lib/efi_loader/efi_console.c + :internal: diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c index 5109017796..a55e4b33ec 100644 --- a/lib/efi_loader/efi_console.c +++ b/lib/efi_loader/efi_console.c @@ -219,9 +219,9 @@ static bool cout_mode_matches(struct cout_mode *mode, int rows, int cols) /** * query_console_serial() - query console size * - * @rows pointer to return number of rows - * @columns pointer to return number of columns - * Returns 0 on success + * @rows: pointer to return number of rows + * @cols: pointer to return number of columns + * Returns: 0 on success */ static int query_console_serial(int *rows, int *cols) { @@ -464,7 +464,7 @@ struct efi_simple_text_output_protocol efi_con_out = { * struct efi_cin_notify_function - registered console input notify function * * @link: link to list - * @data: key to notify + * @key: key to notify * @function: function to call */ struct efi_cin_notify_function { @@ -482,6 +482,7 @@ static LIST_HEAD(cin_notify_functions); * set_shift_mask() - set shift mask * * @mod: Xterm shift mask + * @key_state: receives the state of the shift, alt, control, and logo keys */ void set_shift_mask(int mod, struct efi_key_state *key_state) { @@ -504,7 +505,7 @@ void set_shift_mask(int mod, struct efi_key_state *key_state) * * This gets called when we have already parsed CSI. * - * @modifiers: bit mask (shift, alt, ctrl) + * @key_state: receives the state of the shift, alt, control, and logo keys * @return: the unmodified code */ static int analyze_modifiers(struct efi_key_state *key_state) diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index cad509bfea..1511e3bdb4 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -319,7 +319,7 @@ static efi_status_t gop_blt_vid_to_buf(struct efi_gop *this, * details. * * @this: the graphical output protocol - * @model_number: the mode to be set + * @mode_number: the mode to be set * Return: status code */ static efi_status_t EFIAPI gop_set_mode(struct efi_gop *this, u32 mode_number) diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index cff6332a68..82d2595847 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -399,7 +399,7 @@ out: * Protocol. See the UEFI spec for details. * * @this: the instance of the Simple Network Protocol - * @readwrite: true for read, false for write + * @read_write: true for read, false for write * @offset: offset in NVRAM * @buffer_size: size of buffer * @buffer: buffer @@ -639,6 +639,9 @@ out: * efi_net_set_dhcp_ack() - take note of a selected DHCP IP address * * This function is called by dhcp_handler(). + * + * @pkt: packet received by dhcp_handler() + * @len: length of the packet received */ void efi_net_set_dhcp_ack(void *pkt, int len) { @@ -668,8 +671,8 @@ static void efi_net_push(void *pkt, int len) * * This notification function is called in every timer cycle. * - * @event the event for which this notification function is registered - * @context event context - not used in this function + * @event: the event for which this notification function is registered + * @context: event context - not used in this function */ static void EFIAPI efi_network_timer_notify(struct efi_event *event, void *context) -- 2.23.0.rc1
participants (1)
-
Heinrich Schuchardt