John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33152
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
src/soc/intel: Fix Coverity scan report
Coverity detects pointer mem_info as NULL_RETURNS. Add sanity check for mem_info to prevent null pointer dereference.
BUG=CID 1401394 TEST=Built and boot up to kernel.
Change-Id: I9d78ab38b8b2dd3734e0143acfd88d9093f16ce6 Signed-off-by: John Zhao john.zhao@intel.com --- M src/soc/intel/broadwell/romstage/raminit.c 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/33152/1
diff --git a/src/soc/intel/broadwell/romstage/raminit.c b/src/soc/intel/broadwell/romstage/raminit.c index fc8b7c6..ffa1feb 100644 --- a/src/soc/intel/broadwell/romstage/raminit.c +++ b/src/soc/intel/broadwell/romstage/raminit.c @@ -122,6 +122,10 @@
printk(BIOS_DEBUG, "create cbmem for dimm information\n"); mem_info = cbmem_add(CBMEM_ID_MEMINFO, sizeof(struct memory_info)); + + if (!mem_info) + return; + memset(mem_info, 0, sizeof(*mem_info)); /* Translate pei_memory_info struct data into memory_info struct */ mem_info->dimm_cnt = pei_data->meminfo.dimm_cnt;
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/33152/1/src/soc/intel/broadwell/romstage/ram... File src/soc/intel/broadwell/romstage/raminit.c:
https://review.coreboot.org/#/c/33152/1/src/soc/intel/broadwell/romstage/ram... PS1, Line 125: trailing whitespace
https://review.coreboot.org/#/c/33152/1/src/soc/intel/broadwell/romstage/ram... PS1, Line 128: trailing whitespace
https://review.coreboot.org/#/c/33152/1/src/soc/intel/broadwell/romstage/ram... PS1, Line 128: please, no spaces at the start of a line
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33152
to look at the new patch set (#2).
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
src/soc/intel: Fix Coverity scan report
Coverity detects pointer mem_info as NULL_RETURNS. Add sanity check for mem_info to prevent null pointer dereference.
BUG=CID 1401394 TEST=Built and boot up to kernel.
Change-Id: I9d78ab38b8b2dd3734e0143acfd88d9093f16ce6 Signed-off-by: John Zhao john.zhao@intel.com --- M src/soc/intel/broadwell/romstage/raminit.c 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/33152/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/33152/2/src/soc/intel/broadwell/romstage/ram... File src/soc/intel/broadwell/romstage/raminit.c:
https://review.coreboot.org/#/c/33152/2/src/soc/intel/broadwell/romstage/ram... PS2, Line 126: if (!mem_info) Probably add a print indicating error adding cbmem entry?
Hello Balaji Manigandan, Lijian Zhao, build bot (Jenkins), Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33152
to look at the new patch set (#3).
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
src/soc/intel: Fix Coverity scan report
Coverity detects pointer mem_info as NULL_RETURNS. Add sanity check for mem_info to prevent null pointer dereference.
BUG=CID 1401394 TEST=Built and boot up to kernel.
Change-Id: I9d78ab38b8b2dd3734e0143acfd88d9093f16ce6 Signed-off-by: John Zhao john.zhao@intel.com --- M src/soc/intel/broadwell/romstage/raminit.c 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/33152/3
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
Patch Set 3:
(1 comment)
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/33152/2/src/soc/intel/broadwell/romstage/ram... File src/soc/intel/broadwell/romstage/raminit.c:
https://review.coreboot.org/#/c/33152/2/src/soc/intel/broadwell/romstage/ram... PS2, Line 126: if (!mem_info)
Probably add a print indicating error adding cbmem entry?
done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
Patch Set 3: Code-Review+2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
Patch Set 3: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
Patch Set 3: Code-Review-1
(1 comment)
https://review.coreboot.org/#/c/33152/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33152/3//COMMIT_MSG@7 PS3, Line 7: src/soc/intel: Fix Coverity scan report This summary is not useful. Please describe, what is fixed, and just mention the tool as a tag in the commit message body.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
Patch Set 3: Code-Review-1
(1 comment)
https://review.coreboot.org/#/c/33152/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33152/3//COMMIT_MSG@7 PS3, Line 7: src/soc/intel: Fix Coverity scan report
This summary is not useful. […]
Thanks Paul, I missed that. Maybe:
soc/intel: Avoid NULL pointer dereference.
Furquan Shaikh has removed a vote on this change.
Change subject: src/soc/intel: Fix Coverity scan report ......................................................................
Removed Code-Review+2 by Furquan Shaikh furquan@google.com
Hello Balaji Manigandan, Paul Menzel, build bot (Jenkins), Lijian Zhao, Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33152
to look at the new patch set (#4).
Change subject: src/soc/intel: Avoid NULL pointer dereference ......................................................................
src/soc/intel: Avoid NULL pointer dereference
Coverity detects pointer mem_info as NULL_RETURNS. Add sanity check for mem_info to prevent NULL pointer dereference.
BUG=CID 1401394 TEST=Built and boot up to kernel.
Change-Id: I9d78ab38b8b2dd3734e0143acfd88d9093f16ce6 Signed-off-by: John Zhao john.zhao@intel.com --- M src/soc/intel/broadwell/romstage/raminit.c 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/33152/4
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Avoid NULL pointer dereference ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/33152/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33152/3//COMMIT_MSG@7 PS3, Line 7: src/soc/intel: Fix Coverity scan report
Thanks Paul, I missed that. Maybe: […]
done
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Avoid NULL pointer dereference ......................................................................
Patch Set 4: Code-Review+2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Avoid NULL pointer dereference ......................................................................
Patch Set 4:
Paul's request has been addressed, but just changing the commit message doesn't remove the -1.
Marting
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33152 )
Change subject: src/soc/intel: Avoid NULL pointer dereference ......................................................................
src/soc/intel: Avoid NULL pointer dereference
Coverity detects pointer mem_info as NULL_RETURNS. Add sanity check for mem_info to prevent NULL pointer dereference.
BUG=CID 1401394 TEST=Built and boot up to kernel.
Change-Id: I9d78ab38b8b2dd3734e0143acfd88d9093f16ce6 Signed-off-by: John Zhao john.zhao@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33152 Reviewed-by: Martin Roth martinroth@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/broadwell/romstage/raminit.c 1 file changed, 6 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved
Objections: Paul Menzel: I would prefer that you didn't submit this
diff --git a/src/soc/intel/broadwell/romstage/raminit.c b/src/soc/intel/broadwell/romstage/raminit.c index fc8b7c6..c13761d 100644 --- a/src/soc/intel/broadwell/romstage/raminit.c +++ b/src/soc/intel/broadwell/romstage/raminit.c @@ -122,6 +122,12 @@
printk(BIOS_DEBUG, "create cbmem for dimm information\n"); mem_info = cbmem_add(CBMEM_ID_MEMINFO, sizeof(struct memory_info)); + + if (!mem_info) { + printk(BIOS_ERR, "Error! Failed to add mem_info to cbmem\n"); + return; + } + memset(mem_info, 0, sizeof(*mem_info)); /* Translate pei_memory_info struct data into memory_info struct */ mem_info->dimm_cnt = pei_data->meminfo.dimm_cnt;
-
build bot (Jenkins) (Code Review) -
Furquan Shaikh (Code Review)
-
John Zhao (Code Review)
-
Martin Roth (Code Review)
-
Paul Menzel (Code Review)