smmstore: Verify userspace-provided pointer to protect SMM
Use the introduced functions and verify pointers in the SMMSTORE. Make sure to not overwrite or leak data from SMM and update the documentation as well. Change-Id: I70df08657c3fa0f98917742d8e1a6cb1077e3758 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Christian Walter <christian.walter@9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41085 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Patrick Georgi <pgeorgi@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This commit is contained in:
parent
41fec869fb
commit
2fdabd9038
|
@ -5,7 +5,7 @@ storage driver.
|
|||
|
||||
## SMMSTORE
|
||||
|
||||
SMMSTORE is a SMM mediated driver to read from, write to and erase a
|
||||
SMMSTORE is a [SMM] mediated driver to read from, write to and erase a
|
||||
predefined region in flash. It can be enabled by setting
|
||||
`CONFIG_SMMSTORE=y` in menuconfig.
|
||||
|
||||
|
@ -117,7 +117,18 @@ INPUT:
|
|||
- `val`: pointer to the value data
|
||||
- `valsize`: size of the value data
|
||||
|
||||
#### Security
|
||||
|
||||
Pointers provided by the payload or OS are checked to not overlap with the SMM.
|
||||
That protects the SMM handler from being manipulated.
|
||||
|
||||
*However there's no validation done on the source or destination pointing to
|
||||
DRAM. A malicious application that is able to issue SMIs could extract arbitrary
|
||||
data or modify the currently running kernel.*
|
||||
|
||||
## External links
|
||||
|
||||
* [A Tour Beyond BIOS Implementing UEFI Authenticated Variables in SMM with EDKI](https://software.intel.com/sites/default/files/managed/cf/ea/a_tour_beyond_bios_implementing_uefi_authenticated_variables_in_smm_with_edkii.pdf)
|
||||
Note, this differs significantly from coreboot's implementation.
|
||||
|
||||
[SMM]: ../security/smm.md
|
||||
|
|
|
@ -1,6 +1,8 @@
|
|||
/* SPDX-License-Identifier: GPL-2.0-only */
|
||||
|
||||
#include <console/console.h>
|
||||
#include <commonlib/region.h>
|
||||
#include <cpu/x86/smm.h>
|
||||
#include <smmstore.h>
|
||||
#include <stddef.h>
|
||||
#include <stdint.h>
|
||||
|
@ -10,13 +12,14 @@
|
|||
*
|
||||
* Legal means:
|
||||
* - not pointing into SMRAM
|
||||
* - ...?
|
||||
*
|
||||
* returns 0 on success, -1 on failure
|
||||
*/
|
||||
static int range_check(void *start, size_t size)
|
||||
{
|
||||
// TODO: fill in
|
||||
if (smm_points_to_smram(start, size))
|
||||
return -1;
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
@ -30,6 +33,9 @@ uint32_t smmstore_exec(uint8_t command, void *param)
|
|||
printk(BIOS_DEBUG, "Reading from SMM store\n");
|
||||
struct smmstore_params_read *params = param;
|
||||
|
||||
if (range_check(params, sizeof(*params)) != 0)
|
||||
break;
|
||||
|
||||
if (range_check(params->buf, params->bufsize) != 0)
|
||||
break;
|
||||
|
||||
|
@ -42,6 +48,8 @@ uint32_t smmstore_exec(uint8_t command, void *param)
|
|||
printk(BIOS_DEBUG, "Appending into SMM store\n");
|
||||
struct smmstore_params_append *params = param;
|
||||
|
||||
if (range_check(params, sizeof(*params)) != 0)
|
||||
break;
|
||||
if (range_check(params->key, params->keysize) != 0)
|
||||
break;
|
||||
if (range_check(params->val, params->valsize) != 0)
|
||||
|
|
Loading…
Reference in New Issue