Fwd: New Defects reported by Coverity Scan for Das U-Boot
Heinrich Schuchardt
xypron.glpk at gmx.de
Sun Jan 28 09:51:35 CET 2024
On 1/27/24 21:56, Heinrich Schuchardt wrote:
>
>
> Am 27. Januar 2024 16:40:18 MEZ schrieb Tom Rini <trini at konsulko.com>:
>> Hey, I'll just pass this on directly rather than to the list.
>>
>> ---------- Forwarded message ---------
>> From: <scan-admin at 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 at 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
More information about the U-Boot
mailing list