Skip to content

Commit

Permalink
EmbeddedPkg: CodeQL Fixes.
Browse files Browse the repository at this point in the history
Makes changes to comply with alerts raised by CodeQL.

Most of the issues here fall into the following two categories:

1. Potential use of uninitialized pointer.
2. Inconsistent integer width in comparison.

Co-authored-by: Taylor Beebe <31827475+TaylorBeebe@users.noreply.github.com>
Co-authored-by: kenlautner <85201046+kenlautner@users.noreply.github.com>
Co-authored-by: Bret Barkelew <bret@corthon.com>
  • Loading branch information
4 people authored and apop5 committed Aug 23, 2024
1 parent a92830e commit 0704fc9
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 6 deletions.
17 changes: 16 additions & 1 deletion EmbeddedPkg/Library/FdtLib/fdt_overlay.c
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,13 @@ overlay_update_local_node_references (
);
int tree_child;

// MU_CHANGE Start - CodeQL change - unguardednullreturndereference
if (fixup_child_name == NULL) {
return -FDT_ERR_BADOVERLAY;
}

// MU_CHANGE End - CodeQL change - unguardednullreturndereference

tree_child = fdt_subnode_offset (
fdto,
tree_node,
Expand Down Expand Up @@ -758,7 +765,9 @@ overlay_apply_node (
&name,
&prop_len
);
if (prop_len == -FDT_ERR_NOTFOUND) {
// MU_CHANGE Start - CodeQL change - unguardednullreturndereference
if ((prop == NULL) || (prop_len == -FDT_ERR_NOTFOUND)) {
// MU_CHANGE End - CodeQL change - unguardednullreturndereference
return -FDT_ERR_INTERNAL;
}

Expand All @@ -777,6 +786,12 @@ overlay_apply_node (
int nnode;
int ret;

// MU_CHANGE Start - CodeQL change - unguardednullreturndereference
if (name == NULL) {
return -FDT_ERR_INTERNAL;
}

// MU_CHANGE End - CodeQL change - unguardednullreturndereference
nnode = fdt_add_subnode (fdt, target, name);
if (nnode == -FDT_ERR_EXISTS) {
nnode = fdt_subnode_offset (fdt, target, name);
Expand Down
5 changes: 4 additions & 1 deletion EmbeddedPkg/Library/FdtLib/fdt_rw.c
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,13 @@ _fdt_splice (
char *p = splicepoint;
char *end = (char *)fdt + _fdt_data_size (fdt);

if (((p + oldlen) < p) || ((p + oldlen) > end)) {
// MU_CHANGE Start - CodeQL change - badoverflowguard
if ((oldlen > (uintptr_t)(end - p)) || (end < p)) {
return -FDT_ERR_BADOFFSET;
}

// MU_CHANGE End - CodeQL change - badoverflowguard

if ((p < (char *)fdt) || ((end - oldlen + newlen) < (char *)fdt)) {
return -FDT_ERR_BADOFFSET;
}
Expand Down
6 changes: 5 additions & 1 deletion EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,8 @@ DmaFreeBuffer (
BOOLEAN Found;
EFI_STATUS Status;

Alloc = NULL; // MU_CHANGE - CodeQL change - conditionallyuninitializedvariable

if (HostAddress == NULL) {
return EFI_INVALID_PARAMETER;
}
Expand All @@ -626,7 +628,9 @@ DmaFreeBuffer (
}
}

if (!Found) {
// MU_CHANGE Start - CodeQL change - unguardednullreturndereference
if (!Found || (Alloc == NULL)) {
// MU_CHANGE End - CodeQL change - unguardednullreturndereference
ASSERT (FALSE);
return EFI_INVALID_PARAMETER;
}
Expand Down
5 changes: 5 additions & 0 deletions EmbeddedPkg/Library/PrePiHobLib/Hob.c
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,12 @@ BuildGuidDataHob (
ASSERT (Data != NULL || DataLength == 0);

HobData = BuildGuidHob (Guid, DataLength);
// MU_CHANGE Start - CodeQL change - unguardednullreturndereference
if (HobData == NULL) {
return NULL;
}

// MU_CHANGE End - CodeQL change - unguardednullreturndereference
return CopyMem (HobData, Data, DataLength);
}

Expand Down
7 changes: 4 additions & 3 deletions EmbeddedPkg/Library/PrePiLib/FwVol.c
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,9 @@ FindFileEx (

FileOffset = (UINT32)((UINT8 *)FfsFileHeader - (UINT8 *)FwVolHeader);
ASSERT (FileOffset <= 0xFFFFFFFF);

while (FileOffset < (FvLength - sizeof (EFI_FFS_FILE_HEADER))) {
// MU_CHANGE Start - CodeQL change - comparison-with-wider-type
while (FileOffset < (UINT32)(FvLength - sizeof (EFI_FFS_FILE_HEADER))) {
// MU_CHANGE End - CodeQL change - comparison-with-wider-type
//
// Get FileState which is the highest bit of the State
//
Expand Down Expand Up @@ -283,7 +284,7 @@ FfsProcessSection (
{
EFI_STATUS Status;
UINT32 SectionLength;
UINT32 ParsedLength;
UINTN ParsedLength; // MU_CHANGE - CodeQL change - comparison-with-wider-type
EFI_COMPRESSION_SECTION *CompressionSection;
EFI_COMPRESSION_SECTION2 *CompressionSection2;
UINT32 DstBufferSize;
Expand Down
6 changes: 6 additions & 0 deletions EmbeddedPkg/SimpleTextInOutSerial/SimpleTextInOut.c
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,12 @@ OutputString (

Size = StrLen (String) + 1;
OutputString = AllocatePool (Size);
// MU_CHANGE Start - CodeQL change - unguardednullreturndereference
if (OutputString == NULL) {
return EFI_OUT_OF_RESOURCES;
}

// MU_CHANGE End - CodeQL change - unguardednullreturndereference

// If there is any non-ascii characters in String buffer then replace it with '?'
// Eventually, UnicodeStrToAsciiStr API should be fixed.
Expand Down

0 comments on commit 0704fc9

Please sign in to comment.