The varstore shadow FV is kept in sync with actual SPI flash read, write and erase operations. Since we only expose a small slice of the SPI flash for the variable store, we keep an internal LBA offset and take it into account when translating shadow FV LBAs to actual LBAs.
As it turns out, the erase routine applies the LBA offset incorrectly, resulting in the wrong flash block being erased, and the wrong range to be erased in the shadow FV, which could result in a crash if the memory access is out of bounds.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/Drivers/StyxSpiFvDxe/StyxSpiFvDxe.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/Platforms/AMD/Styx/Drivers/StyxSpiFvDxe/StyxSpiFvDxe.c b/Platforms/AMD/Styx/Drivers/StyxSpiFvDxe/StyxSpiFvDxe.c index 03fd9e816b96..f544af3eeb2d 100644 --- a/Platforms/AMD/Styx/Drivers/StyxSpiFvDxe/StyxSpiFvDxe.c +++ b/Platforms/AMD/Styx/Drivers/StyxSpiFvDxe/StyxSpiFvDxe.c @@ -439,7 +439,6 @@ StyxSpiFvDxeErase ( for (Start = VA_ARG (Args, EFI_LBA); Start != EFI_LBA_LIST_TERMINATOR; Start = VA_ARG (Args, EFI_LBA)) { - Start += mNvStorageLbaOffset; Length = VA_ARG (Args, UINTN); Status = mIscpDxeProtocol->AmdExecuteEraseFvBlockDxe (mIscpDxeProtocol, (Start + mNvStorageLbaOffset) * BLOCK_SIZE,
On Fri, Mar 17, 2017 at 06:45:22PM +0000, Ard Biesheuvel wrote:
The varstore shadow FV is kept in sync with actual SPI flash read, write and erase operations. Since we only expose a small slice of the SPI flash for the variable store, we keep an internal LBA offset and take it into account when translating shadow FV LBAs to actual LBAs.
As it turns out, the erase routine applies the LBA offset incorrectly, resulting in the wrong flash block being erased, and the wrong range to be erased in the shadow FV, which could result in a crash if the memory access is out of bounds.
So, this is a very detailed and descriptive commmit message, but the fix is just basically "don't apply offset twice"?
Anyway, this is obviously a fix: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Platforms/AMD/Styx/Drivers/StyxSpiFvDxe/StyxSpiFvDxe.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/Platforms/AMD/Styx/Drivers/StyxSpiFvDxe/StyxSpiFvDxe.c b/Platforms/AMD/Styx/Drivers/StyxSpiFvDxe/StyxSpiFvDxe.c index 03fd9e816b96..f544af3eeb2d 100644 --- a/Platforms/AMD/Styx/Drivers/StyxSpiFvDxe/StyxSpiFvDxe.c +++ b/Platforms/AMD/Styx/Drivers/StyxSpiFvDxe/StyxSpiFvDxe.c @@ -439,7 +439,6 @@ StyxSpiFvDxeErase ( for (Start = VA_ARG (Args, EFI_LBA); Start != EFI_LBA_LIST_TERMINATOR; Start = VA_ARG (Args, EFI_LBA)) {
- Start += mNvStorageLbaOffset; Length = VA_ARG (Args, UINTN); Status = mIscpDxeProtocol->AmdExecuteEraseFvBlockDxe (mIscpDxeProtocol, (Start + mNvStorageLbaOffset) * BLOCK_SIZE,
-- 2.7.4
On 18 March 2017 at 14:48, Leif Lindholm leif.lindholm@linaro.org wrote:
On Fri, Mar 17, 2017 at 06:45:22PM +0000, Ard Biesheuvel wrote:
The varstore shadow FV is kept in sync with actual SPI flash read, write and erase operations. Since we only expose a small slice of the SPI flash for the variable store, we keep an internal LBA offset and take it into account when translating shadow FV LBAs to actual LBAs.
As it turns out, the erase routine applies the LBA offset incorrectly, resulting in the wrong flash block being erased, and the wrong range to be erased in the shadow FV, which could result in a crash if the memory access is out of bounds.
So, this is a very detailed and descriptive commmit message, but the fix is just basically "don't apply offset twice"?
Yes.
Anyway, this is obviously a fix: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org
Thanks
-
Ard Biesheuvel -
Leif Lindholm