Fwd: New Defects reported by Coverity Scan for Das U-Boot
Tom Rini
trini at konsulko.com
Tue Feb 11 23:30:42 CET 2025
On Tue, Feb 11, 2025 at 07:14:19AM +0100, Heiko Schocher wrote:
> 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.
This is probably fine, thanks.
> 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?
It would be good to spend some time looking at the codebase to see what
sort of generic wrapper may or may not help first I think.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250211/faff7502/attachment.sig>
More information about the U-Boot
mailing list