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

[Heinrich Schuchardt]

On 1/27/24 21:56, Heinrich Schuchardt wrote:

Am 27. Januar 2024 16:40:18 MEZ schrieb Tom Rini <tr...@konsulko.com>:

Hey, I'll just pass this on directly rather than to the list.

---------- Forwarded message ---------
From: <scan-ad...@coverity.com>
Date: Sat, Jan 27, 2024 at 10:36 AM
Subject: New Defects reported by Coverity Scan for Das U-Boot
To: <tom.r...@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