linux-kernel - [PATCH v3] RAS/AMD/ATL: Fix unintended sign extension issue from coverity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20241127-coverity1593397signextension-v3-1-60dd0c4287ff@gmail.com>
Date: Wed, 27 Nov 2024 18:23:48 +0000
From: Karan Sanghavi <karansanghvi98@...il.com>
To: Yazen Ghannam <Yazen.Ghannam@....com>, Tony Luck <tony.luck@...el.com>, 
 Borislav Petkov <bp@...en8.de>
Cc: linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org, 
 Shuah Khan <skhan@...uxfoundation.org>, 
 Yazen Ghannam <yazen.ghannam@....com>, 
 Karan Sanghavi <karansanghvi98@...il.com>
Subject: [PATCH v3] RAS/AMD/ATL: Fix unintended sign extension issue from
 coverity

This error is reported by coverity scan stating as

CID 1593397: (#1 of 1): Unintended sign extension (SIGN_EXTENSION)
sign_extension: Suspicious implicit sign extension: pc
with type u16 (16 bits, unsigned) is promoted in
pc << bit_shifts.pc to type int (32 bits, signed),
then sign-extended to type unsigned long (64 bits, unsigned).
If pc << bit_shifts.pc is greater than 0x7FFFFFFF,
the upper bits of the result will all be 1.

Use u32 for bitwise operations to prevent unintentional
sign extension by assigning the u16 value to a u32
variable before performing the bitwise operation to
avoid unintended sign extension and maintain
consistency with the existing code style.

Reviewed-by: Yazen Ghannam <yazen.ghannam@....com>
Signed-off-by: Karan Sanghavi <karansanghvi98@...il.com>
---
Coverity  Link: 
https://scan7.scan.coverity.com/#/project-view/51975/11354?selectedIssue=1593397
---
Changes in v3:
- Updated commit summary log
- Link to v2: https://lore.kernel.org/r/20241108-coverity1593397signextension-v2-1-4acdf3968d2d@gmail.com

Changes in v2:
- Assigning pc value to temp variable before left shifting as mentioned
  in feedback rather then typecasting pc to u32. 
- Link to v1: https://lore.kernel.org/r/20241104-coverity1593397signextension-v1-1-4cfae6532140@gmail.com
---
 drivers/ras/amd/atl/umc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ras/amd/atl/umc.c b/drivers/ras/amd/atl/umc.c
index dc8aa12f63c8..3f4b1f31e14f 100644
--- a/drivers/ras/amd/atl/umc.c
+++ b/drivers/ras/amd/atl/umc.c
@@ -293,7 +293,8 @@ static unsigned long convert_dram_to_norm_addr_mi300(unsigned long addr)
 	}
 
 	/* PC bit */
-	addr |= pc << bit_shifts.pc;
+	temp = pc;
+	addr |= temp << bit_shifts.pc;
 
 	/* SID bits */
 	for (i = 0; i < NUM_SID_BITS; i++) {

---
base-commit: 81983758430957d9a5cb3333fe324fd70cf63e7e
change-id: 20241104-coverity1593397signextension-78c9b2c21d51

Best regards,
-- 
Karan Sanghavi <karansanghvi98@...il.com>