Change in coreboot[master]: soc/intel/tigerlake: Avoid NULL pointer dereference

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

John Zhao (Code Review)

3 Mar 2020 3 Mar '20

3:55 p.m.

John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39261 )

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

soc/intel/tigerlake: Avoid NULL pointer dereference

Coverity detects pointer dev as FORWARD_NULL. Add sanity check for dev to prevent NULL pointer dereference.

BUG=CID 1420208 TEST=Built image successfully.

Change-Id: I2a62da44c7044f9dc281eae0949f7f7b612ab238 Signed-off-by: John Zhao john.zhao@intel.com --- M src/soc/intel/tigerlake/romstage/fsp_params_jsl.c 1 file changed, 4 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39261/1

diff --git a/src/soc/intel/tigerlake/romstage/fsp_params_jsl.c b/src/soc/intel/tigerlake/romstage/fsp_params_jsl.c index 56124f4..09b06f1 100644 --- a/src/soc/intel/tigerlake/romstage/fsp_params_jsl.c +++ b/src/soc/intel/tigerlake/romstage/fsp_params_jsl.c @@ -95,7 +95,10 @@ m_cfg->DdiPortCDdc = config->DdiPortCDdc;

/* Audio */ - m_cfg->PchHdaEnable = pcidev_path_on_root(PCH_DEVFN_HDA) ? dev->enabled : 0; + if (dev) { + m_cfg->PchHdaEnable = pcidev_path_on_root(PCH_DEVFN_HDA) ? dev->enable + : 0; + }

m_cfg->PchHdaDspEnable = config->PchHdaDspEnable; m_cfg->PchHdaAudioLinkHdaEnable = config->PchHdaAudioLinkHdaEnable;

Attachments:

attachment.htm (text/html — 2.9 KB)

Show replies by date

John Zhao (Code Review)

3 Mar 3 Mar

4:04 p.m.

Hello build bot (Jenkins), Martin Roth, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39261

to look at the new patch set (#2).

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

soc/intel/tigerlake: Avoid NULL pointer dereference

Coverity detects pointer dev as FORWARD_NULL. Add sanity check for dev to prevent NULL pointer dereference.

BUG=CID 1420208 TEST=Built image successfully.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39261/2

John Zhao (Code Review)

4:30 p.m.

Hello build bot (Jenkins), Martin Roth, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39261

to look at the new patch set (#3).

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

soc/intel/tigerlake: Avoid NULL pointer dereference

Coverity detects pointer dev as FORWARD_NULL. Add sanity check for dev to prevent NULL pointer dereference.

BUG=CID 1420208 TEST=Built image successfully.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39261/3

John Zhao (Code Review)

6:33 p.m.

Hello build bot (Jenkins), Martin Roth, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39261

to look at the new patch set (#4).

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

soc/intel/tigerlake: Avoid NULL pointer dereference

Coverity detects pointer dev as FORWARD_NULL. Add sanity check for dev to prevent NULL pointer dereference.

BUG=CID 1420208 TEST=Built image successfully.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39261/4

Angel Pons (Code Review)

6:37 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39261 )

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

Patch Set 4:

(1 comment)

Is this a duplicate of

https://review.coreboot.org/c/coreboot/+/39261/2/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params_jsl.c:

https://review.coreboot.org/c/coreboot/+/39261/2/src/soc/intel/tigerlake/rom... PS2, Line 98: dev What is this supposed to be? The IGD?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39261 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a62da44c7044f9dc281eae0949f7f7b612ab238 Gerrit-Change-Number: 39261 Gerrit-PatchSet: 4 Gerrit-Owner: John Zhao john.zhao@intel.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-Comment-Date: Tue, 03 Mar 2020 21:37:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

John Zhao (Code Review)

6:53 p.m.

Hello build bot (Jenkins), Martin Roth, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39261

to look at the new patch set (#5).

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

soc/intel/tigerlake: Avoid NULL pointer dereference

Coverity detects pointer dev as FORWARD_NULL. Add sanity check for dev to prevent NULL pointer dereference.

BUG=CID 1420208 TEST=Built image successfully.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39261/5

John Zhao (Code Review)

6:55 p.m.

John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39261 )

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39261/2/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params_jsl.c:

https://review.coreboot.org/c/coreboot/+/39261/2/src/soc/intel/tigerlake/rom... PS2, Line 98: dev

...

What is this supposed to be? The IGD?

Thanks for pointing the mistake. dev should be HDA instead of IGD.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39261 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a62da44c7044f9dc281eae0949f7f7b612ab238 Gerrit-Change-Number: 39261 Gerrit-PatchSet: 5 Gerrit-Owner: John Zhao john.zhao@intel.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Angel Pons th3fanbus@gmail.com Gerrit-Comment-Date: Tue, 03 Mar 2020 21:55:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

6 Mar 6 Mar

3:52 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39261 )

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

Patch Set 5: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/39261/5//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39261/5//COMMIT_MSG@9 PS5, Line 9: Coverity detects pointer dev as FORWARD_NULL. Add sanity check Maybe add that "dev" did not point to the audio device.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39261 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a62da44c7044f9dc281eae0949f7f7b612ab238 Gerrit-Change-Number: 39261 Gerrit-PatchSet: 5 Gerrit-Owner: John Zhao john.zhao@intel.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 06 Mar 2020 18:52:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

John Zhao (Code Review)

4:05 p.m.

Hello build bot (Jenkins), Martin Roth, Angel Pons, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39261

to look at the new patch set (#6).

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

soc/intel/tigerlake: Avoid NULL pointer dereference

Coverity detects pointer dev as FORWARD_NULL. Add sanity check for dev to prevent NULL pointer dereference if dev did not point to the audio device.

BUG=CID 1420208 TEST=Built image successfully.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39261/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/39261 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a62da44c7044f9dc281eae0949f7f7b612ab238 Gerrit-Change-Number: 39261 Gerrit-PatchSet: 6 Gerrit-Owner: John Zhao john.zhao@intel.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

John Zhao (Code Review)

4:05 p.m.

John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39261 )

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39261/5//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/39261/5//COMMIT_MSG@9 PS5, Line 9: Coverity detects pointer dev as FORWARD_NULL. Add sanity check

...

Maybe add that "dev" did not point to the audio device.

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/39261 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a62da44c7044f9dc281eae0949f7f7b612ab238 Gerrit-Change-Number: 39261 Gerrit-PatchSet: 6 Gerrit-Owner: John Zhao john.zhao@intel.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 06 Mar 2020 19:05:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Patrick Georgi (Code Review)

7 Mar 7 Mar

5:46 p.m.

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39261 )

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

soc/intel/tigerlake: Avoid NULL pointer dereference

Coverity detects pointer dev as FORWARD_NULL. Add sanity check for dev to prevent NULL pointer dereference if dev did not point to the audio device.

BUG=CID 1420208 TEST=Built image successfully.

Change-Id: I2a62da44c7044f9dc281eae0949f7f7b612ab238 Signed-off-by: John Zhao john.zhao@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39261 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/tigerlake/romstage/fsp_params_jsl.c 1 file changed, 5 insertions(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved

diff --git a/src/soc/intel/tigerlake/romstage/fsp_params_jsl.c b/src/soc/intel/tigerlake/romstage/fsp_params_jsl.c index 56124f4..9c70f2e 100644 --- a/src/soc/intel/tigerlake/romstage/fsp_params_jsl.c +++ b/src/soc/intel/tigerlake/romstage/fsp_params_jsl.c @@ -95,7 +95,11 @@ m_cfg->DdiPortCDdc = config->DdiPortCDdc;

/* Audio */ - m_cfg->PchHdaEnable = pcidev_path_on_root(PCH_DEVFN_HDA) ? dev->enabled : 0; + dev = pcidev_path_on_root(PCH_DEVFN_HDA); + if (!dev) + m_cfg->PchHdaEnable = 0; + else + m_cfg->PchHdaEnable = dev->enabled;

m_cfg->PchHdaDspEnable = config->PchHdaDspEnable; m_cfg->PchHdaAudioLinkHdaEnable = config->PchHdaAudioLinkHdaEnable;

-- To view, visit https://review.coreboot.org/c/coreboot/+/39261 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a62da44c7044f9dc281eae0949f7f7b612ab238 Gerrit-Change-Number: 39261 Gerrit-PatchSet: 7 Gerrit-Owner: John Zhao john.zhao@intel.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

9elements QA (Code Review)

6:05 p.m.

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39261 )

Change subject: soc/intel/tigerlake: Avoid NULL pointer dereference ......................................................................

Patch Set 7:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/1194 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1193 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1192

Please note: This test is under development and might not be accurate at all!

-- To view, visit https://review.coreboot.org/c/coreboot/+/39261 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a62da44c7044f9dc281eae0949f7f7b612ab238 Gerrit-Change-Number: 39261 Gerrit-PatchSet: 7 Gerrit-Owner: John Zhao john.zhao@intel.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 07 Mar 2020 21:05:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1871

days inactive

1875

days old

coreboot-gerrit@coreboot.org

11 comments

4 participants

tags (0)

participants (4)

9elements QA (Code Review)
Angel Pons (Code Review)
John Zhao (Code Review)
Patrick Georgi (Code Review)