| Message ID | 20190816054020.3200-1-oohall@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | core/flash: Validate secure boot content size | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (0e1db80c70477d89a73c7f2a1a7e19c7d8292c5f) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On Fri, 2019-08-16 at 15:40 +1000, Oliver O'Halloran wrote: > Currently we don't check if the secure boot payload size fits within > the partition that we are reading it from. This results in strange > failures later on in boot if we cross the boundary between an ECCed > and a non-ECCed partition since libflash does not support reading > from regions with mixed ECC status. > > Without this patch: > > blocklevel_read: Can't cope with partial ecc > FLASH: failed to read content size 15728640 BOOTKERNEL partition, rc > 3 > > With: > > FLASH: Cannot load BOOTKERNEL. Content is larger than the partition > > Cc: Nayna Jain <nayna@linux.ibm.com> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > --- > feel free to bikeshed the log message. > --- > core/flash.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/core/flash.c b/core/flash.c > index bfa4a7207a79..67c39c264f7b 100644 > --- a/core/flash.c > +++ b/core/flash.c > @@ -631,6 +631,10 @@ static int flash_load_resource(enum resource_id > id, uint32_t subid, > prlog(PR_DEBUG,"FLASH: %s partition %s ECC\n", > name, ecc ? "has" : "doesn't have"); > > + /* > + * FIXME: Make the fact we don't support partitions smaller > than 4K > + * more explicit. > + */ > if (ffs_part_size < SECURE_BOOT_HEADERS_SIZE) { > prerror("FLASH: secboot headers bigger than " > "partition size 0x%x\n", ffs_part_size); > @@ -668,6 +672,13 @@ static int flash_load_resource(enum resource_id > id, uint32_t subid, > goto out_free_ffs; > } > > + if (*len > ffs_part_size) { > + prerror("FLASH: Cannot load %s. Content is > larger than the partition\n", > + name); Bikeshedding: Would it be nice to include the sizes in the message? > + rc = OPAL_PARAMETER; > + goto out_free_ffs; > + } > + > ffs_part_start += SECURE_BOOT_HEADERS_SIZE; > > rc = blocklevel_read(flash->bl, ffs_part_start, bufp,
On Fri, Aug 16, 2019, at 3:41 PM, Oliver O'Halloran wrote: > Currently we don't check if the secure boot payload size fits within > the partition that we are reading it from. This results in strange > failures later on in boot if we cross the boundary between an ECCed > and a non-ECCed partition since libflash does not support reading > from regions with mixed ECC status. > > Without this patch: > > blocklevel_read: Can't cope with partial ecc > FLASH: failed to read content size 15728640 BOOTKERNEL partition, rc 3 > > With: > > FLASH: Cannot load BOOTKERNEL. Content is larger than the partition Well, that's plausibly my fault. Acked-by: Stewart Smith <stewart@flamingspork.com>
diff --git a/core/flash.c b/core/flash.c index bfa4a7207a79..67c39c264f7b 100644 --- a/core/flash.c +++ b/core/flash.c @@ -631,6 +631,10 @@ static int flash_load_resource(enum resource_id id, uint32_t subid, prlog(PR_DEBUG,"FLASH: %s partition %s ECC\n", name, ecc ? "has" : "doesn't have"); + /* + * FIXME: Make the fact we don't support partitions smaller than 4K + * more explicit. + */ if (ffs_part_size < SECURE_BOOT_HEADERS_SIZE) { prerror("FLASH: secboot headers bigger than " "partition size 0x%x\n", ffs_part_size); @@ -668,6 +672,13 @@ static int flash_load_resource(enum resource_id id, uint32_t subid, goto out_free_ffs; } + if (*len > ffs_part_size) { + prerror("FLASH: Cannot load %s. Content is larger than the partition\n", + name); + rc = OPAL_PARAMETER; + goto out_free_ffs; + } + ffs_part_start += SECURE_BOOT_HEADERS_SIZE; rc = blocklevel_read(flash->bl, ffs_part_start, bufp,
Currently we don't check if the secure boot payload size fits within the partition that we are reading it from. This results in strange failures later on in boot if we cross the boundary between an ECCed and a non-ECCed partition since libflash does not support reading from regions with mixed ECC status. Without this patch: blocklevel_read: Can't cope with partial ecc FLASH: failed to read content size 15728640 BOOTKERNEL partition, rc 3 With: FLASH: Cannot load BOOTKERNEL. Content is larger than the partition Cc: Nayna Jain <nayna@linux.ibm.com> Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- feel free to bikeshed the log message. --- core/flash.c | 11 +++++++++++ 1 file changed, 11 insertions(+)