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

[Heiko Schocher]

Hello Tom,

On 10.02.25 23:26, Tom Rini wrote:
> Here's the latest report.
> 
> ---------- Forwarded message ---------
> From: <scan-admin at 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 at 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 at denx.de