Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push uhcidxe coverity issues #4856

Merged
merged 2 commits into from
Sep 22, 2023

Conversation

hwu25
Copy link
Member

@hwu25 hwu25 commented Sep 22, 2023

Patch series sent at:
https://edk2.groups.io/g/devel/message/107788 - [PATCH v2 0/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix issues pointed by Coverity
https://edk2.groups.io/g/devel/message/107789 - [PATCH v2 1/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix BAD_SHIFT Coverity issue
https://edk2.groups.io/g/devel/message/107790 - [PATCH v2 2/2] MdeModulePkg/Bus/Pci/UhciDxe: Fix FORWARD_NULL Coverity issues

Cover-letter:
v1 -> v2:

Update patch wrt BAD_SHIFT changes as per review comments
Ranbir Singh (2):
MdeModulePkg/Bus/Pci/UhciDxe: Fix BAD_SHIFT Coverity issue
MdeModulePkg/Bus/Pci/UhciDxe: Fix FORWARD_NULL Coverity issues

MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c | 10 +++++++++-
MdeModulePkg/Bus/Pci/UhciDxe/UsbHcMem.c | 9 +++++++++
2 files changed, 18 insertions(+), 1 deletion(-)

@hwu25 hwu25 added the push Auto push patch series in PR if all checks pass label Sep 22, 2023
Ranbir Singh and others added 2 commits September 22, 2023 12:23
The function UhciConvertPollRate has a check

    ASSERT (Interval != 0);

but this comes into play only in DEBUG mode. In Release mode, there is
no handling if the Interval parameter value is ZERO. To avoid shifting
by a negative amount later in the code flow in this undesirable case,
it is better to handle it as well by treating it same as if 1 is sent.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4211

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Co-authored-by: Veeresh Sangolli <veeresh.sangolli@dellteam.com>
Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
Signed-off-by: Ranbir Singh <rsingh@ventanamicro.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
The function UsbHcGetPciAddressForHostMem has

  ASSERT ((Block != NULL)); and

and the function UsbHcFreeMem has

  ASSERT (Block != NULL);

statement after for loop, but these are applicable only in DEBUG mode.
In RELEASE mode, if for whatever reasons there is no match inside for
loop and the loop exits because of Block != NULL; condition, then there
is no "Block" NULL pointer check afterwards and the code proceeds to do
dereferencing "Block" which will lead to CRASH.

Hence, for safety add NULL pointer checks always.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4211

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Co-authored-by: Veeresh Sangolli <veeresh.sangolli@dellteam.com>
Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
Signed-off-by: Ranbir Singh <rsingh@ventanamicro.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
@hwu25 hwu25 force-pushed the push_uhcidxe_coverity_issues branch from fcf01a7 to 3512c13 Compare September 22, 2023 04:23
@mergify mergify bot merged commit 28a267a into tianocore:master Sep 22, 2023
123 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
push Auto push patch series in PR if all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant