Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot

Heiko Schocher
2025-02-11T06:13:54Z

Hello Tom,

On 10.02.25 23:26, Tom Rini wrote:
> Here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Feb 10, 2025 at 4:12 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das U-Boot
> found with Coverity Scan.
>
> 3 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 3 of 3 defect(s)
>
>
> ** CID 541281: Insecure data handling (TAINTED_SCALAR)
> /lib/tpm-v2.c: 77 in tpm2_scan_masks()
>
>
> ________________________________________________________________________________________________________
> *** CID 541281: Insecure data handling (TAINTED_SCALAR)
> /lib/tpm-v2.c: 77 in tpm2_scan_masks()
> 71 *mask = 0;
> 72
> 73 rc = tpm2_get_pcr_info(dev, &pcrs);
> 74 if (rc)
> 75 return rc;
> 76
>>>> CID 541281: Insecure data handling (TAINTED_SCALAR)
>>>> Using tainted variable "pcrs.count" as a loop boundary.
> 77 for (i = 0; i < pcrs.count; i++) {
> 78 struct tpms_pcr_selection *sel = &pcrs.selection[i];
> 79 size_t j;
> 80 u32 hash_mask = 0;
> 81
> 82 for (j = 0; j < ARRAY_SIZE(hash_algo_list); j++) {
>
> ** CID 541280: Insecure data handling (TAINTED_SCALAR)
> /cmd/tpm-v2.c: 307 in do_tpm2_pcrallocate()
>
>
> ________________________________________________________________________________________________________
> *** CID 541280: Insecure data handling (TAINTED_SCALAR)
> /cmd/tpm-v2.c: 307 in do_tpm2_pcrallocate()
> 301 * first call
> 302 */
> 303 ret = tpm2_get_pcr_info(dev, &pcr);
> 304 if (ret)
> 305 return ret;
> 306
>>>> CID 541280: Insecure data handling (TAINTED_SCALAR)
>>>> Using tainted variable "pcr.count" as a loop boundary.
> 307 for (i = 0; i < pcr.count; i++) {
> 308 struct tpms_pcr_selection *sel =
> &pcr.selection[i];
> 309 const char *name;
> 310
> 311 if (!tpm2_is_active_bank(sel))
> 312 continue;
>
> ** CID 541279: (TAINTED_SCALAR)
> /drivers/led/led-uclass.c: 284 in led_get_function_name()
> /drivers/led/led-uclass.c: 279 in led_get_function_name()
>
>
> ________________________________________________________________________________________________________
> *** CID 541279: (TAINTED_SCALAR)
> /drivers/led/led-uclass.c: 284 in led_get_function_name()
> 278 if (!ret) {
> 279 snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
> 280 "%s:%s-%d",
> 281 cp ? "" : led_colors[color],
> 282 func ? func : "", enumerator);
> 283 } else {
>>>> CID 541279: (TAINTED_SCALAR)
>>>> Using tainted variable "color" as an index into an array
> "led_colors".
> 284 snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
> 285 "%s:%s",
> 286 cp ? "" : led_colors[color],
> 287 func ? func : "");
> 288 }
> 289 uc_plat->label = uc_plat->name;
> /drivers/led/led-uclass.c: 279 in led_get_function_name()
> 273 /* Now try to detect function label name */
> 274 func = dev_read_string(dev, "function");
> 275 cp = dev_read_u32(dev, "color", &color);
> 276 if (cp == 0 || func) {
> 277 ret = dev_read_u32(dev, "function-enumerator",
> &enumerator);
> 278 if (!ret) {
>>>> CID 541279: (TAINTED_SCALAR)
>>>> Using tainted variable "color" as an index into an array
> "led_colors".
> 279 snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
> 280 "%s:%s-%d",
> 281 cp ? "" : led_colors[color],
> 282 func ? func : "", enumerator);
> 283 } else {
> 284 snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
>
>
> ----- End forwarded message -----
>

Just a fast idea:

diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
index 27ef890ed0a..fc15a0811e0 100644
--- a/drivers/led/led-uclass.c
+++ b/drivers/led/led-uclass.c
@@ -273,6 +273,10 @@ static const char *led_get_function_name(struct udevice *dev)
/* Now try to detect function label name */
func = dev_read_string(dev, "function");
cp = dev_read_u32(dev, "color", &color);
+ // prevent coverity scan error CID 541279: (TAINTED_SCALAR)
+ if ((color < LED_COLOR_ID_WHITE) || (color >= LED_COLOR_ID_MAX))
+ cp = -EINVAL;
+
if (cp == 0 || func) {
ret = dev_read_u32(dev, "function-enumerator", &enumerator);
if (!ret) {

If okay, I can send a patch for this.

Or may better, we move this check into a new function:

int dev_read_min_max_u32(const struct udevice *dev, u32 min, u32 max, const char *propname, u32 *outp)

which returns -EINVAL, if readden value is not in [min, max] range?

So may this function can be used at other places too?

Thanks!

bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs@denx.de