Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34708 )
Change subject: arch/x86: Handle smm_subregion() failure ......................................................................
arch/x86: Handle smm_subregion() failure
The callers don't necessarily check return value of function. Make sure the parameters are not left uninitialised in that case.
Change-Id: Ic02db2d35b2ec88506320e7df609940de4aef005 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/amd/picasso/ramtop.c M src/soc/amd/stoneyridge/ramtop.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/memmap.c 8 files changed, 16 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/34708/1
diff --git a/src/soc/amd/picasso/ramtop.c b/src/soc/amd/picasso/ramtop.c index f936dfc..034c30a 100644 --- a/src/soc/amd/picasso/ramtop.c +++ b/src/soc/amd/picasso/ramtop.c @@ -131,6 +131,8 @@ clear_tvalid(); break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/amd/stoneyridge/ramtop.c b/src/soc/amd/stoneyridge/ramtop.c index de643cb..4ff22ec 100644 --- a/src/soc/amd/stoneyridge/ramtop.c +++ b/src/soc/amd/stoneyridge/ramtop.c @@ -131,6 +131,8 @@ clear_tvalid(); break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/apollolake/memmap.c b/src/soc/intel/apollolake/memmap.c index 7494481..17dfb3f 100644 --- a/src/soc/intel/apollolake/memmap.c +++ b/src/soc/intel/apollolake/memmap.c @@ -68,6 +68,8 @@ sub_size = cache_size; break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/braswell/memmap.c b/src/soc/intel/braswell/memmap.c index ab1d6f8..c3dcfbb 100644 --- a/src/soc/intel/braswell/memmap.c +++ b/src/soc/intel/braswell/memmap.c @@ -62,6 +62,8 @@ sub_size = cache_size; break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c index a80ff58..1a9df41 100644 --- a/src/soc/intel/cannonlake/memmap.c +++ b/src/soc/intel/cannonlake/memmap.c @@ -72,6 +72,8 @@ sub_size = ied_size; break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/denverton_ns/memmap.c b/src/soc/intel/denverton_ns/memmap.c index 3d750c7..416b16f 100644 --- a/src/soc/intel/denverton_ns/memmap.c +++ b/src/soc/intel/denverton_ns/memmap.c @@ -96,6 +96,8 @@ sub_size = cache_size; break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c index f8121b8..88debf7 100644 --- a/src/soc/intel/icelake/memmap.c +++ b/src/soc/intel/icelake/memmap.c @@ -70,6 +70,8 @@ sub_size = ied_size; break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c index 5008571..8350134 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/skylake/memmap.c @@ -74,6 +74,8 @@ sub_size = ied_size; break; default: + *start = 0; + *size = 0; return -1; }
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34708 )
Change subject: arch/x86: Handle smm_subregion() failure ......................................................................
Patch Set 4: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34708 )
Change subject: arch/x86: Handle smm_subregion() failure ......................................................................
Patch Set 5: Code-Review+2
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34708 )
Change subject: arch/x86: Handle smm_subregion() failure ......................................................................
arch/x86: Handle smm_subregion() failure
The callers don't necessarily check return value of function. Make sure the parameters are not left uninitialised in that case.
Change-Id: Ic02db2d35b2ec88506320e7df609940de4aef005 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34708 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/picasso/ramtop.c M src/soc/amd/stoneyridge/ramtop.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/memmap.c 8 files changed, 16 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Arthur Heymans: Looks good to me, approved
diff --git a/src/soc/amd/picasso/ramtop.c b/src/soc/amd/picasso/ramtop.c index 344b7f7..6b28ec7 100644 --- a/src/soc/amd/picasso/ramtop.c +++ b/src/soc/amd/picasso/ramtop.c @@ -130,6 +130,8 @@ clear_tvalid(); break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/amd/stoneyridge/ramtop.c b/src/soc/amd/stoneyridge/ramtop.c index 3a23df6..26d84ce 100644 --- a/src/soc/amd/stoneyridge/ramtop.c +++ b/src/soc/amd/stoneyridge/ramtop.c @@ -130,6 +130,8 @@ clear_tvalid(); break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/apollolake/memmap.c b/src/soc/intel/apollolake/memmap.c index 7494481..17dfb3f 100644 --- a/src/soc/intel/apollolake/memmap.c +++ b/src/soc/intel/apollolake/memmap.c @@ -68,6 +68,8 @@ sub_size = cache_size; break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/braswell/memmap.c b/src/soc/intel/braswell/memmap.c index 01594ea..51b7b36 100644 --- a/src/soc/intel/braswell/memmap.c +++ b/src/soc/intel/braswell/memmap.c @@ -62,6 +62,8 @@ sub_size = cache_size; break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c index b5b538c..004e35c 100644 --- a/src/soc/intel/cannonlake/memmap.c +++ b/src/soc/intel/cannonlake/memmap.c @@ -72,6 +72,8 @@ sub_size = ied_size; break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/denverton_ns/memmap.c b/src/soc/intel/denverton_ns/memmap.c index d4265e9..9507d7f 100644 --- a/src/soc/intel/denverton_ns/memmap.c +++ b/src/soc/intel/denverton_ns/memmap.c @@ -96,6 +96,8 @@ sub_size = cache_size; break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c index 046774f..13eb947 100644 --- a/src/soc/intel/icelake/memmap.c +++ b/src/soc/intel/icelake/memmap.c @@ -70,6 +70,8 @@ sub_size = ied_size; break; default: + *start = 0; + *size = 0; return -1; }
diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c index d6ab908..963a500 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/skylake/memmap.c @@ -74,6 +74,8 @@ sub_size = ied_size; break; default: + *start = 0; + *size = 0; return -1; }
-
Arthur Heymans (Code Review) -
Furquan Shaikh (Code Review)
-
Kyösti Mälkki (Code Review)