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

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 1/27/24 21:56, Heinrich Schuchardt wrote:
>
>
> Am 27. Januar 2024 16:40:18 MEZ schrieb Tom Rini <trini@konsulko.com>:
>> Hey, I'll just pass this on directly rather than to the list.
>>
>> ---------- Forwarded message ---------
>> From: <scan-admin@coverity.com>
>> Date: Sat, Jan 27, 2024 at 10:36 AM
>> 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.
>>
>> 1 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>>
>>
>> New defect(s) Reported-by: Coverity Scan
>> Showing 1 of 1 defect(s)
>>
>>
>> ** CID 479279:    (TAINTED_SCALAR)
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 479279:    (TAINTED_SCALAR)
>> /cmd/smbios.c: 180 in do_smbios()
>> 174                             smbios_print_type2((struct smbios_type2 *)pos);
>> 175                             break;
>> 176                     case 127:
>> 177                             smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178                             break;
>> 179                     default:
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>> 180                             smbios_print_generic(pos);
>> 181                             break;
>> 182                     }
>> 183             }
>> 184
>> 185             return CMD_RET_SUCCESS;
>> /cmd/smbios.c: 154 in do_smbios()
>> 148                     size = entry2->length;
>> 149                     max_struct_size = entry2->max_struct_size;
>> 150             } else {
>> 151                     log_err("Unknown SMBIOS anchor format\n");
>> 152                     return CMD_RET_FAILURE;
>> 153             }
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "size" to "table_compute_checksum", which uses it as a loop boundary.
>> 154             if (table_compute_checksum(entry, size)) {
>> 155                     log_err("Invalid anchor checksum\n");
>> 156                     return CMD_RET_FAILURE;
>> 157             }
>> 158             printf("SMBIOS %s present.\n", version);
>> 159
>> /cmd/smbios.c: 174 in do_smbios()
>> 168                            (unsigned long long)map_to_sysmem(pos));
>> 169                     switch (pos->type) {
>> 170                     case 1:
>> 171                             smbios_print_type1((struct smbios_type1 *)pos);
>> 172                             break;
>> 173                     case 2:
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>> 174                             smbios_print_type2((struct smbios_type2 *)pos);
>> 175                             break;
>> 176                     case 127:
>> 177                             smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178                             break;
>> 179                     default:
>> /cmd/smbios.c: 154 in do_smbios()
>> 148                     size = entry2->length;
>> 149                     max_struct_size = entry2->max_struct_size;
>> 150             } else {
>> 151                     log_err("Unknown SMBIOS anchor format\n");
>> 152                     return CMD_RET_FAILURE;
>> 153             }
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "size" to "table_compute_checksum", which uses it as a loop boundary.
>> 154             if (table_compute_checksum(entry, size)) {
>> 155                     log_err("Invalid anchor checksum\n");
>> 156                     return CMD_RET_FAILURE;
>> 157             }
>> 158             printf("SMBIOS %s present.\n", version);
>> 159
>> /cmd/smbios.c: 180 in do_smbios()
>> 174                             smbios_print_type2((struct smbios_type2 *)pos);
>> 175                             break;
>> 176                     case 127:
>> 177                             smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178                             break;
>> 179                     default:
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>> 180                             smbios_print_generic(pos);
>> 181                             break;
>> 182                     }
>> 183             }
>> 184
>> 185             return CMD_RET_SUCCESS;
>> /cmd/smbios.c: 174 in do_smbios()
>> 168                            (unsigned long long)map_to_sysmem(pos));
>> 169                     switch (pos->type) {
>> 170                     case 1:
>> 171                             smbios_print_type1((struct smbios_type1 *)pos);
>> 172                             break;
>> 173                     case 2:
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>> 174                             smbios_print_type2((struct smbios_type2 *)pos);
>> 175                             break;
>> 176                     case 127:
>> 177                             smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178                             break;
>> 179                     default:
>> /cmd/smbios.c: 174 in do_smbios()
>> 168                            (unsigned long long)map_to_sysmem(pos));
>> 169                     switch (pos->type) {
>> 170                     case 1:
>> 171                             smbios_print_type1((struct smbios_type1 *)pos);
>> 172                             break;
>> 173                     case 2:
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>> 174                             smbios_print_type2((struct smbios_type2 *)pos);
>> 175                             break;
>> 176                     case 127:
>> 177                             smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178                             break;
>> 179                     default:
>> /cmd/smbios.c: 180 in do_smbios()
>> 174                             smbios_print_type2((struct smbios_type2 *)pos);
>> 175                             break;
>> 176                     case 127:
>> 177                             smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178                             break;
>> 179                     default:
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>> 180                             smbios_print_generic(pos);
>> 181                             break;
>> 182                     }
>> 183             }
>> 184
>> 185             return CMD_RET_SUCCESS;
>> /cmd/smbios.c: 180 in do_smbios()
>> 174                             smbios_print_type2((struct smbios_type2 *)pos);
>> 175                             break;
>> 176                     case 127:
>> 177                             smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178                             break;
>> 179                     default:
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>> 180                             smbios_print_generic(pos);
>> 181                             break;
>> 182                     }
>> 183             }
>> 184
>> 185             return CMD_RET_SUCCESS;
>> /cmd/smbios.c: 174 in do_smbios()
>> 168                            (unsigned long long)map_to_sysmem(pos));
>> 169                     switch (pos->type) {
>> 170                     case 1:
>> 171                             smbios_print_type1((struct smbios_type1 *)pos);
>> 172                             break;
>> 173                     case 2:
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>> 174                             smbios_print_type2((struct smbios_type2 *)pos);
>> 175                             break;
>> 176                     case 127:
>> 177                             smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178                             break;
>> 179                     default:
>> /cmd/smbios.c: 180 in do_smbios()
>> 174                             smbios_print_type2((struct smbios_type2 *)pos);
>> 175                             break;
>> 176                     case 127:
>> 177                             smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178                             break;
>> 179                     default:
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "pos->length" to "smbios_print_generic", which uses it as a loop boundary.
>> 180                             smbios_print_generic(pos);
>> 181                             break;
>> 182                     }
>> 183             }
>> 184
>> 185             return CMD_RET_SUCCESS;
>> /cmd/smbios.c: 174 in do_smbios()
>> 168                            (unsigned long long)map_to_sysmem(pos));
>> 169                     switch (pos->type) {
>> 170                     case 1:
>> 171                             smbios_print_type1((struct smbios_type1 *)pos);
>> 172                             break;
>> 173                     case 2:
>>>>>      CID 479279:    (TAINTED_SCALAR)
>>>>>      Passing tainted expression "((struct smbios_type2 *)pos)->number_contained_objects" to "smbios_print_type2", which uses it as a loop boundary.
>> 174                             smbios_print_type2((struct smbios_type2 *)pos);
>> 175                             break;
>> 176                     case 127:
>> 177                             smbios_print_type127((struct
>> smbios_type127 *)pos);
>> 178                             break;
>> 179                     default:
>>
>
> The values may come from QEMU, so may be "tainted". We could check the length of the individual structures against the total size of the SMBIOS table.
>

In Coverity I marked this as false positive with the following comment:

"The only case in which the data is tainted is when copying the smbios
table from a prior firmware state when running as EFI app or from QEMU.
Sanity checks should not be in the smbios command but where we import
the table."

Best regards

Heinrich