Re: [PATCH v2 1/2] efi: SPI NOR flash support

[Heinrich Schuchardt]

On 11.02.21 08:36, Michael Lawnick wrote:
>
> Hi,
>
> seven days of silence. In the end no interest for extending EFI support?
>
> KR
> Michael
>
> Am 05.02.2021 um 09:58 schrieb Michael Lawnick:
>> Add EFI SPI NOR driver
>>
>> Use UEFI interface for accessing SPI NOR flashes.
>> If supported the implementation of UEFI boot software abstracts
>> away all those ugly H/W details like SPI controller or protocol.
>> Provided functions:
>> grub_efi_spi_nor_
>>     init
>>     erase
>>     write
>>     read
>>     flash_size
>>     flash_id
>>     erase_block_size
>>
>> This driver might be used for further abstraction to a common
>> (SPI) flash interface.
>>

A commit message should describe what the patch is good for.

What is the use case for GRUB accessing SPI?

In your second patch you introduce a command to write and erase the SPI
flash. Hopefully the firmware has disabled writes.

GRUB writing to SPI would mean that a user program could introduce
malware into the firmware by adding said command to grub.cfg.

This would be a gross security issue. Hopefully the firmware has locked
the SPI flash before entering GRUB.

SPI flash updates should be effected via signed UEFI update capsules and
not via GRUB.

>> Signed-off-by: Michael Lawnick <michael.lawnick@nokia.com>
>> ---
>> [Patch v2 1/2] : fix flaw in EFI header, wrong sequence of methods.
>> ---
>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>> index 68b9e9f68..4d775e5f6 100644
>> --- a/grub-core/Makefile.core.def
>> +++ b/grub-core/Makefile.core.def
>> @@ -446,7 +446,7 @@ image = {
>>      i386_pc = boot/i386/pc/boot.S;
>>
>>      cppflags = '-DHYBRID_BOOT=1';
>> -
>> +
>>      i386_pc_ldflags = '$(TARGET_IMG_LDFLAGS)';
>>      i386_pc_ldflags = '$(TARGET_IMG_BASE_LDOPT),0x7C00';
>>
>> @@ -656,6 +656,12 @@ module = {
>>      enable = i386_multiboot;
>>    };
>>
>> +module = {
>> +  name = efi_spi_nor;
>> +  common = bus/spi/efi_spi_nor.c;
>> +  enable = efi;
>> +};
>> +
>>    module = {
>>      name = nativedisk;
>>      common = commands/nativedisk.c;
>> diff --git a/grub-core/bus/spi/efi_spi_nor.c
>> b/grub-core/bus/spi/efi_spi_nor.c
>> new file mode 100644
>> index 000000000..0e073b436
>> --- /dev/null
>> +++ b/grub-core/bus/spi/efi_spi_nor.c
>> @@ -0,0 +1,298 @@
>> +/*  efi_spi_nor.c  - Give access to SPI NOR flash through UEFI
>> interface.
>> + *  Copyright 2021 Nokia
>> + *  Licensed under the GNU General Public License v3.0 only
>> + *  SPDX-License-Identifier: GPL-3.0-only
>> + *
>> + *  GRUB  --  GRand Unified Bootloader
>> + *  Copyright (C) 2008  Free Software Foundation, Inc.

The FSF wrote part of this code in 2008?

>> + *
>> + *  GRUB is free software: you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation, either version 3 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  GRUB is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include <grub/command.h>
>> +#include <grub/misc.h>
>> +#include <grub/mm.h>
>> +#include <grub/types.h>
>> +#include <grub/efi/api.h>
>> +#include <grub/efi/efi.h>
>> +#include <grub/efi/efi_spi_nor.h>
>> +
>> +GRUB_MOD_LICENSE ("GPLv3+");
>> +
>> +#define EFI_SPI_NOR_FLASH_PROTOCOL_GUID \
>> +    { 0xb57ec3fe, 0xf833, 0x4ba6, \
>> +        {0x85, 0x78, 0x2a, 0x7d, 0x6a, 0x87, 0x44, 0x4b} \
>> +    }

This protocol is not defined in the UEFI spec 2.8B.

It is defined in the Platform Initialization (PI) Specification, Volume
1, Pre-EFI Initialization Core Interface.

So it seems it is not meant to be consumed by UEFI applications.

U-Boot adheres to the UEFI spec but has no PI phase. So don't expect the
protocol there.

Best regards

Heinrich

>> +
>> +#define EFI_FLASHID_LEN 3
>> +
>> +struct efi_spi_nor_flash_protocol {
>> +    struct spi_nor    *spi_peripheral;
>> +    grub_efi_uint32_t    flash_size;
>> +    grub_efi_uint8_t    device_id[EFI_FLASHID_LEN];
>> +    grub_efi_uint32_t    erase_block_size;
>> +
>> +    grub_efi_status_t (* get_flash_id)(struct efi_spi_nor_flash_protocol
>> *this,
>> +                         grub_uint8_t *buffer);
>> +    grub_efi_status_t (* read_data)(struct efi_spi_nor_flash_protocol
>> *this,
>> +                      grub_uint32_t offset, grub_uint32_t len,
>> grub_uint8_t *data);
>> +    grub_efi_status_t (* lf_read_data)(struct efi_spi_nor_flash_protocol
>> *this,
>> +                         grub_uint32_t offset, grub_uint32_t len,
>> grub_uint8_t *data);
>> +    grub_efi_status_t (* read_status)(struct
>> efi_spi_nor_flash_protocol *this,
>> +                        grub_uint32_t num_bytes, grub_uint8_t *status);
>> +    grub_efi_status_t (* write_status)(struct efi_spi_nor_flash_protocol
>> *this,
>> +                         grub_uint32_t num_bytes, grub_uint8_t *status);
>> +    grub_efi_status_t (* write_data)(struct
>> efi_spi_nor_flash_protocol *this,
>> +                       grub_uint32_t offset, grub_uint32_t len,
>> grub_uint8_t *data);
>> +    grub_efi_status_t (* erase_blocks)(struct efi_spi_nor_flash_protocol
>> *this,
>> +                         grub_uint32_t offset, grub_uint32_t blk_count);
>> +};
>> +
>> +/* grub_efi_spi_nor_init - initialize access to SPI NOR flash device
>> + *
>> + * Search pool of SPI NOR flash devices known to underlying EFI
>> bootware.
>> + * Use <flash_id> and <instance> to filter out devices.
>> + *
>> + * IN: flash_id     - optional, pointer to max 3 bytes
>> (EFI_FLASHID_LEN) to match against
>> + *                    SPI flash JEDEC ID, use NULL if no filtering.
>> + * IN: num_id_bytes - number of bytes in flash_id. Maximum 3 bytes
>> + *                    are used for comparison.
>> + * IN: instance     - number of device occurances to skip
>> + *
>> + * returns : pointer to flash device or NULL on failure
>> + */
>> +void *
>> +grub_efi_spi_nor_init(grub_uint8_t *flash_id, grub_uint32_t
>> num_id_bytes, grub_uint32_t instance)
>> +{
>> +    grub_efi_guid_t efi_guid_spi_nor_flash_protocol =
>> EFI_SPI_NOR_FLASH_PROTOCOL_GUID;
>> +    grub_efi_status_t ret;
>> +    grub_efi_uintn_t num_handles;
>> +    grub_efi_handle_t *handles;
>> +    grub_uint8_t found_id[EFI_FLASHID_LEN];
>> +    grub_uint32_t idx, match_cnt=0;
>> +    struct efi_spi_nor_flash_protocol *spi_nor;
>> +
>> +    handles = grub_efi_locate_handle(GRUB_EFI_BY_PROTOCOL,
>> +                &efi_guid_spi_nor_flash_protocol, 0,
>> +                &num_handles);
>> +    grub_boot_time("found %ld SPI NOR flash devices\n", num_handles);
>> +
>> +    if ((num_handles == 0) || (num_handles < instance + 1))
>> +        return NULL;
>> +
>> +    for (idx = 0; idx < num_handles; idx++) {
>> +        spi_nor = grub_efi_open_protocol(handles[idx],
>> +                        &efi_guid_spi_nor_flash_protocol,
>> +                        GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>> +        if (! spi_nor) {
>> +            grub_error(GRUB_ERR_UNKNOWN_DEVICE, "Failed to open
>> device protocol\n");
>> +            grub_free(handles);
>> +            return NULL;
>> +        }
>> +
>> +        ret = spi_nor->get_flash_id(spi_nor, found_id);
>> +        if (ret != GRUB_EFI_SUCCESS) {
>> +            grub_error(GRUB_ERR_READ_ERROR, "Failed to read
>> flash_id\n");
>> +            grub_free(handles);
>> +            return NULL;
>> +        }
>> +
>> +        /* if caller requests filtering by id */
>> +        if (flash_id != NULL) {
>> +            grub_uint32_t id;
>> +
>> +            if (num_id_bytes > EFI_FLASHID_LEN)
>> +                num_id_bytes = EFI_FLASHID_LEN;
>> +
>> +            for (id = 0; id < num_id_bytes; id++)
>> +                if (flash_id[id] != found_id[id])
>> +                    break;
>> +
>> +            if (id != num_id_bytes)
>> +                continue;
>> +        }
>> +
>> +        if (match_cnt < instance) {
>> +            match_cnt++;
>> +            continue;
>> +        }
>> +
>> +        grub_boot_time("Found flash with ID 0x%02x 0x%02x 0x%02x\n",
>> +                        found_id[0], found_id[1], found_id[2]);
>> +
>> +        grub_free(handles);
>> +        return spi_nor;
>> +    }
>> +
>> +    grub_free(handles);
>> +    return NULL;
>> +}
>> +
>> +/* grub_efi_spi_nor_flash_size - get memory size of SPI NOR flash device
>> + *
>> + * IN: efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + *
>> + * returns : memory size of flash device or 0 on failure
>> + */
>> +grub_uint32_t
>> +grub_efi_spi_nor_flash_size(void *efi_flash_dev)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +
>> +    if (spi_nor == NULL)
>> +        return 0;
>> +
>> +    return spi_nor->flash_size;
>> +}
>> +
>> +/* grub_efi_spi_nor_device_id - get three byte JEDEC ID of SPI NOR
>> flash device
>> + *
>> + * IN: efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + *
>> + * returns : three byte JEDEC ID as a uint32 or 0 on failure
>> + */
>> +grub_uint32_t
>> +grub_efi_spi_nor_device_id(void *efi_flash_dev)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +    grub_uint32_t devId = 0;
>> +    grub_efi_status_t ret;
>> +    grub_uint8_t device_id[3];
>> +    int i;
>> +
>> +    if (spi_nor == NULL)
>> +        return 0;
>> +
>> +    ret = spi_nor->get_flash_id(spi_nor, device_id);
>> +    if (ret != GRUB_EFI_SUCCESS)
>> +        return 0;
>> +
>> +    for(i=0; i<3;i++)
>> +        devId = (devId<<8) + device_id[i];
>> +
>> +    return devId;
>> +}
>> +
>> +/* grub_efi_spi_nor_erase_block_size - get erase block size of SPI NOR
>> flash device
>> + *
>> + * Parameters for calls to grub_efi_spi_nor_erase() need to be erase
>> block size
>> + * aligned.
>> + *
>> + * IN: efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + *
>> + * returns : size of erase block of flash device or 0 on failure
>> + */
>> +grub_uint32_t
>> +grub_efi_spi_nor_erase_block_size(void *efi_flash_dev)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +
>> +    if (spi_nor == NULL)
>> +        return 0;
>> +
>> +    return spi_nor->erase_block_size;
>> +}
>> +
>> +/* grub_efi_spi_nor_read - read from SPI NOR flash device
>> + *
>> + * IN:  efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + * OUT: buffer        - pointer to an preallocated buffer of min. bytes
>> size
>> + * IN:  offset        - starting point where to start to read from
>> + * IN:  bytes         - number of bytes to read from flash and write to
>> buffer
>> + *
>> + * returns : GRUB_EFI_SUCCESS if all ok
>> + *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
>> + *
>> + */
>> +grub_err_t
>> +grub_efi_spi_nor_read(void *efi_flash_dev, grub_uint8_t *buffer,
>> grub_uint32_t offset, grub_uint32_t bytes)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +    grub_efi_status_t ret;
>> +
>> +    if (spi_nor == NULL || buffer == NULL)
>> +        return GRUB_ERR_BAD_ARGUMENT;
>> +
>> +    ret = (spi_nor->read_data(spi_nor, offset, bytes, buffer));
>> +    if (ret != GRUB_EFI_SUCCESS)
>> +        return grub_error(GRUB_ERR_READ_ERROR, "Failed to read data
>> @0x%x\n",
>> offset);
>> +
>> +    return GRUB_ERR_NONE;
>> +}
>> +
>> +/* grub_efi_spi_nor_write - write to SPI NOR flash device
>> + *
>> + * IN: efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + * IN: buffer        - pointer to buffer containig min. bytes size data
>> to write
>> + * IN: offset        - starting point where to start to write to
>> + * IN: bytes         - number of bytes to write to buffer
>> + *
>> + * returns : GRUB_EFI_SUCCESS if all ok
>> + *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
>> + *
>> + */
>> +grub_err_t
>> +grub_efi_spi_nor_write(void *efi_flash_dev, grub_uint8_t *buffer,
>> grub_uint32_t offset, grub_uint32_t bytes)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +    grub_efi_status_t ret;
>> +
>> +    if (spi_nor == NULL || buffer == NULL)
>> +        return GRUB_ERR_BAD_ARGUMENT;
>> +
>> +    ret = (spi_nor->write_data(spi_nor, offset, bytes, buffer));
>> +    if (ret != GRUB_EFI_SUCCESS)
>> +        return grub_error(GRUB_ERR_WRITE_ERROR, "Failed to write data
>> @0x%x\n", offset);
>> +
>> +    return GRUB_ERR_NONE;
>> +}
>> +
>> +/* grub_efi_spi_nor_erase - erase sectors on SPI NOR flash device
>> + *
>> + * Parameters offset and bytes need to be erase sector aligned, i.e.
>> + * multiples of the size returned by function
>> grub_efi_spi_nor_erase_block_size()
>> + *
>> + * IN: efi_flash_dev - device identifier returned by
>> grub_efi_spi_nor_init.
>> + * IN: offset        - offset of first sector to erase
>> + * IN: bytes         - length of region to erase
>> + *
>> + * returns : GRUB_EFI_SUCCESS if all ok
>> + *           GRUB_EFI_INVALID_PARAMETER if an argument is bad
>> + *
>> + */
>> +grub_err_t
>> +grub_efi_spi_nor_erase(void *efi_flash_dev, grub_uint32_t offset,
>> grub_uint32_t bytes)
>> +{
>> +    struct efi_spi_nor_flash_protocol *spi_nor = efi_flash_dev;
>> +    grub_efi_status_t ret;
>> +    grub_uint32_t sect_sz;
>> +    grub_uint32_t sect_mask;
>> +
>> +    if (spi_nor == NULL || bytes == 0)
>> +        return GRUB_ERR_BAD_ARGUMENT;
>> +
>> +    sect_sz = grub_efi_spi_nor_erase_block_size(spi_nor);
>> +    sect_mask = sect_sz - 1;
>> +    if ((offset & sect_mask) != 0)
>> +        return grub_error(GRUB_ERR_BAD_ARGUMENT, "SPI NOR erase
>> offset not at
>> sector boundary");
>> +
>> +    if (((offset + bytes) & sect_mask) != 0)
>> +        return grub_error(GRUB_ERR_BAD_ARGUMENT, "SPI NOR erase end
>> not at
>> sector boundary");
>> +
>> +    ret = spi_nor->erase_blocks(spi_nor, offset, (bytes-1) / sect_sz
>> + 1);
>> +
>> +    if (ret != GRUB_EFI_SUCCESS)
>> +        return grub_error(GRUB_ERR_WRITE_ERROR, "SPI NOR erase operation
>> failed");
>> +
>> +    return GRUB_ERR_NONE;
>> +}
>> diff --git a/include/grub/efi/efi_spi_nor.h
>> b/include/grub/efi/efi_spi_nor.h
>> new file mode 100644
>> index 000000000..d09f0c9b7
>> --- /dev/null
>> +++ b/include/grub/efi/efi_spi_nor.h
>> @@ -0,0 +1,37 @@
>> +/*  efi_spi_nor.h  - Give access to SPI NOR flash through UEFI
>> interface.
>> + *  Copyright 2021 Nokia
>> + *  Licensed under the GNU General Public License v3.0 only
>> + *  SPDX-License-Identifier: GPL-3.0-only
>> + *
>> + *  GRUB  --  GRand Unified Bootloader
>> + *  Copyright (C) 2008  Free Software Foundation, Inc.
>> + *
>> + *  GRUB is free software: you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation, either version 3 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  GRUB is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#ifndef GRUB_EFI_SPI_NOR_HEADER
>> +#define GRUB_EFI_SPI_NOR_HEADER
>> +
>> +#include <grub/types.h>
>> +
>> +void *grub_efi_spi_nor_init(grub_uint8_t *flash_id, grub_uint32_t
>> num_id_bytes, grub_uint32_t instance);
>> +
>> +grub_uint32_t grub_efi_spi_nor_flash_size(void *efi_flash_dev);
>> +grub_uint32_t grub_efi_spi_nor_device_id(void *efi_flash_dev);
>> +grub_uint32_t grub_efi_spi_nor_erase_block_size(void *efi_flash_dev);
>> +
>> +grub_err_t grub_efi_spi_nor_read(void *efi_flash_dev, grub_uint8_t
>> *buffer, grub_uint32_t offset, grub_uint32_t bytes);
>> +grub_err_t grub_efi_spi_nor_write(void *efi_flash_dev, grub_uint8_t
>> *buffer, grub_uint32_t offset, grub_uint32_t bytes);
>> +grub_err_t grub_efi_spi_nor_erase(void *efi_flash_dev, grub_uint32_t
>> offset, grub_uint32_t bytes);
>> +
>> +#endif /*GRUB_EFISPINOR*/
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>