u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
@ 2022-08-24 11:40 Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2022-08-24 11:40 UTC (permalink / raw)
  To: u-boot, Simon Glass

[-- Attachment #1: Type: text/plain, Size: 12495 bytes --]

A bit behind on forwarding these along.

----- Forwarded message from Tom Rini <tom.rini@gmail.com> -----

Date: Wed, 24 Aug 2022 07:38:46 -0400
From: Tom Rini <tom.rini@gmail.com>
To: trini@konsulko.com
Subject: Fwd: New Defects reported by Coverity Scan for Das U-Boot

---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Aug 8, 2022 at 8:51 PM
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.

6 new defect(s) introduced to Das U-Boot found with Coverity Scan.
2 defect(s), reported by Coverity Scan earlier, were marked fixed in
the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 6 of 6 defect(s)


** CID 355771:    (PRINTF_ARGS)


________________________________________________________________________________________________________
*** CID 355771:    (PRINTF_ARGS)
/test/cmd/fdt.c: 72 in fdt_test_addr()
66      ut_assertok(run_command("fdt addr", 0));
67      ut_assert_nextline("Working fdt: %08lx", (ulong)map_to_sysmem(fdt));
68      ut_assertok(ut_check_console_end(uts));
69
70      /* Set the working FDT */
71      set_working_fdt_addr(0);
>>>     CID 355771:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
72      ut_assertok(run_commandf("fdt addr %08x", addr));
73      ut_asserteq(addr, map_to_sysmem(working_fdt));
74      ut_assertok(ut_check_console_end(uts));
75      set_working_fdt_addr(0);
76
77      /* Set the working FDT */
/test/cmd/fdt.c: 89 in fdt_test_addr()
83      ut_assertok(ret);
84      ut_asserteq(addr, map_to_sysmem(new_fdt));
85      ut_assertok(ut_check_console_end(uts));
86
87      /* Test setting an invalid FDT */
88      fdt[0] = 123;
>>>     CID 355771:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
89      ut_asserteq(1, run_commandf("fdt addr %08x", addr));
90      ut_assert_nextline("libfdt fdt_check_header(): FDT_ERR_BADMAGIC");
91      ut_assertok(ut_check_console_end(uts));
92
93      /* Test detecting an invalid FDT */
94      fdt[0] = 123;
/test/cmd/fdt.c: 80 in fdt_test_addr()
74      ut_assertok(ut_check_console_end(uts));
75      set_working_fdt_addr(0);
76
77      /* Set the working FDT */
78      fdt_blob = gd->fdt_blob;
79      gd->fdt_blob = NULL;
>>>     CID 355771:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
80      ret = run_commandf("fdt addr -c %08x", addr);
81      new_fdt = gd->fdt_blob;
82      gd->fdt_blob = fdt_blob;
83      ut_assertok(ret);
84      ut_asserteq(addr, map_to_sysmem(new_fdt));
85      ut_assertok(ut_check_console_end(uts));

** CID 355770:  Insecure data handling  (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 355770:  Insecure data handling  (TAINTED_SCALAR)
/test/cmd/fdt.c: 37 in make_test_fdt()
31     static int make_test_fdt(struct unit_test_state *uts, void
*fdt, int size)
32     {
33      ut_assertok(fdt_create(fdt, size));
34      ut_assertok(fdt_finish_reservemap(fdt));
35      ut_assert(fdt_begin_node(fdt, "") >= 0);
36      ut_assertok(fdt_end_node(fdt));
>>>     CID 355770:  Insecure data handling  (TAINTED_SCALAR)
>>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
37      ut_assertok(fdt_finish(fdt));
38
39      return 0;
40     }
41
42     /* Test 'fdt addr' getting/setting address */

** CID 355769:    (PRINTF_ARGS)


________________________________________________________________________________________________________
*** CID 355769:    (PRINTF_ARGS)
/test/cmd/fdt.c: 121 in fdt_test_resize()
115             /* Test setting and resizing the working FDT to a larger size */
116             ut_assertok(console_record_reset_enable());
117             ut_assertok(run_commandf("fdt addr %08x %x", addr, newsize));
118             ut_assertok(ut_check_console_end(uts));
119
120             /* Try shrinking it */
>>>     CID 355769:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
121             ut_assertok(run_commandf("fdt addr %08x %x", addr,
sizeof(fdt) / 4));
122             ut_assert_nextline("New length %d < existing length
%d, ignoring",
123                                (int)sizeof(fdt) / 4, newsize);
124             ut_assertok(ut_check_console_end(uts));
125
126             /* ...quietly */
/test/cmd/fdt.c: 127 in fdt_test_resize()
121             ut_assertok(run_commandf("fdt addr %08x %x", addr,
sizeof(fdt) / 4));
122             ut_assert_nextline("New length %d < existing length
%d, ignoring",
123                                (int)sizeof(fdt) / 4, newsize);
124             ut_assertok(ut_check_console_end(uts));
125
126             /* ...quietly */
>>>     CID 355769:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
127             ut_assertok(run_commandf("fdt addr -q %08x %x", addr,
sizeof(fdt) / 4));
128             ut_assertok(ut_check_console_end(uts));
129
130             /* We cannot easily provoke errors in fdt_open_into(),
so ignore that */
131
132             return 0;
/test/cmd/fdt.c: 117 in fdt_test_resize()
111             ut_assertok(make_test_fdt(uts, fdt, sizeof(fdt)));
112             addr = map_to_sysmem(fdt);
113             set_working_fdt_addr(addr);
114
115             /* Test setting and resizing the working FDT to a larger size */
116             ut_assertok(console_record_reset_enable());
>>>     CID 355769:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
117             ut_assertok(run_commandf("fdt addr %08x %x", addr, newsize));
118             ut_assertok(ut_check_console_end(uts));
119
120             /* Try shrinking it */
121             ut_assertok(run_commandf("fdt addr %08x %x", addr,
sizeof(fdt) / 4));
122             ut_assert_nextline("New length %d < existing length
%d, ignoring",

** CID 355768:  Memory - illegal accesses  (UNINIT)
/drivers/core/lists.c: 252 in lists_bind_fdt()


________________________________________________________________________________________________________
*** CID 355768:  Memory - illegal accesses  (UNINIT)
/drivers/core/lists.c: 252 in lists_bind_fdt()
246                     }
247
248                     if (entry->of_match)
249                             log_debug("   - found match at '%s':
'%s' matches '%s'\n",
250                                       entry->name,
entry->of_match->compatible,
251                                       id->compatible);
>>>     CID 355768:  Memory - illegal accesses  (UNINIT)
>>>     Using uninitialized value "id".
252                     ret = device_bind_with_driver_data(parent, entry, name,
253                                                        id->data,
node, &dev);
254                     if (ret == -ENODEV) {
255                             log_debug("Driver '%s' refuses to
bind\n", entry->name);
256                             continue;
257                     }

** CID 355767:    (OVERRUN)
/lib/addr_map.c: 66 in addrmap_set_entry()
/lib/addr_map.c: 67 in addrmap_set_entry()
/lib/addr_map.c: 65 in addrmap_set_entry()


________________________________________________________________________________________________________
*** CID 355767:    (OVERRUN)
/lib/addr_map.c: 66 in addrmap_set_entry()
60                      phys_size_t size, int idx)
61     {
62      if (idx > CONFIG_SYS_NUM_ADDR_MAP)
63              return;
64
65      address_map[idx].vaddr = vaddr;
>>>     CID 355767:    (OVERRUN)
>>>     Overrunning array "address_map" of 16 16-byte elements at element index 16 (byte offset 271) using index "idx" (which evaluates to 16).
66      address_map[idx].paddr = paddr;
67      address_map[idx].size  = size;
/lib/addr_map.c: 67 in addrmap_set_entry()
61     {
62      if (idx > CONFIG_SYS_NUM_ADDR_MAP)
63              return;
64
65      address_map[idx].vaddr = vaddr;
66      address_map[idx].paddr = paddr;
>>>     CID 355767:    (OVERRUN)
>>>     Overrunning array "address_map" of 16 16-byte elements at element index 16 (byte offset 271) using index "idx" (which evaluates to 16).
67      address_map[idx].size  = size;
/lib/addr_map.c: 65 in addrmap_set_entry()
59     void addrmap_set_entry(unsigned long vaddr, phys_addr_t paddr,
60                      phys_size_t size, int idx)
61     {
62      if (idx > CONFIG_SYS_NUM_ADDR_MAP)
63              return;
64
>>>     CID 355767:    (OVERRUN)
>>>     Overrunning array "address_map" of 16 16-byte elements at element index 16 (byte offset 271) using index "idx" (which evaluates to 16).
65      address_map[idx].vaddr = vaddr;
66      address_map[idx].paddr = paddr;
67      address_map[idx].size  = size;

** CID 355766:    (PRINTF_ARGS)


________________________________________________________________________________________________________
*** CID 355766:    (PRINTF_ARGS)
/test/cmd/fdt.c: 121 in fdt_test_resize()
115             /* Test setting and resizing the working FDT to a larger size */
116             ut_assertok(console_record_reset_enable());
117             ut_assertok(run_commandf("fdt addr %08x %x", addr, newsize));
118             ut_assertok(ut_check_console_end(uts));
119
120             /* Try shrinking it */
>>>     CID 355766:    (PRINTF_ARGS)
>>>     Argument "64UL" to format specifier "%x" was expected to have type "unsigned int" but has type "unsigned long".
121             ut_assertok(run_commandf("fdt addr %08x %x", addr,
sizeof(fdt) / 4));
122             ut_assert_nextline("New length %d < existing length
%d, ignoring",
123                                (int)sizeof(fdt) / 4, newsize);
124             ut_assertok(ut_check_console_end(uts));
125
126             /* ...quietly */
/test/cmd/fdt.c: 127 in fdt_test_resize()
121             ut_assertok(run_commandf("fdt addr %08x %x", addr,
sizeof(fdt) / 4));
122             ut_assert_nextline("New length %d < existing length
%d, ignoring",
123                                (int)sizeof(fdt) / 4, newsize);
124             ut_assertok(ut_check_console_end(uts));
125
126             /* ...quietly */
>>>     CID 355766:    (PRINTF_ARGS)
>>>     Argument "64UL" to format specifier "%x" was expected to have type "unsigned int" but has type "unsigned long".
127             ut_assertok(run_commandf("fdt addr -q %08x %x", addr,
sizeof(fdt) / 4));
128             ut_assertok(ut_check_console_end(uts));
129
130             /* We cannot easily provoke errors in fdt_open_into(),
so ignore that */
131
132             return 0;


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit,
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yoA22WlOQ-2By3ieUvdbKmOyw68TMVT4Kip-2BBzfOGWXJ5yIiYplmPF9KAnKIja4Zd7tU-3DqIQf_EEm8SbLgSDsaDZif-2Bv7ch8WqhKpLoKErHi4nXpwDNTsut95EuJ1dF4QhvWDPMPg-2FzMzS9KRhGAV1f-2FUmmbNbALbLgedwSxyHFzv40zBZbQ-2BXdxVQrhyzuPqo1U5lwMDREo6ylUvUQ2f0J7eVecTS1Gig-2FQwwd7N1OvprpUa-2BpHy38-2BCNKgrt9q-2Fn03k8f5ttSRxORtyDMKSuD38GNNRlqw-3D-3D

  To manage Coverity Scan email notifications for
"tom.rini@gmail.com", click
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXxWeIHzDeopm-2BEWQ6S6K-2FtUHv9ZTk8qZbuzkkz9sa-2BJFzf226DuRd-2B2ygQlLnerl-2BA3jN1AOYejXZ-2FNZ62waJHedPFGpqqjTx8fawy9KPJBno-3DnIoA_EEm8SbLgSDsaDZif-2Bv7ch8WqhKpLoKErHi4nXpwDNTsut95EuJ1dF4QhvWDPMPg-2FMDA5j8o4o-2Bl7QSWv-2Fdquz5Ay0IUrCqd-2B1tBatlmw-2Bk6r7mDQmU6vjrCmsI0TrpSErVm17PM3p-2FVCVAk8-2FYnP-2B4Nf-2F0gam9xYjGZmoklMOMo7wgiMoo2HKgeyxJ3k66OEFYIC-2BDgf2IdTUhB-2FuHnR4A-3D-3D



-- 
Tom

----- End forwarded message -----

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
  2023-12-21 14:48 ` Shantur Rathore
@ 2023-12-21 16:08   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-12-21 16:08 UTC (permalink / raw)
  To: Shantur Rathore; +Cc: u-boot, Simon Glass

[-- Attachment #1: Type: text/plain, Size: 2700 bytes --]

On Thu, Dec 21, 2023 at 02:48:31PM +0000, Shantur Rathore wrote:
> Hi Tom,
> 
> On Mon, Dec 18, 2023 at 4:26 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > Here's the latest report.
> >
> > ---------- Forwarded message ---------
> > From: <scan-admin@coverity.com>
> > Date: Mon, Dec 18, 2023 at 8:42 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 470930:  Uninitialized variables  (UNINIT)
> > /boot/bootmeth_efi.c: 465 in distro_efi_boot()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 470930:  Uninitialized variables  (UNINIT)
> > /boot/bootmeth_efi.c: 465 in distro_efi_boot()
> > 459              */
> > 460             if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
> > 461                     log_debug("Booting with built-in fdt\n");
> > 462                     snprintf(cmd, sizeof(cmd), "bootefi %lx", kernel);
> > 463             } else {
> > 464                     log_debug("Booting with external fdt\n");
> > >>>     CID 470930:  Uninitialized variables  (UNINIT)
> > >>>     Using uninitialized value "fdt" when calling "snprintf". [Note: The source code implementation of the function has been overridden by a builtin model.]
> > 465                     snprintf(cmd, sizeof(cmd), "bootefi %lx %lx",
> > kernel, fdt);
> > 466             }
> > 467
> > 468             if (run_command(cmd, 0))
> > 469                     return log_msg_ret("run", -EINVAL);
> > 470
> >
> 
> The code in question is
> 
> if (!bootmeth_uses_network(bflow)) {
>   // snip
>   if (bflow->flags & ~BOOTFLOWF_USE_BUILTIN_FDT)
>     fdt = bflow->fdt_addr;
> } else {
>   // snip
>   fdt = env_get_hex("fdt_addr_r", 0);
> }
> //snip
> if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
>   log_debug("Booting with built-in fdt\n");
>   snprintf(cmd, sizeof(cmd), "bootefi %lx", kernel);
> } else {
>   log_debug("Booting with external fdt\n");
>   snprintf(cmd, sizeof(cmd), "bootefi %lx %lx", kernel, fdt);
> }
> 
> I am unsure in which case is fdt uninitialised.
> Unless the tool is being thrown off by different version of
> bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT
> check, I don't see the problem.
> 
> Please let me know if you see it.

Ah, right, thanks for re-checking.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
  2023-12-18 16:26 Tom Rini
@ 2023-12-21 14:48 ` Shantur Rathore
  2023-12-21 16:08   ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Shantur Rathore @ 2023-12-21 14:48 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Simon Glass

Hi Tom,

On Mon, Dec 18, 2023 at 4:26 PM Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Dec 18, 2023 at 8:42 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 470930:  Uninitialized variables  (UNINIT)
> /boot/bootmeth_efi.c: 465 in distro_efi_boot()
>
>
> ________________________________________________________________________________________________________
> *** CID 470930:  Uninitialized variables  (UNINIT)
> /boot/bootmeth_efi.c: 465 in distro_efi_boot()
> 459              */
> 460             if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
> 461                     log_debug("Booting with built-in fdt\n");
> 462                     snprintf(cmd, sizeof(cmd), "bootefi %lx", kernel);
> 463             } else {
> 464                     log_debug("Booting with external fdt\n");
> >>>     CID 470930:  Uninitialized variables  (UNINIT)
> >>>     Using uninitialized value "fdt" when calling "snprintf". [Note: The source code implementation of the function has been overridden by a builtin model.]
> 465                     snprintf(cmd, sizeof(cmd), "bootefi %lx %lx",
> kernel, fdt);
> 466             }
> 467
> 468             if (run_command(cmd, 0))
> 469                     return log_msg_ret("run", -EINVAL);
> 470
>

The code in question is

if (!bootmeth_uses_network(bflow)) {
  // snip
  if (bflow->flags & ~BOOTFLOWF_USE_BUILTIN_FDT)
    fdt = bflow->fdt_addr;
} else {
  // snip
  fdt = env_get_hex("fdt_addr_r", 0);
}
//snip
if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
  log_debug("Booting with built-in fdt\n");
  snprintf(cmd, sizeof(cmd), "bootefi %lx", kernel);
} else {
  log_debug("Booting with external fdt\n");
  snprintf(cmd, sizeof(cmd), "bootefi %lx %lx", kernel, fdt);
}

I am unsure in which case is fdt uninitialised.
Unless the tool is being thrown off by different version of
bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT
check, I don't see the problem.

Please let me know if you see it.

Kind regards,
Shantur

>
> --
> Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
@ 2023-12-18 16:26 Tom Rini
  2023-12-21 14:48 ` Shantur Rathore
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2023-12-18 16:26 UTC (permalink / raw)
  To: u-boot, Shantur Rathore, Simon Glass

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

Here's the latest report.

---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Dec 18, 2023 at 8:42 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 470930:  Uninitialized variables  (UNINIT)
/boot/bootmeth_efi.c: 465 in distro_efi_boot()


________________________________________________________________________________________________________
*** CID 470930:  Uninitialized variables  (UNINIT)
/boot/bootmeth_efi.c: 465 in distro_efi_boot()
459              */
460             if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
461                     log_debug("Booting with built-in fdt\n");
462                     snprintf(cmd, sizeof(cmd), "bootefi %lx", kernel);
463             } else {
464                     log_debug("Booting with external fdt\n");
>>>     CID 470930:  Uninitialized variables  (UNINIT)
>>>     Using uninitialized value "fdt" when calling "snprintf". [Note: The source code implementation of the function has been overridden by a builtin model.]
465                     snprintf(cmd, sizeof(cmd), "bootefi %lx %lx",
kernel, fdt);
466             }
467
468             if (run_command(cmd, 0))
469                     return log_msg_ret("run", -EINVAL);
470


-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
  2023-11-08  3:24 ` Alexander Gendin
@ 2023-11-08 14:29   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-11-08 14:29 UTC (permalink / raw)
  To: Alexander Gendin
  Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
	Eddie James, Kever Yang, Johan Jonker, AKASHI Takahiro,
	Etienne Carriere

[-- Attachment #1: Type: text/plain, Size: 2076 bytes --]

On Wed, Nov 08, 2023 at 03:24:38AM +0000, Alexander Gendin wrote:
> On Mon, Nov 06, 2023 at 03:27:52PM -0500, Tom Rini <trini@konsulko.com> wrote:
> 
> > Hey all,
> > 
> > Here's the latest report. I _think_ I passed the right options to
> > get_maintainer.pl such that it would only look far enough back in git to
> > find the likely authors (along with listed maintainers of the files).
> > 
> > ---------- Forwarded message ---------
> > From: <scan-admin@coverity.com>
> > Date: Mon, Nov 6, 2023 at 2:58 PM
> > 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.
> > 
> > 13 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> > 5 defect(s), reported by Coverity Scan earlier, were marked fixed in
> > the recent build analyzed by Coverity Scan.
> > 
> > New defect(s) Reported-by: Coverity Scan
> > Showing 13 of 13 defect(s)
> > 
> > <...skipped...>
> > *** CID 467404:  Control flow issues  (DEADCODE)
> > /test/cmd/mbr.c: 217 in build_mbr_parts()
> > 211                                             return 1;
> > 212                                     strcat(cur_buf, mbr_parts_p5);
> > 213                                     bytes_remaining -= cur_str_size;
> > 214
> > 215                             }
> > 216                             else if (num_parts > 5)
> > >>>     CID 467404:  Control flow issues  (DEADCODE)
> > >>>     Execution cannot reach this statement: "return 1U;".
> > 217                                     return 1;
> > 218                             }
> > 219                     }
> > 220             }
> > 221
> > 222             cur_str_size = sizeof(mbr_parts_tail);
> > <...skipped...>
> > 
> > -- 
> > Tom
> 
> Hi Tom,
> 
> Thanks for the report.
> 
> The patch to fix CID 467404 has been sent to the mailing list.

Thanks for the patch, I'll make sure to grab it before the next -rc.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
  2023-11-07 23:18 ` Johan Jonker
@ 2023-11-08 13:54   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-11-08 13:54 UTC (permalink / raw)
  To: Johan Jonker
  Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
	Eddie James, Kever Yang, AKASHI Takahiro, Etienne Carriere,
	Alexander Gendin

[-- Attachment #1: Type: text/plain, Size: 6718 bytes --]

On Wed, Nov 08, 2023 at 12:18:05AM +0100, Johan Jonker wrote:
> Hi Tom, Simon,
> 
> Please have a look some comments below at 3 issues that are introduced by meself. ;)

Thanks for looking.

> 
> On 11/6/23 21:27, Tom Rini wrote:
> > Hey all,
> > 
> > Here's the latest report. I _think_ I passed the right options to
> > get_maintainer.pl such that it would only look far enough back in git to
> > find the likely authors (along with listed maintainers of the files).
> > 
> > ---------- Forwarded message ---------
> > From: <scan-admin@coverity.com>
> > Date: Mon, Nov 6, 2023 at 2:58 PM
> > 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.
> > 
> > 13 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> > 5 defect(s), reported by Coverity Scan earlier, were marked fixed in
> > the recent build analyzed by Coverity Scan.
> > 
> > New defect(s) Reported-by: Coverity Scan
> > Showing 13 of 13 defect(s)
> > 
> > 
> > ** CID 467411:  Memory - corruptions  (OVERRUN)
> > 
> > 
> > ________________________________________________________________________________________________________
> 
> [..]
> 
> > *** CID 467407:  Uninitialized variables  (UNINIT)
> > /drivers/scsi/scsi.c: 612 in do_scsi_scan_one()
> > 606
> > 607             bdesc = dev_get_uclass_plat(bdev);
> > 608             bdesc->target = id;
> > 609             bdesc->lun = lun;
> > 610             bdesc->removable = bd.removable;
> > 611             bdesc->type = bd.type;
> >>>>     CID 467407:  Uninitialized variables  (UNINIT)
> >>>>     Using uninitialized value "bd.bb".
> > 612             bdesc->bb = bd.bb;
> > 613             memcpy(&bdesc->vendor, &bd.vendor, sizeof(bd.vendor));
> > 614             memcpy(&bdesc->product, &bd.product, sizeof(bd.product));
> > 615             memcpy(&bdesc->revision, &bd.revision,  sizeof(bd.revision));
> > 616             if (IS_ENABLED(CONFIG_SYS_BIG_ENDIAN)) {
> > 617                     ata_swap_buf_le16((u16 *)&bdesc->vendor,
> > sizeof(bd.vendor) / 2);
> > 
> > ** CID 467406:  Memory - corruptions  (OVERRUN)
> > 
> > 
> 
> Scsi devices are not my thing.
> I'm forced to poke in drivers, because someone else is plumbing bounce buffer code to our block class.

OK. So I think Marek might be better able to comment on why yes, we need
bounce buffers more often than we used to, for sanity sake. But...

> Introduced by:
> [PATCH v5 4/8] rockchip: block: blk-uclass: add bounce buffer flag to blk_desc
> https://lore.kernel.org/u-boot/ee332375-8812-8e12-da1c-9973d10a41af@gmail.com/
> 
> static void scsi_init_dev_desc_priv(struct blk_desc *dev_desc)
> {
> [..]
> 
> #if IS_ENABLED(CONFIG_BOUNCE_BUFFER)
> 	dev_desc->bb = true;
> #endif	/* CONFIG_BOUNCE_BUFFER */
> }
> 
> [..]
> 	struct blk_desc bd;
> 
> 	scsi_init_dev_desc_priv(&bd);
> 
> 	bdesc->bb = bd.bb;
> ===
> https://www.kernel.org/doc/html/latest/dev-tools/checkpatch.html
> 
> GLOBAL_INITIALISERS
> 
>     Global variables should not be initialized explicitly to 0 (or NULL, false, etc.). Your compiler (or rather your loader, which is responsible for zeroing out the relevant sections) automatically does it for you.
> INITIALISED_STATIC
> 
>     Static variables should not be initialized explicitly to zero. Your compiler (or rather your loader) automatically does it for you.
> 
> ===
> I assumed that variables are always zeroed in the blk_desc structure.
> The value of bb only matters if IS_ENABLED(CONFIG_BOUNCE_BUFFER).
> 
> For scsi:
> 	dev_desc->bb = IS_ENABLED(CONFIG_BOUNCE_BUFFER) ? true : false;
> 
> The scsi block class worked fine for years without bounce buffer. Do all scsi devices need that buffer all of a sudden? What didn't work then?
> 
> Please advise what is the best approach.

Popping back to do_scsi_scan_one(), 'bd' isn't a global variable, it's a
function variable. And before bounce buffers, scsi_init_dev_desc_priv()
which is called a bit earlier in the function that Coverity is talking
about here would initialize every member, as you note in your analysis.
But it doesn't set bb in the case of CONFIG_BOUNCE_BUFFER=n. A simple
fix to this would be for that function to memset everything to 0 an then
change any values it needs to set. I've done that locally and will post
and CC you for sanity testing all the same.

> ===
> There is a patch in review that changes this file. Better let that go in first before changing something here. What's the status?
> 
> [PATCH] scsi: Forceably finish migration to DM_SCSI
> https://lore.kernel.org/u-boot/20231028005951.1187616-1-trini@konsulko.com/

I've merged that to next this morning, but it doesn't change this part
of the code (it was removing legacy code).

> > ________________________________________________________________________________________________________
> 
> [..]
> 
> > ________________________________________________________________________________________________________
> > *** CID 467402:    (CHECKED_RETURN)
> > /drivers/block/rkmtd.c: 737 in rkmtd_init_plat()
> > 731
> > 732             debug("starting_lba           : %llu\n",
> > le64_to_cpu(plat->gpt_e->starting_lba));
> > 733             debug("ending_lba             : %llu\n",
> > le64_to_cpu(plat->gpt_e->ending_lba));
> > 734
> > 735             memcpy(plat->gpt_e->partition_type_guid.b,
> > &partition_basic_data_guid, 16);
> > 736
> 
> 
> >>>>     CID 467402:    (CHECKED_RETURN)
> >>>>     Calling "uuid_str_to_bin" without checking return value (as is done elsewhere 9 out of 11 times).
> > 737             uuid_str_to_bin(plat->uuid_part_str,
> > plat->gpt_e->unique_partition_guid.b,
> > 738                             UUID_STR_FORMAT_GUID);
> 
> Comment 2:
> 
> 	gen_rand_uuid_str(plat->uuid_disk_str, UUID_STR_FORMAT_GUID);
> 	uuid_str_to_bin(plat->uuid_part_str, plat->gpt_e->unique_partition_guid.b,
> 			UUID_STR_FORMAT_GUID);
> 
> 
> The function uuid_str_to_bin() gets a string from gen_rand_uuid_str() which is guarantied correct.
> Checking the output is unnecessary. 
>  
> 	if (!uuid_str_valid(uuid_str)) {
> 		return -EINVAL;
> 	}
> 
> This is more a false positive. What should we do with it?

In Coverity scan terms "I know what I did here and it's right" is I
think "Intentional" and not "False Positive" (where that's supposed to
mean the tool is wrong and should have known better). Thanks for
explaining, I'll go mark it as such in the dashboard for both cases.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
  2023-11-06 20:27 Tom Rini
  2023-11-07 11:18 ` Ilias Apalodimas
  2023-11-07 23:18 ` Johan Jonker
@ 2023-11-08  3:24 ` Alexander Gendin
  2023-11-08 14:29   ` Tom Rini
  2 siblings, 1 reply; 20+ messages in thread
From: Alexander Gendin @ 2023-11-08  3:24 UTC (permalink / raw)
  To: Tom Rini
  Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, Simon Glass,
	Eddie James, Kever Yang, Johan Jonker, AKASHI Takahiro,
	Etienne Carriere

On Mon, Nov 06, 2023 at 03:27:52PM -0500, Tom Rini <trini@konsulko.com> wrote:

> Hey all,
> 
> Here's the latest report. I _think_ I passed the right options to
> get_maintainer.pl such that it would only look far enough back in git to
> find the likely authors (along with listed maintainers of the files).
> 
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Nov 6, 2023 at 2:58 PM
> 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.
> 
> 13 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 5 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
> 
> New defect(s) Reported-by: Coverity Scan
> Showing 13 of 13 defect(s)
> 
> <...skipped...>
> *** CID 467404:  Control flow issues  (DEADCODE)
> /test/cmd/mbr.c: 217 in build_mbr_parts()
> 211                                             return 1;
> 212                                     strcat(cur_buf, mbr_parts_p5);
> 213                                     bytes_remaining -= cur_str_size;
> 214
> 215                             }
> 216                             else if (num_parts > 5)
> >>>     CID 467404:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach this statement: "return 1U;".
> 217                                     return 1;
> 218                             }
> 219                     }
> 220             }
> 221
> 222             cur_str_size = sizeof(mbr_parts_tail);
> <...skipped...>
> 
> -- 
> Tom

Hi Tom,

Thanks for the report.

The patch to fix CID 467404 has been sent to the mailing list.

Regards,
Alex

-- 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
  2023-11-06 20:27 Tom Rini
  2023-11-07 11:18 ` Ilias Apalodimas
@ 2023-11-07 23:18 ` Johan Jonker
  2023-11-08 13:54   ` Tom Rini
  2023-11-08  3:24 ` Alexander Gendin
  2 siblings, 1 reply; 20+ messages in thread
From: Johan Jonker @ 2023-11-07 23:18 UTC (permalink / raw)
  To: Tom Rini, u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass, Eddie James,
	Kever Yang, AKASHI Takahiro, Etienne Carriere, Alexander Gendin

Hi Tom, Simon,

Please have a look some comments below at 3 issues that are introduced by meself. ;)

On 11/6/23 21:27, Tom Rini wrote:
> Hey all,
> 
> Here's the latest report. I _think_ I passed the right options to
> get_maintainer.pl such that it would only look far enough back in git to
> find the likely authors (along with listed maintainers of the files).
> 
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Nov 6, 2023 at 2:58 PM
> 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.
> 
> 13 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 5 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
> 
> New defect(s) Reported-by: Coverity Scan
> Showing 13 of 13 defect(s)
> 
> 
> ** CID 467411:  Memory - corruptions  (OVERRUN)
> 
> 
> ________________________________________________________________________________________________________

[..]

> *** CID 467407:  Uninitialized variables  (UNINIT)
> /drivers/scsi/scsi.c: 612 in do_scsi_scan_one()
> 606
> 607             bdesc = dev_get_uclass_plat(bdev);
> 608             bdesc->target = id;
> 609             bdesc->lun = lun;
> 610             bdesc->removable = bd.removable;
> 611             bdesc->type = bd.type;
>>>>     CID 467407:  Uninitialized variables  (UNINIT)
>>>>     Using uninitialized value "bd.bb".
> 612             bdesc->bb = bd.bb;
> 613             memcpy(&bdesc->vendor, &bd.vendor, sizeof(bd.vendor));
> 614             memcpy(&bdesc->product, &bd.product, sizeof(bd.product));
> 615             memcpy(&bdesc->revision, &bd.revision,  sizeof(bd.revision));
> 616             if (IS_ENABLED(CONFIG_SYS_BIG_ENDIAN)) {
> 617                     ata_swap_buf_le16((u16 *)&bdesc->vendor,
> sizeof(bd.vendor) / 2);
> 
> ** CID 467406:  Memory - corruptions  (OVERRUN)
> 
> 

Scsi devices are not my thing.
I'm forced to poke in drivers, because someone else is plumbing bounce buffer code to our block class.


Introduced by:
[PATCH v5 4/8] rockchip: block: blk-uclass: add bounce buffer flag to blk_desc
https://lore.kernel.org/u-boot/ee332375-8812-8e12-da1c-9973d10a41af@gmail.com/

static void scsi_init_dev_desc_priv(struct blk_desc *dev_desc)
{
[..]

#if IS_ENABLED(CONFIG_BOUNCE_BUFFER)
	dev_desc->bb = true;
#endif	/* CONFIG_BOUNCE_BUFFER */
}

[..]
	struct blk_desc bd;

	scsi_init_dev_desc_priv(&bd);

	bdesc->bb = bd.bb;
===
https://www.kernel.org/doc/html/latest/dev-tools/checkpatch.html

GLOBAL_INITIALISERS

    Global variables should not be initialized explicitly to 0 (or NULL, false, etc.). Your compiler (or rather your loader, which is responsible for zeroing out the relevant sections) automatically does it for you.
INITIALISED_STATIC

    Static variables should not be initialized explicitly to zero. Your compiler (or rather your loader) automatically does it for you.

===
I assumed that variables are always zeroed in the blk_desc structure.
The value of bb only matters if IS_ENABLED(CONFIG_BOUNCE_BUFFER).

For scsi:
	dev_desc->bb = IS_ENABLED(CONFIG_BOUNCE_BUFFER) ? true : false;

The scsi block class worked fine for years without bounce buffer. Do all scsi devices need that buffer all of a sudden? What didn't work then?

Please advise what is the best approach.

===
There is a patch in review that changes this file. Better let that go in first before changing something here. What's the status?

[PATCH] scsi: Forceably finish migration to DM_SCSI
https://lore.kernel.org/u-boot/20231028005951.1187616-1-trini@konsulko.com/

> ________________________________________________________________________________________________________

[..]

> ________________________________________________________________________________________________________
> *** CID 467402:    (CHECKED_RETURN)
> /drivers/block/rkmtd.c: 737 in rkmtd_init_plat()
> 731
> 732             debug("starting_lba           : %llu\n",
> le64_to_cpu(plat->gpt_e->starting_lba));
> 733             debug("ending_lba             : %llu\n",
> le64_to_cpu(plat->gpt_e->ending_lba));
> 734
> 735             memcpy(plat->gpt_e->partition_type_guid.b,
> &partition_basic_data_guid, 16);
> 736


>>>>     CID 467402:    (CHECKED_RETURN)
>>>>     Calling "uuid_str_to_bin" without checking return value (as is done elsewhere 9 out of 11 times).
> 737             uuid_str_to_bin(plat->uuid_part_str,
> plat->gpt_e->unique_partition_guid.b,
> 738                             UUID_STR_FORMAT_GUID);

Comment 2:

	gen_rand_uuid_str(plat->uuid_disk_str, UUID_STR_FORMAT_GUID);
	uuid_str_to_bin(plat->uuid_part_str, plat->gpt_e->unique_partition_guid.b,
			UUID_STR_FORMAT_GUID);


The function uuid_str_to_bin() gets a string from gen_rand_uuid_str() which is guarantied correct.
Checking the output is unnecessary. 
 
	if (!uuid_str_valid(uuid_str)) {
		return -EINVAL;
	}

This is more a false positive. What should we do with it?

> 739
> 740             efiname_len = sizeof(plat->gpt_e->partition_name) /
> sizeof(efi_char16_t);
> 741             dosname_len = sizeof(name);
> 742
> /drivers/block/rkmtd.c: 755 in rkmtd_init_plat()
> 749             plat->gpt_h->header_size = cpu_to_le32(sizeof(gpt_header));
> 750             plat->gpt_h->first_usable_lba = cpu_to_le64(64);
> 751             plat->gpt_h->last_usable_lba = cpu_to_le64(LBA - 34);
> 752             plat->gpt_h->num_partition_entries = cpu_to_le32(1);
> 753             plat->gpt_h->sizeof_partition_entry =
> cpu_to_le32(sizeof(gpt_entry));
> 754


>>>>     CID 467402:    (CHECKED_RETURN)
>>>>     Calling "uuid_str_to_bin" without checking return value (as is done elsewhere 9 out of 11 times).
> 755             uuid_str_to_bin(plat->uuid_disk_str, plat->gpt_h->disk_guid.b,
> 756                             UUID_STR_FORMAT_GUID);

Comment 3:
Dito as comment 2.


> 757
> 758             plat->gpt_h->partition_entry_array_crc32 = 0;
> 759             calc_crc32 = efi_crc32((const unsigned char *)plat->gpt_e,
> 760
> le32_to_cpu(plat->gpt_h->num_partition_entries) *
> 
> ** CID 467401:  Memory - corruptions  (OVERRUN)
> /drivers/firmware/scmi/sandbox-scmi_agent.c: 629 in sandbox_scmi_pwd_state_set()
> 
> 
[..]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
  2023-11-06 20:27 Tom Rini
@ 2023-11-07 11:18 ` Ilias Apalodimas
  2023-11-07 23:18 ` Johan Jonker
  2023-11-08  3:24 ` Alexander Gendin
  2 siblings, 0 replies; 20+ messages in thread
From: Ilias Apalodimas @ 2023-11-07 11:18 UTC (permalink / raw)
  To: Tom Rini, Eddie James
  Cc: u-boot, Heinrich Schuchardt, Simon Glass, Kever Yang,
	Johan Jonker, AKASHI Takahiro, Etienne Carriere,
	Alexander Gendin

Hi Tom,

Thanks for the report.
Eddie, can you please check the TCG related ones?

Thanks
/Ilias

On Mon, 6 Nov 2023 at 22:27, Tom Rini <trini@konsulko.com> wrote:
>
> Hey all,
>
> Here's the latest report. I _think_ I passed the right options to
> get_maintainer.pl such that it would only look far enough back in git to
> find the likely authors (along with listed maintainers of the files).
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Nov 6, 2023 at 2:58 PM
> 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.
>
> 13 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 5 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 13 of 13 defect(s)
>
>
> ** CID 467411:  Memory - corruptions  (OVERRUN)
>
>
> ________________________________________________________________________________________________________
> *** CID 467411:  Memory - corruptions  (OVERRUN)
> /lib/efi_loader/efi_tcg2.c: 1395 in efi_tcg2_measure_efi_app_invocation()
> 1389
> 1390            ret = tcg2_measure_gpt_data(dev, handle);
> 1391            if (ret != EFI_SUCCESS)
> 1392                    goto out;
> 1393
> 1394            for (pcr_index = 0; pcr_index <= 7; pcr_index++) {
> >>>     CID 467411:  Memory - corruptions  (OVERRUN)
> >>>     Overrunning buffer pointed to by "(u8 *)&event" of 4 bytes by passing it to a function which accesses it at byte offset 63.
> 1395                    ret = measure_event(dev, pcr_index, EV_SEPARATOR,
> 1396                                        sizeof(event), (u8 *)&event);
> 1397                    if (ret != EFI_SUCCESS)
> 1398                            goto out;
> 1399            }
> 1400
>
> ** CID 467410:    (TAINTED_SCALAR)
>
>
> ________________________________________________________________________________________________________
> *** CID 467410:    (TAINTED_SCALAR)
> /lib/efi_loader/efi_tcg2.c: 1385 in efi_tcg2_measure_efi_app_invocation()
> 1379                                (u8 *)EFI_CALLING_EFI_APPLICATION);
> 1380            if (ret != EFI_SUCCESS)
> 1381                    goto out;
> 1382
> 1383            entry = (struct smbios_entry *)find_smbios_table();
> 1384            if (entry) {
> >>>     CID 467410:    (TAINTED_SCALAR)
> >>>     Passing tainted expression "entry->struct_table_length" to "tcg2_measure_smbios", which uses it as an offset.
> 1385                    ret = tcg2_measure_smbios(dev, entry);
> 1386                    if (ret != EFI_SUCCESS)
> 1387                            goto out;
> 1388            }
> 1389
> 1390            ret = tcg2_measure_gpt_data(dev, handle);
> /lib/efi_loader/efi_tcg2.c: 1385 in efi_tcg2_measure_efi_app_invocation()
> 1379                                (u8 *)EFI_CALLING_EFI_APPLICATION);
> 1380            if (ret != EFI_SUCCESS)
> 1381                    goto out;
> 1382
> 1383            entry = (struct smbios_entry *)find_smbios_table();
> 1384            if (entry) {
> >>>     CID 467410:    (TAINTED_SCALAR)
> >>>     Passing tainted expression "entry->struct_count" to "tcg2_measure_smbios", which uses it as a loop boundary.
> 1385                    ret = tcg2_measure_smbios(dev, entry);
> 1386                    if (ret != EFI_SUCCESS)
> 1387                            goto out;
> 1388            }
> 1389
> 1390            ret = tcg2_measure_gpt_data(dev, handle);
>
> ** CID 467409:  Uninitialized variables  (UNINIT)
>
>
> ________________________________________________________________________________________________________
> *** CID 467409:  Uninitialized variables  (UNINIT)
> /test/boot/measurement.c: 48 in measure()
> 42      for (i = 0; i < size; ++i) {
> 43              kernel[i] = 0xf0 | (i & 0xf);
> 44              initrd[i] = (i & 0xf0) | 0xf;
> 45              images.ft_addr[i] = i & 0xff;
> 46      }
> 47
> >>>     CID 467409:  Uninitialized variables  (UNINIT)
> >>>     Using uninitialized value "images.os.os" when calling "bootm_measure".
> 48      ut_assertok(bootm_measure(&images));
> 49
> 50      free(images.ft_addr);
> 51      free(initrd);
> 52      free(kernel);
> 53
>
> ** CID 467408:  Insecure data handling  (TAINTED_SCALAR)
>
>
> ________________________________________________________________________________________________________
> *** CID 467408:  Insecure data handling  (TAINTED_SCALAR)
> /boot/bootm.c: 826 in do_bootm_states()
> 820                             env_set_hex("initrd_end", images->initrd_end);
> 821                     }
> 822             }
> 823     #endif
> 824     #if CONFIG_IS_ENABLED(OF_LIBFDT) && defined(CONFIG_LMB)
> 825             if (!ret && (states & BOOTM_STATE_FDT)) {
> >>>     CID 467408:  Insecure data handling  (TAINTED_SCALAR)
> >>>     Passing tainted expression "*images->ft_addr" to "boot_fdt_add_mem_rsv_regions", which uses it as an offset.
> 826                     boot_fdt_add_mem_rsv_regions(&images->lmb,
> images->ft_addr);
> 827                     ret = boot_relocate_fdt(&images->lmb, &images->ft_addr,
> 828                                             &images->ft_len);
> 829             }
> 830     #endif
> 831
>
> ** CID 467407:  Uninitialized variables  (UNINIT)
> /drivers/scsi/scsi.c: 612 in do_scsi_scan_one()
>
>
> ________________________________________________________________________________________________________
> *** CID 467407:  Uninitialized variables  (UNINIT)
> /drivers/scsi/scsi.c: 612 in do_scsi_scan_one()
> 606
> 607             bdesc = dev_get_uclass_plat(bdev);
> 608             bdesc->target = id;
> 609             bdesc->lun = lun;
> 610             bdesc->removable = bd.removable;
> 611             bdesc->type = bd.type;
> >>>     CID 467407:  Uninitialized variables  (UNINIT)
> >>>     Using uninitialized value "bd.bb".
> 612             bdesc->bb = bd.bb;
> 613             memcpy(&bdesc->vendor, &bd.vendor, sizeof(bd.vendor));
> 614             memcpy(&bdesc->product, &bd.product, sizeof(bd.product));
> 615             memcpy(&bdesc->revision, &bd.revision,  sizeof(bd.revision));
> 616             if (IS_ENABLED(CONFIG_SYS_BIG_ENDIAN)) {
> 617                     ata_swap_buf_le16((u16 *)&bdesc->vendor,
> sizeof(bd.vendor) / 2);
>
> ** CID 467406:  Memory - corruptions  (OVERRUN)
>
>
> ________________________________________________________________________________________________________
> *** CID 467406:  Memory - corruptions  (OVERRUN)
> /lib/efi_loader/efi_tcg2.c: 885 in efi_append_scrtm_version()
> 879      * @Return:     status code
> 880      */
> 881     static efi_status_t efi_append_scrtm_version(struct udevice *dev)
> 882     {
> 883             efi_status_t ret;
> 884
> >>>     CID 467406:  Memory - corruptions  (OVERRUN)
> >>>     Overrunning array "version_string" of 50 bytes by passing it to a function which accesses it at byte offset 63.
> 885             ret = measure_event(dev, 0, EV_S_CRTM_VERSION,
> 886                                 strlen(version_string) + 1, (u8
> *)version_string);
> 887
> 888             return ret;
> 889     }
> 890
>
> ** CID 467405:  Memory - illegal accesses  (OVERRUN)
> /drivers/firmware/scmi/sandbox-scmi_agent.c: 662 in sandbox_scmi_pwd_state_get()
>
>
> ________________________________________________________________________________________________________
> *** CID 467405:  Memory - illegal accesses  (OVERRUN)
> /drivers/firmware/scmi/sandbox-scmi_agent.c: 662 in sandbox_scmi_pwd_state_get()
> 656             if (domain_id > ARRAY_SIZE(scmi_pwdom)) {
> 657                     out->status = SCMI_NOT_FOUND;
> 658
> 659                     return 0;
> 660             }
> 661
> >>>     CID 467405:  Memory - illegal accesses  (OVERRUN)
> >>>     Overrunning array "scmi_pwdom" of 3 8-byte elements at element index 3 (byte offset 31) using index "domain_id" (which evaluates to 3).
> 662             out->pstate = scmi_pwdom[domain_id].pstate;
> 663             out->status = SCMI_SUCCESS;
> 664
> 665             return 0;
> 666     }
> 667
>
> ** CID 467404:  Control flow issues  (DEADCODE)
> /test/cmd/mbr.c: 217 in build_mbr_parts()
>
>
> ________________________________________________________________________________________________________
> *** CID 467404:  Control flow issues  (DEADCODE)
> /test/cmd/mbr.c: 217 in build_mbr_parts()
> 211                                             return 1;
> 212                                     strcat(cur_buf, mbr_parts_p5);
> 213                                     bytes_remaining -= cur_str_size;
> 214
> 215                             }
> 216                             else if (num_parts > 5)
> >>>     CID 467404:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach this statement: "return 1U;".
> 217                                     return 1;
> 218                             }
> 219                     }
> 220             }
> 221
> 222             cur_str_size = sizeof(mbr_parts_tail);
>
> ** CID 467403:  Error handling issues  (CHECKED_RETURN)
> /test/dm/ofnode.c: 869 in dm_test_ofnode_livetree_writing()
>
>
> ________________________________________________________________________________________________________
> *** CID 467403:  Error handling issues  (CHECKED_RETURN)
> /test/dm/ofnode.c: 869 in dm_test_ofnode_livetree_writing()
> 863             node = ofnode_path("/usb@2");
> 864
> 865             ut_assert(!ofnode_is_enabled(node));
> 866             ut_assertok(ofnode_set_enabled(node, true));
> 867             ut_asserteq(true, ofnode_is_enabled(node));
> 868
> >>>     CID 467403:  Error handling issues  (CHECKED_RETURN)
> >>>     Calling "device_bind_driver_to_node" without checking return value (as is done elsewhere 12 out of 15 times).
> 869             device_bind_driver_to_node(dm_root(), "usb_sandbox",
> "usb@2", node,
> 870                                        &dev);
> 871             ut_assertok(uclass_find_device_by_seq(UCLASS_USB, 2, &dev));
> 872
> 873             /* Test string property setting */
> 874             ut_assert(device_is_compatible(dev, "sandbox,usb"));
>
> ** CID 467402:    (CHECKED_RETURN)
> /drivers/block/rkmtd.c: 737 in rkmtd_init_plat()
> /drivers/block/rkmtd.c: 755 in rkmtd_init_plat()
>
>
> ________________________________________________________________________________________________________
> *** CID 467402:    (CHECKED_RETURN)
> /drivers/block/rkmtd.c: 737 in rkmtd_init_plat()
> 731
> 732             debug("starting_lba           : %llu\n",
> le64_to_cpu(plat->gpt_e->starting_lba));
> 733             debug("ending_lba             : %llu\n",
> le64_to_cpu(plat->gpt_e->ending_lba));
> 734
> 735             memcpy(plat->gpt_e->partition_type_guid.b,
> &partition_basic_data_guid, 16);
> 736
> >>>     CID 467402:    (CHECKED_RETURN)
> >>>     Calling "uuid_str_to_bin" without checking return value (as is done elsewhere 9 out of 11 times).
> 737             uuid_str_to_bin(plat->uuid_part_str,
> plat->gpt_e->unique_partition_guid.b,
> 738                             UUID_STR_FORMAT_GUID);
> 739
> 740             efiname_len = sizeof(plat->gpt_e->partition_name) /
> sizeof(efi_char16_t);
> 741             dosname_len = sizeof(name);
> 742
> /drivers/block/rkmtd.c: 755 in rkmtd_init_plat()
> 749             plat->gpt_h->header_size = cpu_to_le32(sizeof(gpt_header));
> 750             plat->gpt_h->first_usable_lba = cpu_to_le64(64);
> 751             plat->gpt_h->last_usable_lba = cpu_to_le64(LBA - 34);
> 752             plat->gpt_h->num_partition_entries = cpu_to_le32(1);
> 753             plat->gpt_h->sizeof_partition_entry =
> cpu_to_le32(sizeof(gpt_entry));
> 754
> >>>     CID 467402:    (CHECKED_RETURN)
> >>>     Calling "uuid_str_to_bin" without checking return value (as is done elsewhere 9 out of 11 times).
> 755             uuid_str_to_bin(plat->uuid_disk_str, plat->gpt_h->disk_guid.b,
> 756                             UUID_STR_FORMAT_GUID);
> 757
> 758             plat->gpt_h->partition_entry_array_crc32 = 0;
> 759             calc_crc32 = efi_crc32((const unsigned char *)plat->gpt_e,
> 760
> le32_to_cpu(plat->gpt_h->num_partition_entries) *
>
> ** CID 467401:  Memory - corruptions  (OVERRUN)
> /drivers/firmware/scmi/sandbox-scmi_agent.c: 629 in sandbox_scmi_pwd_state_set()
>
>
> ________________________________________________________________________________________________________
> *** CID 467401:  Memory - corruptions  (OVERRUN)
> /drivers/firmware/scmi/sandbox-scmi_agent.c: 629 in sandbox_scmi_pwd_state_set()
> 623                 (in->pstate != SCMI_PWD_PSTATE_TYPE_LOST && in->pstate)) {
> 624                     *status = SCMI_INVALID_PARAMETERS;
> 625
> 626                     return 0;
> 627             }
> 628
> >>>     CID 467401:  Memory - corruptions  (OVERRUN)
> >>>     Overrunning array "scmi_pwdom" of 3 8-byte elements at element index 3 (byte offset 31) using index "in->domain_id" (which evaluates to 3).
> 629             scmi_pwdom[in->domain_id].pstate = in->pstate;
> 630             *status = SCMI_SUCCESS;
> 631
> 632             return 0;
> 633     }
> 634
>
> ** CID 467400:  Memory - illegal accesses  (OVERRUN)
> /lib/efi_loader/efi_tcg2.c: 998 in tcg2_measure_variable()
>
>
> ________________________________________________________________________________________________________
> *** CID 467400:  Memory - illegal accesses  (OVERRUN)
> /lib/efi_loader/efi_tcg2.c: 998 in tcg2_measure_variable()
> 992             guidcpy(&event->variable_name, guid);
> 993             event->unicode_name_length = u16_strlen(var_name);
> 994             event->variable_data_length = data_size;
> 995             memcpy(event->unicode_name, var_name,
> 996                    (event->unicode_name_length * sizeof(u16)));
> 997             if (data) {
> >>>     CID 467400:  Memory - illegal accesses  (OVERRUN)
> >>>     Overrunning array of 2 bytes at byte offset 2 by dereferencing pointer "(u16 *)event->unicode_name + event->unicode_name_length". [Note: The source code implementation of the function has been overridden by a builtin model.]
> 998                     memcpy((u16 *)event->unicode_name +
> event->unicode_name_length,
> 999                            data, data_size);
> 1000            }
> 1001            ret = measure_event(dev, pcr_index, event_type, event_size,
> 1002                                (u8 *)event);
> 1003            free(event);
>
> ** CID 467399:  Code maintainability issues  (UNUSED_VALUE)
> /lib/efi_loader/efi_tcg2.c: 948 in efi_init_event_log()
>
>
> ________________________________________________________________________________________________________
> *** CID 467399:  Code maintainability issues  (UNUSED_VALUE)
> /lib/efi_loader/efi_tcg2.c: 948 in efi_init_event_log()
> 942
> 943             /*
> 944              * Add SCRTM version to the log if previous firmmware
> 945              * doesn't pass an eventlog.
> 946              */
> 947             if (!elog.found)
> >>>     CID 467399:  Code maintainability issues  (UNUSED_VALUE)
> >>>     Assigning value from "efi_append_scrtm_version(dev)" to "ret" here, but that stored value is overwritten before it can be used.
> 948                     ret = efi_append_scrtm_version(dev);
> 949
> 950             ret = create_final_event();
> 951             if (ret != EFI_SUCCESS)
> 952                     goto free_pool;
> 953
>
>
> --
> Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
@ 2023-11-06 20:27 Tom Rini
  2023-11-07 11:18 ` Ilias Apalodimas
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Tom Rini @ 2023-11-06 20:27 UTC (permalink / raw)
  To: u-boot
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass, Eddie James,
	Kever Yang, Johan Jonker, AKASHI Takahiro, Etienne Carriere,
	Alexander Gendin

[-- Attachment #1: Type: text/plain, Size: 15089 bytes --]

Hey all,

Here's the latest report. I _think_ I passed the right options to
get_maintainer.pl such that it would only look far enough back in git to
find the likely authors (along with listed maintainers of the files).

---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Nov 6, 2023 at 2:58 PM
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.

13 new defect(s) introduced to Das U-Boot found with Coverity Scan.
5 defect(s), reported by Coverity Scan earlier, were marked fixed in
the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 13 of 13 defect(s)


** CID 467411:  Memory - corruptions  (OVERRUN)


________________________________________________________________________________________________________
*** CID 467411:  Memory - corruptions  (OVERRUN)
/lib/efi_loader/efi_tcg2.c: 1395 in efi_tcg2_measure_efi_app_invocation()
1389
1390            ret = tcg2_measure_gpt_data(dev, handle);
1391            if (ret != EFI_SUCCESS)
1392                    goto out;
1393
1394            for (pcr_index = 0; pcr_index <= 7; pcr_index++) {
>>>     CID 467411:  Memory - corruptions  (OVERRUN)
>>>     Overrunning buffer pointed to by "(u8 *)&event" of 4 bytes by passing it to a function which accesses it at byte offset 63.
1395                    ret = measure_event(dev, pcr_index, EV_SEPARATOR,
1396                                        sizeof(event), (u8 *)&event);
1397                    if (ret != EFI_SUCCESS)
1398                            goto out;
1399            }
1400

** CID 467410:    (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 467410:    (TAINTED_SCALAR)
/lib/efi_loader/efi_tcg2.c: 1385 in efi_tcg2_measure_efi_app_invocation()
1379                                (u8 *)EFI_CALLING_EFI_APPLICATION);
1380            if (ret != EFI_SUCCESS)
1381                    goto out;
1382
1383            entry = (struct smbios_entry *)find_smbios_table();
1384            if (entry) {
>>>     CID 467410:    (TAINTED_SCALAR)
>>>     Passing tainted expression "entry->struct_table_length" to "tcg2_measure_smbios", which uses it as an offset.
1385                    ret = tcg2_measure_smbios(dev, entry);
1386                    if (ret != EFI_SUCCESS)
1387                            goto out;
1388            }
1389
1390            ret = tcg2_measure_gpt_data(dev, handle);
/lib/efi_loader/efi_tcg2.c: 1385 in efi_tcg2_measure_efi_app_invocation()
1379                                (u8 *)EFI_CALLING_EFI_APPLICATION);
1380            if (ret != EFI_SUCCESS)
1381                    goto out;
1382
1383            entry = (struct smbios_entry *)find_smbios_table();
1384            if (entry) {
>>>     CID 467410:    (TAINTED_SCALAR)
>>>     Passing tainted expression "entry->struct_count" to "tcg2_measure_smbios", which uses it as a loop boundary.
1385                    ret = tcg2_measure_smbios(dev, entry);
1386                    if (ret != EFI_SUCCESS)
1387                            goto out;
1388            }
1389
1390            ret = tcg2_measure_gpt_data(dev, handle);

** CID 467409:  Uninitialized variables  (UNINIT)


________________________________________________________________________________________________________
*** CID 467409:  Uninitialized variables  (UNINIT)
/test/boot/measurement.c: 48 in measure()
42      for (i = 0; i < size; ++i) {
43              kernel[i] = 0xf0 | (i & 0xf);
44              initrd[i] = (i & 0xf0) | 0xf;
45              images.ft_addr[i] = i & 0xff;
46      }
47
>>>     CID 467409:  Uninitialized variables  (UNINIT)
>>>     Using uninitialized value "images.os.os" when calling "bootm_measure".
48      ut_assertok(bootm_measure(&images));
49
50      free(images.ft_addr);
51      free(initrd);
52      free(kernel);
53

** CID 467408:  Insecure data handling  (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 467408:  Insecure data handling  (TAINTED_SCALAR)
/boot/bootm.c: 826 in do_bootm_states()
820                             env_set_hex("initrd_end", images->initrd_end);
821                     }
822             }
823     #endif
824     #if CONFIG_IS_ENABLED(OF_LIBFDT) && defined(CONFIG_LMB)
825             if (!ret && (states & BOOTM_STATE_FDT)) {
>>>     CID 467408:  Insecure data handling  (TAINTED_SCALAR)
>>>     Passing tainted expression "*images->ft_addr" to "boot_fdt_add_mem_rsv_regions", which uses it as an offset.
826                     boot_fdt_add_mem_rsv_regions(&images->lmb,
images->ft_addr);
827                     ret = boot_relocate_fdt(&images->lmb, &images->ft_addr,
828                                             &images->ft_len);
829             }
830     #endif
831

** CID 467407:  Uninitialized variables  (UNINIT)
/drivers/scsi/scsi.c: 612 in do_scsi_scan_one()


________________________________________________________________________________________________________
*** CID 467407:  Uninitialized variables  (UNINIT)
/drivers/scsi/scsi.c: 612 in do_scsi_scan_one()
606
607             bdesc = dev_get_uclass_plat(bdev);
608             bdesc->target = id;
609             bdesc->lun = lun;
610             bdesc->removable = bd.removable;
611             bdesc->type = bd.type;
>>>     CID 467407:  Uninitialized variables  (UNINIT)
>>>     Using uninitialized value "bd.bb".
612             bdesc->bb = bd.bb;
613             memcpy(&bdesc->vendor, &bd.vendor, sizeof(bd.vendor));
614             memcpy(&bdesc->product, &bd.product, sizeof(bd.product));
615             memcpy(&bdesc->revision, &bd.revision,  sizeof(bd.revision));
616             if (IS_ENABLED(CONFIG_SYS_BIG_ENDIAN)) {
617                     ata_swap_buf_le16((u16 *)&bdesc->vendor,
sizeof(bd.vendor) / 2);

** CID 467406:  Memory - corruptions  (OVERRUN)


________________________________________________________________________________________________________
*** CID 467406:  Memory - corruptions  (OVERRUN)
/lib/efi_loader/efi_tcg2.c: 885 in efi_append_scrtm_version()
879      * @Return:     status code
880      */
881     static efi_status_t efi_append_scrtm_version(struct udevice *dev)
882     {
883             efi_status_t ret;
884
>>>     CID 467406:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "version_string" of 50 bytes by passing it to a function which accesses it at byte offset 63.
885             ret = measure_event(dev, 0, EV_S_CRTM_VERSION,
886                                 strlen(version_string) + 1, (u8
*)version_string);
887
888             return ret;
889     }
890

** CID 467405:  Memory - illegal accesses  (OVERRUN)
/drivers/firmware/scmi/sandbox-scmi_agent.c: 662 in sandbox_scmi_pwd_state_get()


________________________________________________________________________________________________________
*** CID 467405:  Memory - illegal accesses  (OVERRUN)
/drivers/firmware/scmi/sandbox-scmi_agent.c: 662 in sandbox_scmi_pwd_state_get()
656             if (domain_id > ARRAY_SIZE(scmi_pwdom)) {
657                     out->status = SCMI_NOT_FOUND;
658
659                     return 0;
660             }
661
>>>     CID 467405:  Memory - illegal accesses  (OVERRUN)
>>>     Overrunning array "scmi_pwdom" of 3 8-byte elements at element index 3 (byte offset 31) using index "domain_id" (which evaluates to 3).
662             out->pstate = scmi_pwdom[domain_id].pstate;
663             out->status = SCMI_SUCCESS;
664
665             return 0;
666     }
667

** CID 467404:  Control flow issues  (DEADCODE)
/test/cmd/mbr.c: 217 in build_mbr_parts()


________________________________________________________________________________________________________
*** CID 467404:  Control flow issues  (DEADCODE)
/test/cmd/mbr.c: 217 in build_mbr_parts()
211                                             return 1;
212                                     strcat(cur_buf, mbr_parts_p5);
213                                     bytes_remaining -= cur_str_size;
214
215                             }
216                             else if (num_parts > 5)
>>>     CID 467404:  Control flow issues  (DEADCODE)
>>>     Execution cannot reach this statement: "return 1U;".
217                                     return 1;
218                             }
219                     }
220             }
221
222             cur_str_size = sizeof(mbr_parts_tail);

** CID 467403:  Error handling issues  (CHECKED_RETURN)
/test/dm/ofnode.c: 869 in dm_test_ofnode_livetree_writing()


________________________________________________________________________________________________________
*** CID 467403:  Error handling issues  (CHECKED_RETURN)
/test/dm/ofnode.c: 869 in dm_test_ofnode_livetree_writing()
863             node = ofnode_path("/usb@2");
864
865             ut_assert(!ofnode_is_enabled(node));
866             ut_assertok(ofnode_set_enabled(node, true));
867             ut_asserteq(true, ofnode_is_enabled(node));
868
>>>     CID 467403:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "device_bind_driver_to_node" without checking return value (as is done elsewhere 12 out of 15 times).
869             device_bind_driver_to_node(dm_root(), "usb_sandbox",
"usb@2", node,
870                                        &dev);
871             ut_assertok(uclass_find_device_by_seq(UCLASS_USB, 2, &dev));
872
873             /* Test string property setting */
874             ut_assert(device_is_compatible(dev, "sandbox,usb"));

** CID 467402:    (CHECKED_RETURN)
/drivers/block/rkmtd.c: 737 in rkmtd_init_plat()
/drivers/block/rkmtd.c: 755 in rkmtd_init_plat()


________________________________________________________________________________________________________
*** CID 467402:    (CHECKED_RETURN)
/drivers/block/rkmtd.c: 737 in rkmtd_init_plat()
731
732             debug("starting_lba           : %llu\n",
le64_to_cpu(plat->gpt_e->starting_lba));
733             debug("ending_lba             : %llu\n",
le64_to_cpu(plat->gpt_e->ending_lba));
734
735             memcpy(plat->gpt_e->partition_type_guid.b,
&partition_basic_data_guid, 16);
736
>>>     CID 467402:    (CHECKED_RETURN)
>>>     Calling "uuid_str_to_bin" without checking return value (as is done elsewhere 9 out of 11 times).
737             uuid_str_to_bin(plat->uuid_part_str,
plat->gpt_e->unique_partition_guid.b,
738                             UUID_STR_FORMAT_GUID);
739
740             efiname_len = sizeof(plat->gpt_e->partition_name) /
sizeof(efi_char16_t);
741             dosname_len = sizeof(name);
742
/drivers/block/rkmtd.c: 755 in rkmtd_init_plat()
749             plat->gpt_h->header_size = cpu_to_le32(sizeof(gpt_header));
750             plat->gpt_h->first_usable_lba = cpu_to_le64(64);
751             plat->gpt_h->last_usable_lba = cpu_to_le64(LBA - 34);
752             plat->gpt_h->num_partition_entries = cpu_to_le32(1);
753             plat->gpt_h->sizeof_partition_entry =
cpu_to_le32(sizeof(gpt_entry));
754
>>>     CID 467402:    (CHECKED_RETURN)
>>>     Calling "uuid_str_to_bin" without checking return value (as is done elsewhere 9 out of 11 times).
755             uuid_str_to_bin(plat->uuid_disk_str, plat->gpt_h->disk_guid.b,
756                             UUID_STR_FORMAT_GUID);
757
758             plat->gpt_h->partition_entry_array_crc32 = 0;
759             calc_crc32 = efi_crc32((const unsigned char *)plat->gpt_e,
760
le32_to_cpu(plat->gpt_h->num_partition_entries) *

** CID 467401:  Memory - corruptions  (OVERRUN)
/drivers/firmware/scmi/sandbox-scmi_agent.c: 629 in sandbox_scmi_pwd_state_set()


________________________________________________________________________________________________________
*** CID 467401:  Memory - corruptions  (OVERRUN)
/drivers/firmware/scmi/sandbox-scmi_agent.c: 629 in sandbox_scmi_pwd_state_set()
623                 (in->pstate != SCMI_PWD_PSTATE_TYPE_LOST && in->pstate)) {
624                     *status = SCMI_INVALID_PARAMETERS;
625
626                     return 0;
627             }
628
>>>     CID 467401:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "scmi_pwdom" of 3 8-byte elements at element index 3 (byte offset 31) using index "in->domain_id" (which evaluates to 3).
629             scmi_pwdom[in->domain_id].pstate = in->pstate;
630             *status = SCMI_SUCCESS;
631
632             return 0;
633     }
634

** CID 467400:  Memory - illegal accesses  (OVERRUN)
/lib/efi_loader/efi_tcg2.c: 998 in tcg2_measure_variable()


________________________________________________________________________________________________________
*** CID 467400:  Memory - illegal accesses  (OVERRUN)
/lib/efi_loader/efi_tcg2.c: 998 in tcg2_measure_variable()
992             guidcpy(&event->variable_name, guid);
993             event->unicode_name_length = u16_strlen(var_name);
994             event->variable_data_length = data_size;
995             memcpy(event->unicode_name, var_name,
996                    (event->unicode_name_length * sizeof(u16)));
997             if (data) {
>>>     CID 467400:  Memory - illegal accesses  (OVERRUN)
>>>     Overrunning array of 2 bytes at byte offset 2 by dereferencing pointer "(u16 *)event->unicode_name + event->unicode_name_length". [Note: The source code implementation of the function has been overridden by a builtin model.]
998                     memcpy((u16 *)event->unicode_name +
event->unicode_name_length,
999                            data, data_size);
1000            }
1001            ret = measure_event(dev, pcr_index, event_type, event_size,
1002                                (u8 *)event);
1003            free(event);

** CID 467399:  Code maintainability issues  (UNUSED_VALUE)
/lib/efi_loader/efi_tcg2.c: 948 in efi_init_event_log()


________________________________________________________________________________________________________
*** CID 467399:  Code maintainability issues  (UNUSED_VALUE)
/lib/efi_loader/efi_tcg2.c: 948 in efi_init_event_log()
942
943             /*
944              * Add SCRTM version to the log if previous firmmware
945              * doesn't pass an eventlog.
946              */
947             if (!elog.found)
>>>     CID 467399:  Code maintainability issues  (UNUSED_VALUE)
>>>     Assigning value from "efi_append_scrtm_version(dev)" to "ret" here, but that stored value is overwritten before it can be used.
948                     ret = efi_append_scrtm_version(dev);
949
950             ret = create_final_event();
951             if (ret != EFI_SUCCESS)
952                     goto free_pool;
953


-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
  2023-10-24 15:05 ` Sughosh Ganu
@ 2023-10-24 18:05   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-10-24 18:05 UTC (permalink / raw)
  To: Sughosh Ganu; +Cc: u-boot

[-- Attachment #1: Type: text/plain, Size: 3920 bytes --]

On Tue, Oct 24, 2023 at 08:35:58PM +0530, Sughosh Ganu wrote:
> hi Tom,
> 
> On Tue, 24 Oct 2023 at 06:48, Tom Rini <trini@konsulko.com> wrote:
> >
> > Here's the latest report
> >
> > ---------- Forwarded message ---------
> > From: <scan-admin@coverity.com>
> > Date: Mon, Oct 23, 2023 at 4:40 PM
> > 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.
> >
> > 16 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> > 6 defect(s), reported by Coverity Scan earlier, were marked fixed in
> > the recent build analyzed by Coverity Scan.
> >
> > New defect(s) Reported-by: Coverity Scan
> > Showing 16 of 16 defect(s)
> >
> 
> <snip>
> 
> >
> > ** CID 467053:    (RESOURCE_LEAK)
> > /tools/mkeficapsule.c: 859 in dump_capsule_contents()
> > /tools/mkeficapsule.c: 859 in dump_capsule_contents()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 467053:    (RESOURCE_LEAK)
> > /tools/mkeficapsule.c: 859 in dump_capsule_contents()
> > 853                     empty_capsule_dump(ptr);
> > 854             } else {
> > 855                     fprintf(stderr, "Unable to decode the capsule
> > file: %s\n",
> > 856                             capsule_file);
> > 857                     exit(EXIT_FAILURE);
> > 858             }
> > >>>     CID 467053:    (RESOURCE_LEAK)
> > >>>     Variable "ptr" going out of scope leaks the storage it points to.
> > 859     }
> > 860
> > 861     /**
> > 862      * main - main entry function of mkeficapsule
> > 863      * @argc:       Number of arguments
> > 864      * @argv:       Array of pointers to arguments
> > /tools/mkeficapsule.c: 859 in dump_capsule_contents()
> > 853                     empty_capsule_dump(ptr);
> > 854             } else {
> > 855                     fprintf(stderr, "Unable to decode the capsule
> > file: %s\n",
> > 856                             capsule_file);
> > 857                     exit(EXIT_FAILURE);
> > 858             }
> > >>>     CID 467053:    (RESOURCE_LEAK)
> > >>>     Variable "ptr" going out of scope leaks the storage it points to.
> > 859     }
> > 860
> > 861     /**
> > 862      * main - main entry function of mkeficapsule
> > 863      * @argc:       Number of arguments
> > 864      * @argv:       Array of pointers to arguments
> >
> 
> <snip>
> 
> > ** CID 467045:  Resource leaks  (RESOURCE_LEAK)
> > /tools/mkeficapsule.c: 859 in dump_capsule_contents()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 467045:  Resource leaks  (RESOURCE_LEAK)
> > /tools/mkeficapsule.c: 859 in dump_capsule_contents()
> > 853                     empty_capsule_dump(ptr);
> > 854             } else {
> > 855                     fprintf(stderr, "Unable to decode the capsule
> > file: %s\n",
> > 856                             capsule_file);
> > 857                     exit(EXIT_FAILURE);
> > 858             }
> > >>>     CID 467045:  Resource leaks  (RESOURCE_LEAK)
> > >>>     Handle variable "fd" going out of scope leaks the handle.
> > 859     }
> > 860
> > 861     /**
> > 862      * main - main entry function of mkeficapsule
> > 863      * @argc:       Number of arguments
> > 864      * @argv:       Array of pointers to arguments
> >
> >
> 
> Both the pointer and file descriptor are not being freed since the
> process exits once the dump_capaule_contents() function returns. These
> can be marked as false positives. Thanks.

I would say that's "intentional" rather than false positive (and perhaps
a bad example) but indeed not a fatal problem. Thanks for checking.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
  2023-10-24  1:18 Tom Rini
@ 2023-10-24 15:05 ` Sughosh Ganu
  2023-10-24 18:05   ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Sughosh Ganu @ 2023-10-24 15:05 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot

hi Tom,

On Tue, 24 Oct 2023 at 06:48, Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest report
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Oct 23, 2023 at 4:40 PM
> 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.
>
> 16 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 6 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 16 of 16 defect(s)
>

<snip>

>
> ** CID 467053:    (RESOURCE_LEAK)
> /tools/mkeficapsule.c: 859 in dump_capsule_contents()
> /tools/mkeficapsule.c: 859 in dump_capsule_contents()
>
>
> ________________________________________________________________________________________________________
> *** CID 467053:    (RESOURCE_LEAK)
> /tools/mkeficapsule.c: 859 in dump_capsule_contents()
> 853                     empty_capsule_dump(ptr);
> 854             } else {
> 855                     fprintf(stderr, "Unable to decode the capsule
> file: %s\n",
> 856                             capsule_file);
> 857                     exit(EXIT_FAILURE);
> 858             }
> >>>     CID 467053:    (RESOURCE_LEAK)
> >>>     Variable "ptr" going out of scope leaks the storage it points to.
> 859     }
> 860
> 861     /**
> 862      * main - main entry function of mkeficapsule
> 863      * @argc:       Number of arguments
> 864      * @argv:       Array of pointers to arguments
> /tools/mkeficapsule.c: 859 in dump_capsule_contents()
> 853                     empty_capsule_dump(ptr);
> 854             } else {
> 855                     fprintf(stderr, "Unable to decode the capsule
> file: %s\n",
> 856                             capsule_file);
> 857                     exit(EXIT_FAILURE);
> 858             }
> >>>     CID 467053:    (RESOURCE_LEAK)
> >>>     Variable "ptr" going out of scope leaks the storage it points to.
> 859     }
> 860
> 861     /**
> 862      * main - main entry function of mkeficapsule
> 863      * @argc:       Number of arguments
> 864      * @argv:       Array of pointers to arguments
>

<snip>

> ** CID 467045:  Resource leaks  (RESOURCE_LEAK)
> /tools/mkeficapsule.c: 859 in dump_capsule_contents()
>
>
> ________________________________________________________________________________________________________
> *** CID 467045:  Resource leaks  (RESOURCE_LEAK)
> /tools/mkeficapsule.c: 859 in dump_capsule_contents()
> 853                     empty_capsule_dump(ptr);
> 854             } else {
> 855                     fprintf(stderr, "Unable to decode the capsule
> file: %s\n",
> 856                             capsule_file);
> 857                     exit(EXIT_FAILURE);
> 858             }
> >>>     CID 467045:  Resource leaks  (RESOURCE_LEAK)
> >>>     Handle variable "fd" going out of scope leaks the handle.
> 859     }
> 860
> 861     /**
> 862      * main - main entry function of mkeficapsule
> 863      * @argc:       Number of arguments
> 864      * @argv:       Array of pointers to arguments
>
>

Both the pointer and file descriptor are not being freed since the
process exits once the dump_capaule_contents() function returns. These
can be marked as false positives. Thanks.

-sughosh

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
@ 2023-10-24  1:18 Tom Rini
  2023-10-24 15:05 ` Sughosh Ganu
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2023-10-24  1:18 UTC (permalink / raw)
  To: u-boot

[-- Attachment #1: Type: text/plain, Size: 18122 bytes --]

Here's the latest report

---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Oct 23, 2023 at 4:40 PM
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.

16 new defect(s) introduced to Das U-Boot found with Coverity Scan.
6 defect(s), reported by Coverity Scan earlier, were marked fixed in
the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 16 of 16 defect(s)


** CID 467060:    (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 467060:    (TAINTED_SCALAR)
/boot/bootmeth_cros.c: 184 in scan_part()
178             if (ret != num_blks) {
179                     free(hdr);
180                     return log_msg_ret("inf", -EIO);
181             }
182
183             if (memcmp(VB2_KEYBLOCK_MAGIC, hdr->magic,
VB2_KEYBLOCK_MAGIC_SIZE)) {
>>>     CID 467060:    (TAINTED_SCALAR)
>>>     Passing tainted expression "*hdr" to "dlfree", which uses it as an offset.
184                     free(hdr);
185                     log_debug("no magic\n");
186                     return -ENOENT;
187             }
188
189             *hdrp = hdr;
/boot/bootmeth_cros.c: 179 in scan_part()
173                       blk->name, (ulong)info->start, num_blks);
174             hdr = memalign(SZ_1K, PROBE_SIZE);
175             if (!hdr)
176                     return log_msg_ret("hdr", -ENOMEM);
177             ret = blk_read(blk, info->start, num_blks, hdr);
178             if (ret != num_blks) {
>>>     CID 467060:    (TAINTED_SCALAR)
>>>     Passing tainted expression "*hdr" to "dlfree", which uses it as an offset.
179                     free(hdr);
180                     return log_msg_ret("inf", -EIO);
181             }
182
183             if (memcmp(VB2_KEYBLOCK_MAGIC, hdr->magic,
VB2_KEYBLOCK_MAGIC_SIZE)) {
184                     free(hdr);

** CID 467059:  Integer handling issues  (INCOMPATIBLE_CAST)


________________________________________________________________________________________________________
*** CID 467059:  Integer handling issues  (INCOMPATIBLE_CAST)
/drivers/mtd/nvmxip/nvmxip_qspi.c: 47 in nvmxip_qspi_of_to_plat()
41      ret = dev_read_u32(dev, "lba_shift", &plat->lba_shift);
42      if (ret) {
43              log_err("[%s]: can not get lba_shift from device
tree\n", dev->name);
44              return -EINVAL;
45      }
46
>>>     CID 467059:  Integer handling issues  (INCOMPATIBLE_CAST)
>>>     Pointer "&plat->lba" points to an object whose effective type is "unsigned long" (64 bits, unsigned) but is dereferenced as a narrower "unsigned int" (32 bits, unsigned). This may lead to unexpected results depending on machine endianness.
47      ret = dev_read_u32(dev, "lba", (u32 *)&plat->lba);
48      if (ret) {
49              log_err("[%s]: can not get lba from device tree\n", dev->name);
50              return -EINVAL;
51      }
52

** CID 467058:  Insecure data handling  (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 467058:  Insecure data handling  (TAINTED_SCALAR)
/drivers/core/ofnode.c: 1629 in ofnode_write_u32()
1623            log_debug("%s = %x", propname, value);
1624            val = malloc(sizeof(*val));
1625            if (!val)
1626                    return -ENOMEM;
1627            *val = cpu_to_fdt32(value);
1628
>>>     CID 467058:  Insecure data handling  (TAINTED_SCALAR)
>>>     Passing tainted expression "*val" to "ofnode_write_prop", which uses it as an offset.
1629            return ofnode_write_prop(node, propname, val,
sizeof(value), true);
1630     }
1631
1632     int ofnode_write_u64(ofnode node, const char *propname, u64 value)
1633     {
1634            fdt64_t *val;

** CID 467057:  Uninitialized variables  (UNINIT)


________________________________________________________________________________________________________
*** CID 467057:  Uninitialized variables  (UNINIT)
/boot/bootflow.c: 320 in iter_incr()
314                              * Probe the bootdev. This does not
probe any attached
315                              * block device, since they are siblings
316                              */
317                             ret = device_probe(dev);
318                             log_debug("probe %s %d\n", dev->name, ret);
319                             if (!log_msg_ret("probe", ret))
>>>     CID 467057:  Uninitialized variables  (UNINIT)
>>>     Using uninitialized value "method_flags" when calling "bootflow_iter_set_dev".
320                                     bootflow_iter_set_dev(iter,
dev, method_flags);
321                     }
322             }
323
324             /* if there are no more bootdevs, give up */
325             if (ret)

** CID 467056:  Control flow issues  (NO_EFFECT)
/common/cli_readline.c: 321 in cread_line_process_ch()


________________________________________________________________________________________________________
*** CID 467056:  Control flow issues  (NO_EFFECT)
/common/cli_readline.c: 321 in cread_line_process_ch()
315                     break;
316             case CTL_CH('w'):
317                     if (cls->num) {
318                             uint base, wlen;
319
320                             for (base = cls->num - 1;
>>>     CID 467056:  Control flow issues  (NO_EFFECT)
>>>     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "base >= 0U".
321                                  base >= 0 && buf[base] == ' ';)
322                                     base--;
323                             for (; base > 0 && buf[base - 1] != ' ';)
324                                     base--;
325
326                             /* now delete chars from base to cls->num */

** CID 467055:    (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 467055:    (TAINTED_SCALAR)
/boot/bootmeth_cros.c: 372 in cros_read_bootflow()
366                     log_debug("- scan failed: err=%d\n", ret);
367                     return log_msg_ret("scan", ret);
368             }
369
370             priv = malloc(sizeof(struct cros_priv));
371             if (!priv) {
>>>     CID 467055:    (TAINTED_SCALAR)
>>>     Passing tainted expression "*hdr" to "dlfree", which uses it as an offset.
372                     free(hdr);
373                     return log_msg_ret("buf", -ENOMEM);
374             }
375             bflow->bootmeth_priv = priv;
376
377             log_debug("Selected partition %d, header at %lx\n", bflow->part,
/boot/bootmeth_cros.c: 391 in cros_read_bootflow()
385             /* Now read everything we can learn about kernel */
386     #if CONFIG_IS_ENABLED(PARTITION_UUIDS)
387             uuid = info.uuid;
388     #endif
389             ret = cros_read_info(bflow, uuid, preamble);
390             preamble = NULL;
>>>     CID 467055:    (TAINTED_SCALAR)
>>>     Passing tainted expression "*hdr" to "dlfree", which uses it as an offset.
391             free(hdr);
392             if (ret) {
393                     free(priv->info_buf);
394                     free(priv);
395                     return log_msg_ret("inf", ret);
396             }

** CID 467054:  Resource leaks  (RESOURCE_LEAK)
/tools/sfspl.c: 118 in sfspl_image_extract_subimage()


________________________________________________________________________________________________________
*** CID 467054:  Resource leaks  (RESOURCE_LEAK)
/tools/sfspl.c: 118 in sfspl_image_extract_subimage()
112             if (fd == -1) {
113                     perror("Can write file");
114                     return EXIT_FAILURE;
115             }
116             if (write(fd, &buf[hdr_size], file_size) != file_size) {
117                     perror("Cannot write file");
>>>     CID 467054:  Resource leaks  (RESOURCE_LEAK)
>>>     Handle variable "fd" going out of scope leaks the handle.
118                     return EXIT_FAILURE;
119             }
120             close(fd);
121
122             return EXIT_SUCCESS;
123     }

** CID 467053:    (RESOURCE_LEAK)
/tools/mkeficapsule.c: 859 in dump_capsule_contents()
/tools/mkeficapsule.c: 859 in dump_capsule_contents()


________________________________________________________________________________________________________
*** CID 467053:    (RESOURCE_LEAK)
/tools/mkeficapsule.c: 859 in dump_capsule_contents()
853                     empty_capsule_dump(ptr);
854             } else {
855                     fprintf(stderr, "Unable to decode the capsule
file: %s\n",
856                             capsule_file);
857                     exit(EXIT_FAILURE);
858             }
>>>     CID 467053:    (RESOURCE_LEAK)
>>>     Variable "ptr" going out of scope leaks the storage it points to.
859     }
860
861     /**
862      * main - main entry function of mkeficapsule
863      * @argc:       Number of arguments
864      * @argv:       Array of pointers to arguments
/tools/mkeficapsule.c: 859 in dump_capsule_contents()
853                     empty_capsule_dump(ptr);
854             } else {
855                     fprintf(stderr, "Unable to decode the capsule
file: %s\n",
856                             capsule_file);
857                     exit(EXIT_FAILURE);
858             }
>>>     CID 467053:    (RESOURCE_LEAK)
>>>     Variable "ptr" going out of scope leaks the storage it points to.
859     }
860
861     /**
862      * main - main entry function of mkeficapsule
863      * @argc:       Number of arguments
864      * @argv:       Array of pointers to arguments

** CID 467052:  Insecure data handling  (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 467052:  Insecure data handling  (TAINTED_SCALAR)
/drivers/core/ofnode.c: 1644 in ofnode_write_u64()
1638            log_debug("%s = %llx", propname, (unsigned long long)value);
1639            val = malloc(sizeof(*val));
1640            if (!val)
1641                    return -ENOMEM;
1642            *val = cpu_to_fdt64(value);
1643
>>>     CID 467052:  Insecure data handling  (TAINTED_SCALAR)
>>>     Passing tainted expression "*val" to "ofnode_write_prop", which uses it as an offset.
1644            return ofnode_write_prop(node, propname, val,
sizeof(value), true);
1645     }
1646
1647     int ofnode_write_bool(ofnode node, const char *propname, bool value)
1648     {
1649            if (value)

** CID 467051:  API usage errors  (CHAR_IO)
/common/cli_readline.c: 550 in cread_line_simple()


________________________________________________________________________________________________________
*** CID 467051:  API usage errors  (CHAR_IO)
/common/cli_readline.c: 550 in cread_line_simple()
544
545             for (;;) {
546                     if (bootretry_tstc_timeout())
547                             return -2;      /* timed out */
548                     schedule();     /* Trigger watchdog, if needed */
549
>>>     CID 467051:  API usage errors  (CHAR_IO)
>>>     Assigning the return value of "getchar" to char "c" truncates its value.
550                     c = getchar();
551
552                     /*
553                      * Special character handling
554                      */
555                     switch (c) {

** CID 467050:  Control flow issues  (NO_EFFECT)
/drivers/firmware/scmi/sandbox-scmi_agent.c: 220 in
sandbox_scmi_base_message_attrs()


________________________________________________________________________________________________________
*** CID 467050:  Control flow issues  (NO_EFFECT)
/drivers/firmware/scmi/sandbox-scmi_agent.c: 220 in
sandbox_scmi_base_message_attrs()
214                 !msg->out_msg || msg->out_msg_sz < sizeof(*out))
215                     return -EINVAL;
216
217             message_id = *(u32 *)msg->in_msg;
218             out = (struct scmi_protocol_msg_attrs_out *)msg->out_msg;
219
>>>     CID 467050:  Control flow issues  (NO_EFFECT)
>>>     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "message_id >= SCMI_PROTOCOL_VERSION".
220             if (message_id >= SCMI_PROTOCOL_VERSION &&
221                 message_id <= SCMI_BASE_RESET_AGENT_CONFIGURATION &&
222                 message_id != SCMI_BASE_NOTIFY_ERRORS) {
223                     out->attributes = 0;
224                     out->status = SCMI_SUCCESS;
225             } else {

** CID 467049:  Null pointer dereferences  (REVERSE_INULL)
/drivers/core/ofnode.c: 1764 in ofnode_read_bootscript_flash()


________________________________________________________________________________________________________
*** CID 467049:  Null pointer dereferences  (REVERSE_INULL)
/drivers/core/ofnode.c: 1764 in ofnode_read_bootscript_flash()
1758
1759            ret = ofnode_read_u64(uboot, "bootscr-flash-size",
1760                                  bootscr_flash_size);
1761            if (ret)
1762                    return -EINVAL;
1763
>>>     CID 467049:  Null pointer dereferences  (REVERSE_INULL)
>>>     Null-checking "bootscr_flash_size" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
1764            if (!bootscr_flash_size) {
1765                    debug("bootscr-flash-size is zero. Ignoring
properties!\n");
1766                    *bootscr_flash_offset = 0;
1767                    return -EINVAL;
1768            }
1769

** CID 467048:  Null pointer dereferences  (FORWARD_NULL)


________________________________________________________________________________________________________
*** CID 467048:  Null pointer dereferences  (FORWARD_NULL)
/common/cli_readline.c: 602 in cread_line_simple()
596                                     if (IS_ENABLED(CONFIG_AUTO_COMPLETE)) {
597                                             /*
598                                              * if auto-completion
triggered just
599                                              * continue
600                                              */
601                                             *p = '\0';
>>>     CID 467048:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing null pointer "prompt" to "cmd_auto_complete", which dereferences it.
602                                             if (cmd_auto_complete(prompt,
603
console_buffer,
604
&n, &col)) {
605                                                     p = p_buf + n;
 /* reset */
606                                                     continue;
607                                             }

** CID 467047:  Error handling issues  (CHECKED_RETURN)
/boot/image-fit.c: 2477 in boot_get_fdt_fit()


________________________________________________________________________________________________________
*** CID 467047:  Error handling issues  (CHECKED_RETURN)
/boot/image-fit.c: 2477 in boot_get_fdt_fit()
2471                    /* the verbose method prints out messages on error */
2472                    err = fdt_overlay_apply_verbose(base, ovcopy);
2473                    if (err < 0) {
2474                            fdt_noffset = err;
2475                            goto out;
2476                    }
>>>     CID 467047:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "fdt_pack" without checking return value (as is done elsewhere 4 out of 5 times).
2477                    fdt_pack(base);
2478                    len = fdt_totalsize(base);
2479            }
2480     #else
2481            printf("config with overlays but
CONFIG_OF_LIBFDT_OVERLAY not set\n");
2482            fdt_noffset = -EBADF;

** CID 467046:  Error handling issues  (CHECKED_RETURN)
/drivers/phy/phy-uclass.c: 331 in generic_phy_power_on()


________________________________________________________________________________________________________
*** CID 467046:  Error handling issues  (CHECKED_RETURN)
/drivers/phy/phy-uclass.c: 331 in generic_phy_power_on()
325             ops = phy_dev_ops(phy->dev);
326             if (ops->power_on) {
327                     ret = ops->power_on(phy);
328                     if (ret) {
329                             dev_err(phy->dev, "PHY: Failed to
power on %s: %d.\n",
330                                     phy->dev->name, ret);
>>>     CID 467046:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "regulator_set_enable_if_allowed" without checking return value (as is done elsewhere 9 out of 11 times).
331
regulator_set_enable_if_allowed(counts->supply, false);
332                             return ret;
333                     }
334             }
335             counts->power_on_count = 1;
336

** CID 467045:  Resource leaks  (RESOURCE_LEAK)
/tools/mkeficapsule.c: 859 in dump_capsule_contents()


________________________________________________________________________________________________________
*** CID 467045:  Resource leaks  (RESOURCE_LEAK)
/tools/mkeficapsule.c: 859 in dump_capsule_contents()
853                     empty_capsule_dump(ptr);
854             } else {
855                     fprintf(stderr, "Unable to decode the capsule
file: %s\n",
856                             capsule_file);
857                     exit(EXIT_FAILURE);
858             }
>>>     CID 467045:  Resource leaks  (RESOURCE_LEAK)
>>>     Handle variable "fd" going out of scope leaks the handle.
859     }
860
861     /**
862      * main - main entry function of mkeficapsule
863      * @argc:       Number of arguments
864      * @argv:       Array of pointers to arguments



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
@ 2023-07-27 17:38 Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-07-27 17:38 UTC (permalink / raw)
  To: u-boot, Simon Glass, Heinrich Schuchardt

[-- Attachment #1: Type: text/plain, Size: 7498 bytes --]

Here's the latest report.

---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Tue, Jul 25, 2023 at 5:29 PM
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.

5 new defect(s) introduced to Das U-Boot found with Coverity Scan.
30 defect(s), reported by Coverity Scan earlier, were marked fixed in
the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 5 of 5 defect(s)


** CID 463147:  Insecure data handling  (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 463147:  Insecure data handling  (TAINTED_SCALAR)
/common/fdt_support.c: 1115 in fdt_copy_fixed_partitions()
1109                            res = fdt_setprop_string(blob, suboff, "label",
1110
ofnode_read_string(subnode, "label"));
1111                            if (res)
1112                                    return res;
1113
1114                            reg = ofnode_get_property(subnode, "reg", &len);
>>>     CID 463147:  Insecure data handling  (TAINTED_SCALAR)
>>>     Passing tainted expression "len" to "fdt_setprop", which uses it as an offset.
1115                            res = fdt_setprop(blob, suboff, "reg",
reg, len);
1116                            if (res)
1117                                    return res;
1118                    }
1119
1120                    /* go to next fixed-partitions node */

** CID 463146:  Null pointer dereferences  (NULL_RETURNS)


________________________________________________________________________________________________________
*** CID 463146:  Null pointer dereferences  (NULL_RETURNS)
/common/fdt_support.c: 1115 in fdt_copy_fixed_partitions()
1109                            res = fdt_setprop_string(blob, suboff, "label",
1110
ofnode_read_string(subnode, "label"));
1111                            if (res)
1112                                    return res;
1113
1114                            reg = ofnode_get_property(subnode, "reg", &len);
>>>     CID 463146:  Null pointer dereferences  (NULL_RETURNS)
>>>     Dereferencing a pointer that might be "NULL" "reg" when calling "fdt_setprop".
1115                            res = fdt_setprop(blob, suboff, "reg",
reg, len);
1116                            if (res)
1117                                    return res;
1118                    }
1119
1120                    /* go to next fixed-partitions node */

** CID 463145:  Error handling issues  (CHECKED_RETURN)
/lib/efi_loader/efi_firmware.c: 176 in efi_firmware_get_lsv_from_dtb()


________________________________________________________________________________________________________
*** CID 463145:  Error handling issues  (CHECKED_RETURN)
/lib/efi_loader/efi_firmware.c: 176 in efi_firmware_get_lsv_from_dtb()
170             fdt_for_each_subnode(offset, fdt, parent) {
171                     efi_guid_t guid;
172
173                     guid_str = fdt_getprop(fdt, offset,
"image-type-id", &len);
174                     if (!guid_str)
175                             continue;
>>>     CID 463145:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "uuid_str_to_bin" without checking return value (as is done elsewhere 7 out of 8 times).
176                     uuid_str_to_bin(guid_str, guid.b, UUID_STR_FORMAT_GUID);
177
178                     val = fdt_getprop(fdt, offset, "image-index", &len);
179                     if (!val)
180                             continue;
181                     index = fdt32_to_cpu(*val);

** CID 463144:    (CHECKED_RETURN)
/boot/expo.c: 257 in expo_apply_theme()
/boot/expo.c: 258 in expo_apply_theme()
/boot/expo.c: 259 in expo_apply_theme()


________________________________________________________________________________________________________
*** CID 463144:    (CHECKED_RETURN)
/boot/expo.c: 257 in expo_apply_theme()
251             struct expo_theme *theme = &exp->theme;
252             int ret;
253
254             log_debug("Applying theme %s\n", ofnode_get_name(node));
255
256             memset(theme, '\0', sizeof(struct expo_theme));
>>>     CID 463144:    (CHECKED_RETURN)
>>>     Calling "ofnode_read_u32" without checking return value (as is done elsewhere 33 out of 41 times).
257             ofnode_read_u32(node, "font-size", &theme->font_size);
258             ofnode_read_u32(node, "menu-inset", &theme->menu_inset);
259             ofnode_read_u32(node, "menuitem-gap-y", &theme->menuitem_gap_y);
260
261             list_for_each_entry(scn, &exp->scene_head, sibling) {
262                     ret = scene_apply_theme(scn, theme);
/boot/expo.c: 258 in expo_apply_theme()
252             int ret;
253
254             log_debug("Applying theme %s\n", ofnode_get_name(node));
255
256             memset(theme, '\0', sizeof(struct expo_theme));
257             ofnode_read_u32(node, "font-size", &theme->font_size);
>>>     CID 463144:    (CHECKED_RETURN)
>>>     Calling "ofnode_read_u32" without checking return value (as is done elsewhere 33 out of 41 times).
258             ofnode_read_u32(node, "menu-inset", &theme->menu_inset);
259             ofnode_read_u32(node, "menuitem-gap-y", &theme->menuitem_gap_y);
260
261             list_for_each_entry(scn, &exp->scene_head, sibling) {
262                     ret = scene_apply_theme(scn, theme);
263                     if (ret)
/boot/expo.c: 259 in expo_apply_theme()
253
254             log_debug("Applying theme %s\n", ofnode_get_name(node));
255
256             memset(theme, '\0', sizeof(struct expo_theme));
257             ofnode_read_u32(node, "font-size", &theme->font_size);
258             ofnode_read_u32(node, "menu-inset", &theme->menu_inset);
>>>     CID 463144:    (CHECKED_RETURN)
>>>     Calling "ofnode_read_u32" without checking return value (as is done elsewhere 33 out of 41 times).
259             ofnode_read_u32(node, "menuitem-gap-y", &theme->menuitem_gap_y);
260
261             list_for_each_entry(scn, &exp->scene_head, sibling) {
262                     ret = scene_apply_theme(scn, theme);
263                     if (ret)
264                             return log_msg_ret("app", ret);
265             }
266
267             return 0;

** CID 463143:  Control flow issues  (DEADCODE)
/test/dm/part.c: 124 in do_get_info_test()


________________________________________________________________________________________________________
*** CID 463143:  Control flow issues  (DEADCODE)
/test/dm/part.c: 124 in do_get_info_test()
118
119             memset(&p, 0, sizeof(p));
120
121             ret = part_get_info_by_type(dev_desc, part, part_type, &p);
122             printf("part_get_info_by_type(%d, 0x%x) = %d\n", part,
part_type, ret);
123             if (ut_assertok(ret)) {
>>>     CID 463143:  Control flow issues  (DEADCODE)
>>>     Execution cannot reach this statement: "return 0;".
124                     return 0;
125             }
126
127             ut_asserteq(reference->start, p.start);
128             ut_asserteq(reference->size, p.size);
129             ut_asserteq(reference->sys_ind, p.sys_ind);


________________________________________________________________________________________________________

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
@ 2023-05-29 20:04 Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-05-29 20:04 UTC (permalink / raw)
  To: u-boot; +Cc: Ralph Siemsen

[-- Attachment #1: Type: text/plain, Size: 2547 bytes --]

Here's the latest report.

---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, May 29, 2023, 11:10 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.

2 new defect(s) introduced to Das U-Boot found with Coverity Scan.


New defect(s) Reported-by: Coverity Scan
Showing 2 of 2 defect(s)


** CID 461871:  Null pointer dereferences  (NULL_RETURNS)
/tools/renesas_spkgimage.c: 56 in spkgimage_parse_config_line()


________________________________________________________________________________________________________
*** CID 461871:  Null pointer dereferences  (NULL_RETURNS)
/tools/renesas_spkgimage.c: 56 in spkgimage_parse_config_line()
50      char *saveptr;
51      char *delim = "\t ";
52      char *name = strtok_r(line, delim, &saveptr);
53      char *val_str = strtok_r(NULL, delim, &saveptr);
54      int value = atoi(val_str);
55
>>>     CID 461871:  Null pointer dereferences  (NULL_RETURNS)
>>>     Dereferencing a pointer that might be "NULL" "name" when calling
"strcmp". [Note: The source code implementation of the function has been
overridden by a builtin model.]
56      if (!strcmp("VERSION", name)) {
57              conf.version = check_range(name, value, 1, 15);
58      } else if (!strcmp("NAND_ECC_ENABLE", name)) {
59              conf.ecc_enable = check_range(name, value, 0, 1);
60      } else if (!strcmp("NAND_ECC_BLOCK_SIZE", name)) {
61              conf.ecc_block_size = check_range(name, value, 0, 2);

** CID 461870:  Resource leaks  (RESOURCE_LEAK)
/tools/renesas_spkgimage.c: 106 in spkgimage_parse_config_file()


________________________________________________________________________________________________________
*** CID 461870:  Resource leaks  (RESOURCE_LEAK)
/tools/renesas_spkgimage.c: 106 in spkgimage_parse_config_file()
100
101                     /* Strip any trailing newline */
102                     line[strcspn(line, "\n")] = 0;
103
104                     /* Parse the line */
105                     if (spkgimage_parse_config_line(line, line_num))
>>>     CID 461870:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "fcfg" going out of scope leaks the storage it points to.
106                             return -EINVAL;
107             }
108
109             fclose(fcfg);
110
111             /* Avoid divide-by-zero later on */


-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
@ 2023-03-27 19:19 Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-03-27 19:19 UTC (permalink / raw)
  To: u-boot

[-- Attachment #1: Type: text/plain, Size: 14976 bytes --]

Here's the latest report.

---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Mar 27, 2023 at 2:36 PM
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.

6 new defect(s) introduced to Das U-Boot found with Coverity Scan.
2 defect(s), reported by Coverity Scan earlier, were marked fixed in
the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 6 of 6 defect(s)


** CID 451089:  Incorrect expression  (EVALUATION_ORDER)
/lib/efi_loader/efi_device_path.c: 752 in dp_fill()


________________________________________________________________________________________________________
*** CID 451089:  Incorrect expression  (EVALUATION_ORDER)
/lib/efi_loader/efi_device_path.c: 752 in dp_fill()
746                             memcpy(&dp->ns_id, &ns_id, sizeof(ns_id));
747                             return &dp[1];
748                             }
749     #endif
750     #if defined(CONFIG_USB)
751                     case UCLASS_MASS_STORAGE: {
>>>     CID 451089:  Incorrect expression  (EVALUATION_ORDER)
>>>     In "desc = desc = dev_get_uclass_plat(dev)", "desc" is written twice with the same value.
752                             struct blk_desc *desc = desc =
dev_get_uclass_plat(dev);
753                             struct efi_device_path_controller *dp =
754                                     dp_fill(buf, dev->parent);
755
756                             dp->dp.type     =
DEVICE_PATH_TYPE_HARDWARE_DEVICE;
757                             dp->dp.sub_type =
DEVICE_PATH_SUB_TYPE_CONTROLLER;

** CID 450973:    (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 450973:    (TAINTED_SCALAR)
/test/cmd/fdt.c: 133 in make_fuller_fdt()
127             ut_assertok(fdt_property_cell(fdt, "#size-cells", 0));
128             ut_assertok(fdt_property_string(fdt, "compatible",
"u-boot,fdt-subnode-test-device"));
129             ut_assertok(fdt_end_node(fdt));
130             ut_assertok(fdt_end_node(fdt));
131
132             ut_assertok(fdt_end_node(fdt));
>>>     CID 450973:    (TAINTED_SCALAR)
>>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
133             ut_assertok(fdt_finish(fdt));
134
135             return 0;
136     }
137
138     /* Test 'fdt addr' getting/setting address */
/test/cmd/fdt.c: 133 in make_fuller_fdt()
127             ut_assertok(fdt_property_cell(fdt, "#size-cells", 0));
128             ut_assertok(fdt_property_string(fdt, "compatible",
"u-boot,fdt-subnode-test-device"));
129             ut_assertok(fdt_end_node(fdt));
130             ut_assertok(fdt_end_node(fdt));
131
132             ut_assertok(fdt_end_node(fdt));
>>>     CID 450973:    (TAINTED_SCALAR)
>>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
133             ut_assertok(fdt_finish(fdt));
134
135             return 0;
136     }
137
138     /* Test 'fdt addr' getting/setting address */
/test/cmd/fdt.c: 133 in make_fuller_fdt()
127             ut_assertok(fdt_property_cell(fdt, "#size-cells", 0));
128             ut_assertok(fdt_property_string(fdt, "compatible",
"u-boot,fdt-subnode-test-device"));
129             ut_assertok(fdt_end_node(fdt));
130             ut_assertok(fdt_end_node(fdt));
131
132             ut_assertok(fdt_end_node(fdt));
>>>     CID 450973:    (TAINTED_SCALAR)
>>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
133             ut_assertok(fdt_finish(fdt));
134
135             return 0;
136     }
137
138     /* Test 'fdt addr' getting/setting address */
/test/cmd/fdt.c: 133 in make_fuller_fdt()
127             ut_assertok(fdt_property_cell(fdt, "#size-cells", 0));
128             ut_assertok(fdt_property_string(fdt, "compatible",
"u-boot,fdt-subnode-test-device"));
129             ut_assertok(fdt_end_node(fdt));
130             ut_assertok(fdt_end_node(fdt));
131
132             ut_assertok(fdt_end_node(fdt));
>>>     CID 450973:    (TAINTED_SCALAR)
>>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
133             ut_assertok(fdt_finish(fdt));
134
135             return 0;
136     }
137
138     /* Test 'fdt addr' getting/setting address */
/test/cmd/fdt.c: 133 in make_fuller_fdt()
127             ut_assertok(fdt_property_cell(fdt, "#size-cells", 0));
128             ut_assertok(fdt_property_string(fdt, "compatible",
"u-boot,fdt-subnode-test-device"));
129             ut_assertok(fdt_end_node(fdt));
130             ut_assertok(fdt_end_node(fdt));
131
132             ut_assertok(fdt_end_node(fdt));
>>>     CID 450973:    (TAINTED_SCALAR)
>>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
133             ut_assertok(fdt_finish(fdt));
134
135             return 0;
136     }
137
138     /* Test 'fdt addr' getting/setting address */
/test/cmd/fdt.c: 133 in make_fuller_fdt()
127             ut_assertok(fdt_property_cell(fdt, "#size-cells", 0));
128             ut_assertok(fdt_property_string(fdt, "compatible",
"u-boot,fdt-subnode-test-device"));
129             ut_assertok(fdt_end_node(fdt));
130             ut_assertok(fdt_end_node(fdt));
131
132             ut_assertok(fdt_end_node(fdt));
>>>     CID 450973:    (TAINTED_SCALAR)
>>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
133             ut_assertok(fdt_finish(fdt));
134
135             return 0;
136     }
137
138     /* Test 'fdt addr' getting/setting address */
/test/cmd/fdt.c: 133 in make_fuller_fdt()
127             ut_assertok(fdt_property_cell(fdt, "#size-cells", 0));
128             ut_assertok(fdt_property_string(fdt, "compatible",
"u-boot,fdt-subnode-test-device"));
129             ut_assertok(fdt_end_node(fdt));
130             ut_assertok(fdt_end_node(fdt));
131
132             ut_assertok(fdt_end_node(fdt));
>>>     CID 450973:    (TAINTED_SCALAR)
>>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
133             ut_assertok(fdt_finish(fdt));
134
135             return 0;
136     }
137
138     /* Test 'fdt addr' getting/setting address */

** CID 450972:    (PRINTF_ARGS)


________________________________________________________________________________________________________
*** CID 450972:    (PRINTF_ARGS)
/test/cmd/fdt.c: 267 in fdt_test_move()
261             ut_assertok(run_commandf("fdt move %08x %08x %x",
addr, newaddr, ts));
262             ut_assert_nextline("Working FDT set to %lx", newaddr);
263             ut_assertok(ut_check_console_end(uts));
264
265             /* Compare the source and destination DTs */
266             ut_assertok(console_record_reset_enable());
>>>     CID 450972:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
267             ut_assertok(run_commandf("cmp.b %08x %08x %x", addr,
newaddr, ts));
268             ut_assert_nextline("Total of %d byte(s) were the same", ts);
269             ut_assertok(ut_check_console_end(uts));
270
271             return 0;
272     }
/test/cmd/fdt.c: 261 in fdt_test_move()
255             /* Moved target DT location */
256             buf = map_sysmem(newaddr, size);
257             memset(buf, 0, size);
258
259             /* Test moving the working FDT to a new location */
260             ut_assertok(console_record_reset_enable());
>>>     CID 450972:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
261             ut_assertok(run_commandf("fdt move %08x %08x %x",
addr, newaddr, ts));
262             ut_assert_nextline("Working FDT set to %lx", newaddr);
263             ut_assertok(ut_check_console_end(uts));
264
265             /* Compare the source and destination DTs */
266             ut_assertok(console_record_reset_enable());

** CID 450970:    (PRINTF_ARGS)


________________________________________________________________________________________________________
*** CID 450970:    (PRINTF_ARGS)
/test/cmd/fdt.c: 224 in fdt_test_addr_resize()
218             ut_assertok(console_record_reset_enable());
219             ut_assertok(run_commandf("fdt addr %08x %x", addr, newsize));
220             ut_assert_nextline("Working FDT set to %lx", addr);
221             ut_assertok(ut_check_console_end(uts));
222
223             /* Try shrinking it */
>>>     CID 450970:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
224             ut_assertok(run_commandf("fdt addr %08x %x", addr,
sizeof(fdt) / 4));
225             ut_assert_nextline("Working FDT set to %lx", addr);
226             ut_assert_nextline("New length %d < existing length
%d, ignoring",
227                                (int)sizeof(fdt) / 4, newsize);
228             ut_assertok(ut_check_console_end(uts));
229
/test/cmd/fdt.c: 219 in fdt_test_addr_resize()
213             ut_assertok(make_test_fdt(uts, fdt, sizeof(fdt)));
214             addr = map_to_sysmem(fdt);
215             set_working_fdt_addr(addr);
216
217             /* Test setting and resizing the working FDT to a larger size */
218             ut_assertok(console_record_reset_enable());
>>>     CID 450970:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
219             ut_assertok(run_commandf("fdt addr %08x %x", addr, newsize));
220             ut_assert_nextline("Working FDT set to %lx", addr);
221             ut_assertok(ut_check_console_end(uts));
222
223             /* Try shrinking it */
224             ut_assertok(run_commandf("fdt addr %08x %x", addr,
sizeof(fdt) / 4));
/test/cmd/fdt.c: 231 in fdt_test_addr_resize()
225             ut_assert_nextline("Working FDT set to %lx", addr);
226             ut_assert_nextline("New length %d < existing length
%d, ignoring",
227                                (int)sizeof(fdt) / 4, newsize);
228             ut_assertok(ut_check_console_end(uts));
229
230             /* ...quietly */
>>>     CID 450970:    (PRINTF_ARGS)
>>>     Argument "addr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
231             ut_assertok(run_commandf("fdt addr -q %08x %x", addr,
sizeof(fdt) / 4));
232             ut_assert_nextline("Working FDT set to %lx", addr);
233             ut_assertok(ut_check_console_end(uts));
234
235             /* We cannot easily provoke errors in fdt_open_into(),
so ignore that */
236

** CID 450968:    (PRINTF_ARGS)


________________________________________________________________________________________________________
*** CID 450968:    (PRINTF_ARGS)
/test/cmd/fdt.c: 224 in fdt_test_addr_resize()
218             ut_assertok(console_record_reset_enable());
219             ut_assertok(run_commandf("fdt addr %08x %x", addr, newsize));
220             ut_assert_nextline("Working FDT set to %lx", addr);
221             ut_assertok(ut_check_console_end(uts));
222
223             /* Try shrinking it */
>>>     CID 450968:    (PRINTF_ARGS)
>>>     Argument "64UL" to format specifier "%x" was expected to have type "unsigned int" but has type "unsigned long".
224             ut_assertok(run_commandf("fdt addr %08x %x", addr,
sizeof(fdt) / 4));
225             ut_assert_nextline("Working FDT set to %lx", addr);
226             ut_assert_nextline("New length %d < existing length
%d, ignoring",
227                                (int)sizeof(fdt) / 4, newsize);
228             ut_assertok(ut_check_console_end(uts));
229
/test/cmd/fdt.c: 231 in fdt_test_addr_resize()
225             ut_assert_nextline("Working FDT set to %lx", addr);
226             ut_assert_nextline("New length %d < existing length
%d, ignoring",
227                                (int)sizeof(fdt) / 4, newsize);
228             ut_assertok(ut_check_console_end(uts));
229
230             /* ...quietly */
>>>     CID 450968:    (PRINTF_ARGS)
>>>     Argument "64UL" to format specifier "%x" was expected to have type "unsigned int" but has type "unsigned long".
231             ut_assertok(run_commandf("fdt addr -q %08x %x", addr,
sizeof(fdt) / 4));
232             ut_assert_nextline("Working FDT set to %lx", addr);
233             ut_assertok(ut_check_console_end(uts));
234
235             /* We cannot easily provoke errors in fdt_open_into(),
so ignore that */
236

** CID 450967:    (PRINTF_ARGS)


________________________________________________________________________________________________________
*** CID 450967:    (PRINTF_ARGS)
/test/cmd/fdt.c: 261 in fdt_test_move()
255             /* Moved target DT location */
256             buf = map_sysmem(newaddr, size);
257             memset(buf, 0, size);
258
259             /* Test moving the working FDT to a new location */
260             ut_assertok(console_record_reset_enable());
>>>     CID 450967:    (PRINTF_ARGS)
>>>     Argument "newaddr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
261             ut_assertok(run_commandf("fdt move %08x %08x %x",
addr, newaddr, ts));
262             ut_assert_nextline("Working FDT set to %lx", newaddr);
263             ut_assertok(ut_check_console_end(uts));
264
265             /* Compare the source and destination DTs */
266             ut_assertok(console_record_reset_enable());
/test/cmd/fdt.c: 267 in fdt_test_move()
261             ut_assertok(run_commandf("fdt move %08x %08x %x",
addr, newaddr, ts));
262             ut_assert_nextline("Working FDT set to %lx", newaddr);
263             ut_assertok(ut_check_console_end(uts));
264
265             /* Compare the source and destination DTs */
266             ut_assertok(console_record_reset_enable());
>>>     CID 450967:    (PRINTF_ARGS)
>>>     Argument "newaddr" to format specifier "%08x" was expected to have type "unsigned int" but has type "unsigned long".
267             ut_assertok(run_commandf("cmp.b %08x %08x %x", addr,
newaddr, ts));
268             ut_assert_nextline("Total of %d byte(s) were the same", ts);
269             ut_assertok(ut_check_console_end(uts));
270
271             return 0;
272     }


----- End forwarded message -----

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
@ 2023-01-31 15:02 Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2023-01-31 15:02 UTC (permalink / raw)
  To: u-boot

[-- Attachment #1: Type: text/plain, Size: 28119 bytes --]

----- Forwarded message from Tom Rini <tom.rini@gmail.com> -----

Date: Tue, 31 Jan 2023 07:30:23 -0500
From: Tom Rini <tom.rini@gmail.com>
To: trini@konsulko.com
Subject: Fwd: New Defects reported by Coverity Scan for Das U-Boot

On Mon, Jan 30, 2023, 4:15 PM <scan-admin@coverity.com> wrote:

> Hi,
>
> Please find the latest report on new defect(s) introduced to Das U-Boot
> found with Coverity Scan.
>
> 18 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 9 defect(s), reported by Coverity Scan earlier, were marked fixed in the
> recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 18 of 18 defect(s)
>
>
> ** CID 435669:  Control flow issues  (MISSING_BREAK)
> /lib/vsprintf.c: 681 in vsnprintf_internal()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435669:  Control flow issues  (MISSING_BREAK)
> /lib/vsprintf.c: 681 in vsnprintf_internal()
> 675                     case 'x':
> 676                             flags |= SMALL;
> 677                     case 'X':
> 678                             base = 16;
> 679                             break;
> 680
> >>>     CID 435669:  Control flow issues  (MISSING_BREAK)
> >>>     The case for value "'d'" is not terminated by a "break" statement.
> 681                     case 'd':
> 682                             if (fmt[1] == 'E')
> 683                                     flags |= ERRSTR;
> 684                     case 'i':
> 685                             flags |= SIGN;
> 686                     case 'u':
>
> ** CID 435668:  Insecure data handling  (TAINTED_SCALAR)
> /boot/image-fdt.c: 397 in select_fdt()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435668:  Insecure data handling  (TAINTED_SCALAR)
> /boot/image-fdt.c: 397 in select_fdt()
> 391                                     return -EFAULT;
> 392                             }
> 393
> 394                             debug("   Loading FDT from 0x%08lx to
> 0x%08lx\n",
> 395                                   image_data, load);
> 396
> >>>     CID 435668:  Insecure data handling  (TAINTED_SCALAR)
> >>>     Passing tainted expression "image_get_data_size(fdt_hdr)" to
> "memmove", which uses it as an offset. [Note: The source code
> implementation of the function has been overridden by a builtin model.]
> 397                             memmove((void *)load,
> 398                                     (void *)image_data,
> 399                                     image_get_data_size(fdt_hdr));
> 400
> 401                             fdt_addr = load;
> 402                             break;
>
> ** CID 435667:  Memory - corruptions  (OVERRUN)
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435667:  Memory - corruptions  (OVERRUN)
> /lib/zstd/decompress/zstd_decompress.c: 88 in ZSTD_DDictHashSet_getIndex()
> 82     #define DDICT_HASHSET_RESIZE_FACTOR 2
> 83
> 84     /* Hash function to determine starting position of dict insertion
> within the table
> 85      * Returns an index between [0, hashSet->ddictPtrTableSize]
> 86      */
> 87     static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet*
> hashSet, U32 dictID) {
> >>>     CID 435667:  Memory - corruptions  (OVERRUN)
> >>>     Overrunning buffer pointed to by "&dictID" of 4 bytes by passing
> it to a function which accesses it at byte offset 7.
> 88         const U64 hash = xxh64(&dictID, sizeof(U32), 0);
> 89         /* DDict ptr table size is a multiple of 2, use size - 1 as
> mask to get index within [0, hashSet->ddictPtrTableSize) */
> 90         return hash & (hashSet->ddictPtrTableSize - 1);
> 91     }
> 92
> 93     /* Adds DDict to a hashset without resizing it.
>
> ** CID 435666:  Insecure data handling  (TAINTED_SCALAR)
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435666:  Insecure data handling  (TAINTED_SCALAR)
> /common/command.c: 674 in cmd_source_script()
> 668             ret = image_locate_script(buf, 0, fit_uname, confname,
> &data, &len);
> 669             unmap_sysmem(buf);
> 670             if (ret)
> 671                     return CMD_RET_FAILURE;
> 672
> 673             debug("** Script length: %d\n", len);
> >>>     CID 435666:  Insecure data handling  (TAINTED_SCALAR)
> >>>     Passing tainted expression "len" to "run_command_list", which uses
> it as an offset.
> 674             return run_command_list(data, len, 0);
>
> ** CID 435665:    (DEADCODE)
> /tools/fit_image.c: 342 in fit_write_images()
> /tools/fit_image.c: 322 in fit_write_images()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435665:    (DEADCODE)
> /tools/fit_image.c: 342 in fit_write_images()
> 336                     ret = fdt_property_file(params, fdt, FIT_DATA_PROP,
> 337                                             params->fit_ramdisk);
> 338                     if (ret)
> 339                             return ret;
> 340                     fit_add_hash_or_sign(params, fdt, true);
> 341                     if (ret)
> >>>     CID 435665:    (DEADCODE)
> >>>     Execution cannot reach this statement: "return ret;".
> 342                             return ret;
> 343                     fdt_end_node(fdt);
> 344             }
> 345
> 346             fdt_end_node(fdt);
> 347
> /tools/fit_image.c: 322 in fit_write_images()
> 316                     fdt_property_string(fdt, FIT_ARCH_PROP,
> 317
>  genimg_get_arch_short_name(params->arch));
> 318                     fdt_property_string(fdt, FIT_COMP_PROP,
> 319
>  genimg_get_comp_short_name(IH_COMP_NONE));
> 320                     fit_add_hash_or_sign(params, fdt, true);
> 321                     if (ret)
> >>>     CID 435665:    (DEADCODE)
> >>>     Execution cannot reach this statement: "return ret;".
> 322                             return ret;
> 323                     fdt_end_node(fdt);
> 324             }
> 325
> 326             /* And a ramdisk file if available */
> 327             if (params->fit_ramdisk) {
>
> ** CID 435664:  Insecure data handling  (TAINTED_SCALAR)
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435664:  Insecure data handling  (TAINTED_SCALAR)
> /drivers/iommu/iommu-uclass.c: 71 in dev_pci_iommu_enable()
> 65                      return ret;
> 66              }
> 67              dev->iommu = dev_iommu;
> 68              break;
> 69      }
> 70
> >>>     CID 435664:  Insecure data handling  (TAINTED_SCALAR)
> >>>     Passing tainted expression "*iommu_map" to "dlfree", which uses it
> as an offset.
> 71      free(iommu_map);
> 72      return 0;
> 73     }
> 74     #endif
> 75
> 76     int dev_iommu_enable(struct udevice *dev)
>
> ** CID 435663:  Code maintainability issues  (UNUSED_VALUE)
> /boot/bootdev-uclass.c: 703 in bootdev_setup_iter()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435663:  Code maintainability issues  (UNUSED_VALUE)
> /boot/bootdev-uclass.c: 703 in bootdev_setup_iter()
> 697                     iter->labels = bootstd_get_bootdev_order(bootstd,
> &ok);
> 698                     if (!ok)
> 699                             return log_msg_ret("ord", -ENOMEM);
> 700                     log_debug("setup labels %p\n", iter->labels);
> 701                     if (iter->labels) {
> 702                             iter->cur_label = -1;
> >>>     CID 435663:  Code maintainability issues  (UNUSED_VALUE)
> >>>     Assigning value from "bootdev_next_label(iter, &dev,
> &method_flags)" to "ret" here, but that stored value is overwritten before
> it can be used.
> 703                             ret = bootdev_next_label(iter, &dev,
> &method_flags);
> 704                     } else {
> 705                             ret = bootdev_next_prio(iter, &dev);
> 706                             method_flags = 0;
> 707                     }
> 708                     if (!dev)
>
> ** CID 435662:  Null pointer dereferences  (REVERSE_INULL)
> /boot/scene_menu.c: 385 in scene_menu_display()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435662:  Null pointer dereferences  (REVERSE_INULL)
> /boot/scene_menu.c: 385 in scene_menu_display()
> 379
> 380                     desc = scene_obj_find(scn, item->desc_id,
> SCENEOBJT_TEXT);
> 381                     if (desc)
> 382                             dstr = expo_get_str(exp, desc->str_id);
> 383
> 384                     printf("%3s  %3s  %-10s  %s\n",
> >>>     CID 435662:  Null pointer dereferences  (REVERSE_INULL)
> >>>     Null-checking "pointer" suggests that it may be null, but it has
> already been dereferenced on all paths leading to the check.
> 385                            pointer && menu->cur_item_id == item->id ?
> pstr : "",
> 386                            kstr, lstr, dstr);
> 387             }
> 388
> 389             return -ENOTSUPP;
>
> ** CID 435661:    (TAINTED_SCALAR)
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435661:    (TAINTED_SCALAR)
> /lib/zstd/decompress/huf_decompress.c: 591 in
> HUF_decompress4X1_usingDTable_internal_body()
> 585             DTableDesc const dtd = HUF_getDTableDesc(DTable);
> 586             U32 const dtLog = dtd.tableLog;
> 587             U32 endSignal = 1;
> 588
> 589             if (length4 > cSrcSize) return
> ERROR(corruption_detected);   /* overflow */
> 590             if (opStart4 > oend) return ERROR(corruption_detected);
>   /* overflow */
> >>>     CID 435661:    (TAINTED_SCALAR)
> >>>     Passing tainted expression "length1" to "BIT_initDStream", which
> uses it as an offset.
> 591             CHECK_F( BIT_initDStream(&bitD1, istart1, length1) );
> 592             CHECK_F( BIT_initDStream(&bitD2, istart2, length2) );
> 593             CHECK_F( BIT_initDStream(&bitD3, istart3, length3) );
> 594             CHECK_F( BIT_initDStream(&bitD4, istart4, length4) );
> 595
> 596             /* up to 16 symbols per loop (4 symbols per stream) in
> 64-bit mode */
> /lib/zstd/decompress/huf_decompress.c: 593 in
> HUF_decompress4X1_usingDTable_internal_body()
> 587             U32 endSignal = 1;
> 588
> 589             if (length4 > cSrcSize) return
> ERROR(corruption_detected);   /* overflow */
> 590             if (opStart4 > oend) return ERROR(corruption_detected);
>   /* overflow */
> 591             CHECK_F( BIT_initDStream(&bitD1, istart1, length1) );
> 592             CHECK_F( BIT_initDStream(&bitD2, istart2, length2) );
> >>>     CID 435661:    (TAINTED_SCALAR)
> >>>     Passing tainted expression "length3" to "BIT_initDStream", which
> uses it as an offset.
> 593             CHECK_F( BIT_initDStream(&bitD3, istart3, length3) );
> 594             CHECK_F( BIT_initDStream(&bitD4, istart4, length4) );
> 595
> 596             /* up to 16 symbols per loop (4 symbols per stream) in
> 64-bit mode */
> 597             if ((size_t)(oend - op4) >= sizeof(size_t)) {
> 598                 for ( ; (endSignal) & (op4 < olimit) ; ) {
> /lib/zstd/decompress/huf_decompress.c: 592 in
> HUF_decompress4X1_usingDTable_internal_body()
> 586             U32 const dtLog = dtd.tableLog;
> 587             U32 endSignal = 1;
> 588
> 589             if (length4 > cSrcSize) return
> ERROR(corruption_detected);   /* overflow */
> 590             if (opStart4 > oend) return ERROR(corruption_detected);
>   /* overflow */
> 591             CHECK_F( BIT_initDStream(&bitD1, istart1, length1) );
> >>>     CID 435661:    (TAINTED_SCALAR)
> >>>     Passing tainted expression "length2" to "BIT_initDStream", which
> uses it as an offset.
> 592             CHECK_F( BIT_initDStream(&bitD2, istart2, length2) );
> 593             CHECK_F( BIT_initDStream(&bitD3, istart3, length3) );
> 594             CHECK_F( BIT_initDStream(&bitD4, istart4, length4) );
> 595
> 596             /* up to 16 symbols per loop (4 symbols per stream) in
> 64-bit mode */
> 597             if ((size_t)(oend - op4) >= sizeof(size_t)) {
>
> ** CID 435660:    (PRINTF_ARGS)
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435660:    (PRINTF_ARGS)
> /test/cmd/exit.c: 69 in cmd_exit_test()
> 63      ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz'
> ; run foo ; echo $?", i));
> 64      ut_assert_nextline("bar");
> 65      ut_assert_nextline("0");
> 66      ut_assertok(ut_check_console_end(uts));
> 67
> 68      ut_assertok(console_record_reset_enable());
> >>>     CID 435660:    (PRINTF_ARGS)
> >>>     This argument was not used by the format string: "i".
> 69      ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz'
> ; run foo && echo quux ; echo $?", i));
> 70      ut_assert_nextline("bar");
> 71      ut_assert_nextline("quux");
> 72      ut_assert_nextline("0");
> 73      ut_assertok(ut_check_console_end(uts));
> 74
> /test/cmd/exit.c: 104 in cmd_exit_test()
> 98      ut_assert_nextline("bar");
> 99      /* The 'true' returns 0 */
> 100             ut_assert_nextline("0");
> 101             ut_assertok(ut_check_console_end(uts));
> 102
> 103             ut_assertok(console_record_reset_enable());
> >>>     CID 435660:    (PRINTF_ARGS)
> >>>     This argument was not used by the format string: "i".
> 104             ut_assertok(run_commandf("setenv foo 'echo bar ; false' ;
> run foo ; echo $?", i));
> 105             ut_assert_nextline("bar");
> 106             ut_assert_nextline("1");
> 107             ut_assertok(ut_check_console_end(uts));
> 108
> 109             ut_assertok(console_record_reset_enable());
> /test/cmd/exit.c: 110 in cmd_exit_test()
> 104             ut_assertok(run_commandf("setenv foo 'echo bar ; false' ;
> run foo ; echo $?", i));
> 105             ut_assert_nextline("bar");
> 106             ut_assert_nextline("1");
> 107             ut_assertok(ut_check_console_end(uts));
> 108
> 109             ut_assertok(console_record_reset_enable());
> >>>     CID 435660:    (PRINTF_ARGS)
> >>>     This argument was not used by the format string: "i".
> 110             ut_assertok(run_commandf("setenv foo 'echo bar ; false' ;
> run foo && echo quux ; echo $?", i));
> 111             ut_assert_nextline("bar");
> 112             ut_assert_nextline("1");
> 113             ut_assertok(ut_check_console_end(uts));
> 114
> 115             ut_assertok(console_record_reset_enable());
> /test/cmd/exit.c: 63 in cmd_exit_test()
> 57              ut_assert_nextline("0");
> 58              ut_assertok(ut_check_console_end(uts));
> 59      }
> 60
> 61      /* Validate that 'exit' behaves the same way as 'exit 0' */
> 62      ut_assertok(console_record_reset_enable());
> >>>     CID 435660:    (PRINTF_ARGS)
> >>>     This argument was not used by the format string: "i".
> 63      ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz'
> ; run foo ; echo $?", i));
> 64      ut_assert_nextline("bar");
> 65      ut_assert_nextline("0");
> 66      ut_assertok(ut_check_console_end(uts));
> 67
> 68      ut_assertok(console_record_reset_enable());
> /test/cmd/exit.c: 84 in cmd_exit_test()
> 78      /* Either 'exit' returns 0, or 'echo quux' returns 0 */
> 79      ut_assert_nextline("0");
> 80      ut_assertok(ut_check_console_end(uts));
> 81
> 82      /* Validate that return value still propagates from 'run' command
> */
> 83      ut_assertok(console_record_reset_enable());
> >>>     CID 435660:    (PRINTF_ARGS)
> >>>     This argument was not used by the format string: "i".
> 84      ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo ;
> echo $?", i));
> 85      ut_assert_nextline("bar");
> 86      ut_assert_nextline("0");
> 87      ut_assertok(ut_check_console_end(uts));
> 88
> 89      ut_assertok(console_record_reset_enable());
> /test/cmd/exit.c: 116 in cmd_exit_test()
> 110             ut_assertok(run_commandf("setenv foo 'echo bar ; false' ;
> run foo && echo quux ; echo $?", i));
> 111             ut_assert_nextline("bar");
> 112             ut_assert_nextline("1");
> 113             ut_assertok(ut_check_console_end(uts));
> 114
> 115             ut_assertok(console_record_reset_enable());
> >>>     CID 435660:    (PRINTF_ARGS)
> >>>     This argument was not used by the format string: "i".
> 116             ut_assertok(run_commandf("setenv foo 'echo bar ; false' ;
> run foo || echo quux ; echo $?", i));
> 117             ut_assert_nextline("bar");
> 118             ut_assert_nextline("quux");
> 119             /* The 'echo quux' returns 0 */
> 120             ut_assert_nextline("0");
> 121             ut_assertok(ut_check_console_end(uts));
> /test/cmd/exit.c: 76 in cmd_exit_test()
> 70      ut_assert_nextline("bar");
> 71      ut_assert_nextline("quux");
> 72      ut_assert_nextline("0");
> 73      ut_assertok(ut_check_console_end(uts));
> 74
> 75      ut_assertok(console_record_reset_enable());
> >>>     CID 435660:    (PRINTF_ARGS)
> >>>     This argument was not used by the format string: "i".
> 76      ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz'
> ; run foo || echo quux ; echo $?", i));
> 77      ut_assert_nextline("bar");
> 78      /* Either 'exit' returns 0, or 'echo quux' returns 0 */
> 79      ut_assert_nextline("0");
> 80      ut_assertok(ut_check_console_end(uts));
> 81
> /test/cmd/exit.c: 90 in cmd_exit_test()
> 84      ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo ;
> echo $?", i));
> 85      ut_assert_nextline("bar");
> 86      ut_assert_nextline("0");
> 87      ut_assertok(ut_check_console_end(uts));
> 88
> 89      ut_assertok(console_record_reset_enable());
> >>>     CID 435660:    (PRINTF_ARGS)
> >>>     This argument was not used by the format string: "i".
> 90      ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo
> && echo quux ; echo $?", i));
> 91      ut_assert_nextline("bar");
> 92      ut_assert_nextline("quux");
> 93      ut_assert_nextline("0");
> 94      ut_assertok(ut_check_console_end(uts));
> 95
> /test/cmd/exit.c: 97 in cmd_exit_test()
> 91      ut_assert_nextline("bar");
> 92      ut_assert_nextline("quux");
> 93      ut_assert_nextline("0");
> 94      ut_assertok(ut_check_console_end(uts));
> 95
> 96      ut_assertok(console_record_reset_enable());
> >>>     CID 435660:    (PRINTF_ARGS)
> >>>     This argument was not used by the format string: "i".
> 97      ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo
> || echo quux ; echo $?", i));
> 98      ut_assert_nextline("bar");
> 99      /* The 'true' returns 0 */
> 100             ut_assert_nextline("0");
> 101             ut_assertok(ut_check_console_end(uts));
> 102
>
> ** CID 435659:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
> /cmd/eficonfig.c: 429 in eficonfig_process_common()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435659:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
> /cmd/eficonfig.c: 429 in eficonfig_process_common()
> 423             struct menu *menu;
> 424             void *choice = NULL;
> 425             struct list_head *pos, *n;
> 426             struct eficonfig_entry *entry;
> 427             efi_status_t ret = EFI_SUCCESS;
> 428
> >>>     CID 435659:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
> >>>     "efi_menu->count > 2147483647 /* (int)(~0U >> 1) */" is always
> false regardless of the values of its operands. This occurs as the logical
> operand of "if".
> 429             if (efi_menu->count > EFICONFIG_ENTRY_NUM_MAX)
> 430                     return EFI_OUT_OF_RESOURCES;
> 431
> 432             efi_menu->delay = -1;
> 433             efi_menu->active = 0;
> 434             efi_menu->start = 0;
>
> ** CID 435658:  Insecure data handling  (TAINTED_SCALAR)
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435658:  Insecure data handling  (TAINTED_SCALAR)
> /cmd/ximg.c: 256 in do_imgextract()
> 250                     puts("OK\n");
> 251             }
> 252
> 253             flush_cache(dest, ALIGN(len, ARCH_DMA_MINALIGN));
> 254
> 255             env_set_hex("fileaddr", data);
> >>>     CID 435658:  Insecure data handling  (TAINTED_SCALAR)
> >>>     Passing tainted expression "len" to "env_set_hex", which uses it
> as an offset.
> 256             env_set_hex("filesize", len);
> 257
> 258             return 0;
> 259     }
> 260
> 261     #ifdef CONFIG_SYS_LONGHELP
>
> ** CID 435657:  Integer handling issues  (NEGATIVE_RETURNS)
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435657:  Integer handling issues  (NEGATIVE_RETURNS)
> /fs/squashfs/sqfs_decompressor.c: 146 in sqfs_decompress()
> 140                     break;
> 141     #endif
> 142     #if IS_ENABLED(CONFIG_ZSTD)
> 143             case SQFS_COMP_ZSTD:
> 144                     ret = sqfs_zstd_decompress(ctxt, dest, *dest_len,
> source, src_len);
> 145                     if (ret) {
> >>>     CID 435657:  Integer handling issues  (NEGATIVE_RETURNS)
> >>>     "ret" is passed to a parameter that cannot be negative.
> 146                             printf("ZSTD Error code: %d\n",
> zstd_get_error_code(ret));
> 147                             return -EINVAL;
> 148                     }
> 149
> 150                     break;
> 151     #endif
>
> ** CID 435656:  Code maintainability issues  (UNUSED_VALUE)
> /boot/bootdev-uclass.c: 705 in bootdev_setup_iter()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435656:  Code maintainability issues  (UNUSED_VALUE)
> /boot/bootdev-uclass.c: 705 in bootdev_setup_iter()
> 699                             return log_msg_ret("ord", -ENOMEM);
> 700                     log_debug("setup labels %p\n", iter->labels);
> 701                     if (iter->labels) {
> 702                             iter->cur_label = -1;
> 703                             ret = bootdev_next_label(iter, &dev,
> &method_flags);
> 704                     } else {
> >>>     CID 435656:  Code maintainability issues  (UNUSED_VALUE)
> >>>     Assigning value from "bootdev_next_prio(iter, &dev)" to "ret"
> here, but that stored value is overwritten before it can be used.
> 705                             ret = bootdev_next_prio(iter, &dev);
> 706                             method_flags = 0;
> 707                     }
> 708                     if (!dev)
> 709                             return log_msg_ret("fin", -ENOENT);
> 710                     log_debug("Selected bootdev: %s\n", dev->name);
>
> ** CID 435655:  Error handling issues  (CHECKED_RETURN)
> /boot/scene.c: 219 in scene_obj_set_pos()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435655:  Error handling issues  (CHECKED_RETURN)
> /boot/scene.c: 219 in scene_obj_set_pos()
> 213             obj = scene_obj_find(scn, id, SCENEOBJT_NONE);
> 214             if (!obj)
> 215                     return log_msg_ret("find", -ENOENT);
> 216             obj->x = x;
> 217             obj->y = y;
> 218             if (obj->type == SCENEOBJT_MENU)
> >>>     CID 435655:  Error handling issues  (CHECKED_RETURN)
> >>>     Calling "scene_menu_arrange" without checking return value (as is
> done elsewhere 4 out of 5 times).
> 219                     scene_menu_arrange(scn, (struct scene_obj_menu
> *)obj);
> 220
> 221             return 0;
> 222     }
> 223
> 224     int scene_obj_set_hide(struct scene *scn, uint id, bool hide)
>
> ** CID 435654:  Null pointer dereferences  (NULL_RETURNS)
> /boot/scene_menu.c: 365 in scene_menu_display()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435654:  Null pointer dereferences  (NULL_RETURNS)
> /boot/scene_menu.c: 365 in scene_menu_display()
> 359             }
> 360
> 361             if (list_empty(&menu->item_head))
> 362                     return 0;
> 363
> 364             pointer = scene_obj_find(scn, menu->pointer_id,
> SCENEOBJT_TEXT);
> >>>     CID 435654:  Null pointer dereferences  (NULL_RETURNS)
> >>>     Dereferencing "pointer", which is known to be "NULL".
> 365             pstr = expo_get_str(scn->expo, pointer->str_id);
> 366
> 367             list_for_each_entry(item, &menu->item_head, sibling) {
> 368                     struct scene_obj_txt *key = NULL, *label = NULL;
> 369                     struct scene_obj_txt *desc = NULL;
> 370                     const char *kstr = NULL, *lstr = NULL, *dstr =
> NULL;
>
> ** CID 435653:  Code maintainability issues  (UNUSED_VALUE)
> /boot/scene.c: 290 in scene_obj_render()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 435653:  Code maintainability issues  (UNUSED_VALUE)
> /boot/scene.c: 290 in scene_obj_render()
> 284             struct expo *exp = scn->expo;
> 285             struct udevice *cons, *dev = exp->display;
> 286             int x, y, ret;
> 287
> 288             cons = NULL;
> 289             if (!text_mode) {
> >>>     CID 435653:  Code maintainability issues  (UNUSED_VALUE)
> >>>     Assigning value from "device_find_first_child_by_uclass(dev,
> UCLASS_VIDEO_CONSOLE, &cons)" to "ret" here, but that stored value is
> overwritten before it can be used.
> 290                     ret = device_find_first_child_by_uclass(dev,
> 291
>  UCLASS_VIDEO_CONSOLE,
> 292                                                             &cons);
> 293             }
> 294
> 295             x = obj->x;
>
> ** CID 188663:  Control flow issues  (DEADCODE)
> /lib/zstd/decompress/zstd_decompress_block.c: 1989 in
> ZSTD_decompressBlock_internal()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 188663:  Control flow issues  (DEADCODE)
> /lib/zstd/decompress/zstd_decompress_block.c: 1989 in
> ZSTD_decompressBlock_internal()
> 1983         /* isLongOffset must be true if there are long offsets.
> 1984          * Offsets are long if they are larger than
> 2^STREAM_ACCUMULATOR_MIN.
> 1985          * We don't expect that to be the case in 64-bit mode.
> 1986          * In block mode, window size is not known, so we have to be
> conservative.
> 1987          * (note: but it could be evaluated from current-lowLimit)
> 1988          */
> >>>     CID 188663:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach the expression "57" inside this statement:
> "isLongOffset = (ZSTD_longOf...".
> 1989         ZSTD_longOffset_e const isLongOffset =
> (ZSTD_longOffset_e)(MEM_32bits() && (!frame || (dctx->fParams.windowSize >
> (1ULL << STREAM_ACCUMULATOR_MIN))));
> 1990         DEBUGLOG(5, "ZSTD_decompressBlock_internal (size : %u)",
> (U32)srcSize);
> 1991
> 1992         RETURN_ERROR_IF(srcSize >= ZSTD_BLOCKSIZE_MAX, srcSize_wrong,
> "");
> 1993
> 1994         /* Decode literals section */
>
>
>

----- End forwarded message -----

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
@ 2022-12-06 14:51 Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2022-12-06 14:51 UTC (permalink / raw)
  To: u-boot

[-- Attachment #1: Type: text/plain, Size: 5060 bytes --]

Here's the latest report

---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Dec 5, 2022, 3:35 PM
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.

4 new defect(s) introduced to Das U-Boot found with Coverity Scan.
1 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 4 of 4 defect(s)


** CID 430977:  Null pointer dereferences  (FORWARD_NULL)
/net/ndisc.c: 268 in ndisc_receive()


________________________________________________________________________________________________________
*** CID 430977:  Null pointer dereferences  (FORWARD_NULL)
/net/ndisc.c: 268 in ndisc_receive()
262                                 sizeof(struct in6_addr)) == 0) &&
263                         ndisc_has_option(ip6, ND_OPT_TARGET_LL_ADDR)) {
264                             ndisc_extract_enetaddr(ndisc,
neigh_eth_addr);
265
266                             /* save address for later use */
267                             if (!net_nd_packet_mac)
>>>     CID 430977:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing null pointer "net_nd_packet_mac" to "memcpy", which
dereferences it. [Note: The source code implementation of the function has
been overridden by a builtin model.]
268                                     memcpy(net_nd_packet_mac,
neigh_eth_addr, 7);
269
270                             /* modify header, and transmit it */
271                             memcpy(((struct ethernet_hdr
*)net_nd_tx_packet)->et_dest,
272                                    neigh_eth_addr, 6);
273

** CID 430976:  Control flow issues  (DEADCODE)
/net/tftp.c: 744 in sanitize_tftp_block_size_option()


________________________________________________________________________________________________________
*** CID 430976:  Control flow issues  (DEADCODE)
/net/tftp.c: 744 in sanitize_tftp_block_size_option()
738                     }
739                     /*
740                      * If not CONFIG_IP_DEFRAG, cap at the same value as
741                      * for tftp put, namely normal MTU minus protocol
742                      * overhead.
743                      */
>>>     CID 430976:  Control flow issues  (DEADCODE)
>>>     Execution cannot reach this statement: "[[fallthrough]];".
744                     fallthrough;
745             case TFTPPUT:
746             default:
747                     /*
748                      * U-Boot does not support IP fragmentation on TX,
so
749                      * this must be small enough that it fits normal MTU

** CID 430975:  Control flow issues  (MISSING_BREAK)
/net/net.c: 1270 in net_process_received_packet()


________________________________________________________________________________________________________
*** CID 430975:  Control flow issues  (MISSING_BREAK)
/net/net.c: 1270 in net_process_received_packet()
1264     #ifdef CONFIG_CMD_RARP
1265            case PROT_RARP:
1266                    rarp_receive(ip, len);
1267                    break;
1268     #endif
1269     #if IS_ENABLED(CONFIG_IPV6)
>>>     CID 430975:  Control flow issues  (MISSING_BREAK)
>>>     The case for value "34525" is not terminated by a "break" statement.
1270            case PROT_IP6:
1271                    net_ip6_handler(et, (struct ip6_hdr *)ip, len);
1272     #endif
1273            case PROT_IP:
1274                    debug_cond(DEBUG_NET_PKT, "Got IP\n");
1275                    /* Before we start poking the header, make sure it
is there */

** CID 430974:  Memory - corruptions  (OVERRUN)
/net/ndisc.c: 268 in ndisc_receive()


________________________________________________________________________________________________________
*** CID 430974:  Memory - corruptions  (OVERRUN)
/net/ndisc.c: 268 in ndisc_receive()
262                                 sizeof(struct in6_addr)) == 0) &&
263                         ndisc_has_option(ip6, ND_OPT_TARGET_LL_ADDR)) {
264                             ndisc_extract_enetaddr(ndisc,
neigh_eth_addr);
265
266                             /* save address for later use */
267                             if (!net_nd_packet_mac)
>>>     CID 430974:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "neigh_eth_addr" of 6 bytes by passing it to a
function which accesses it at byte offset 6 using argument "7UL". [Note:
The source code implementation of the function has been overridden by a
builtin model.]
268                                     memcpy(net_nd_packet_mac,
neigh_eth_addr, 7);
269
270                             /* modify header, and transmit it */
271                             memcpy(((struct ethernet_hdr
*)net_nd_tx_packet)->et_dest,
272                                    neigh_eth_addr, 6);
273


-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
  2022-08-24 11:40 Tom Rini
@ 2022-09-08 18:19 ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2022-09-08 18:19 UTC (permalink / raw)
  To: Tom Rini; +Cc: U-Boot Mailing List

Hi Tom,

On Wed, 24 Aug 2022 at 05:40, Tom Rini <trini@konsulko.com> wrote:
>
> And here's the most recent one.
>
> ----- Forwarded message from Tom Rini <tom.rini@gmail.com> -----
>
> Date: Wed, 24 Aug 2022 07:38:55 -0400
> From: Tom Rini <tom.rini@gmail.com>
> To: trini@konsulko.com
> Subject: Fwd: New Defects reported by Coverity Scan for Das U-Boot
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Aug 22, 2022 at 7:07 PM
> 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.
>
> 3 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 2 defect(s), reported by Coverity Scan earlier, were marked fixed in
> the recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 3 of 3 defect(s)
>
>
> ** CID 356244:  Null pointer dereferences  (FORWARD_NULL)
>
>
> ________________________________________________________________________________________________________
> *** CID 356244:  Null pointer dereferences  (FORWARD_NULL)
> /boot/vbe.c: 46 in vbe_find_first_device()
> 40     int vbe_find_first_device(struct udevice **devp)
> 41     {
> 42      uclass_find_first_device(UCLASS_BOOTMETH, devp);
> 43      if (*devp && is_vbe(*devp))
> 44              return 0;
> 45
> >>>     CID 356244:  Null pointer dereferences  (FORWARD_NULL)
> >>>     Passing "devp" to "vbe_find_next_device", which dereferences null "*devp".
> 46      return vbe_find_next_device(devp);
> 47     }
> 48
> 49     int vbe_list(void)
> 50     {
> 51      struct bootstd_priv *std;
>
> ** CID 356243:  Code maintainability issues  (UNUSED_VALUE)
> /boot/vbe_simple.c: 237 in bootmeth_vbe_simple_ft_fixup()
>
>
> ________________________________________________________________________________________________________
> *** CID 356243:  Code maintainability issues  (UNUSED_VALUE)
> /boot/vbe_simple.c: 237 in bootmeth_vbe_simple_ft_fixup()
> 231             /*
> 232              * Ideally we would have driver model support for
> fixups, but that does
> 233              * not exist yet. It is a step too far to try to do
> this before VBE is
> 234              * in place.
> 235              */
> 236             for (ret = vbe_find_first_device(&dev); dev;
> >>>     CID 356243:  Code maintainability issues  (UNUSED_VALUE)
> >>>     Assigning value from "vbe_find_next_device(&dev)" to "ret" here, but that stored value is overwritten before it can be used.
> 237                  ret = vbe_find_next_device(&dev)) {
> 238                     struct simple_state state;
> 239
> 240                     if (strcmp("vbe_simple", dev->driver->name))
> 241                             continue;
> 242
>
> ** CID 356242:    (TAINTED_SCALAR)
>
>
> ________________________________________________________________________________________________________
> *** CID 356242:    (TAINTED_SCALAR)
> /test/dm/ofnode.c: 501 in make_ofnode_fdt()
> 495             ut_assertok(fdt_end_node(fdt));
> 496
> 497             ut_assert(fdt_begin_node(fdt, "new-mmc") >= 0);
> 498             ut_assertok(fdt_end_node(fdt));
> 499
> 500             ut_assertok(fdt_end_node(fdt));
> >>>     CID 356242:    (TAINTED_SCALAR)
> >>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
> 501             ut_assertok(fdt_finish(fdt));
> 502
> 503             return 0;
> 504     }
> 505
> 506     static int dm_test_ofnode_root(struct unit_test_state *uts)
> /test/dm/ofnode.c: 501 in make_ofnode_fdt()
> 495             ut_assertok(fdt_end_node(fdt));
> 496
> 497             ut_assert(fdt_begin_node(fdt, "new-mmc") >= 0);
> 498             ut_assertok(fdt_end_node(fdt));
> 499
> 500             ut_assertok(fdt_end_node(fdt));
> >>>     CID 356242:    (TAINTED_SCALAR)
> >>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
> 501             ut_assertok(fdt_finish(fdt));
> 502
> 503             return 0;
> 504     }
> 505
> 506     static int dm_test_ofnode_root(struct unit_test_state *uts)
>
>
> ________________________________________________________________________________________________________
> To view the defects in Coverity Scan visit,
> https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yoA22WlOQ-2By3ieUvdbKmOyw68TMVT4Kip-2BBzfOGWXJ5yIiYplmPF9KAnKIja4Zd7tU-3Dl_S3_EEm8SbLgSDsaDZif-2Bv7ch8WqhKpLoKErHi4nXpwDNTu-2FviBcJy3TYnkbff9O1lpJB2a065UniCzfVIBu-2Brs6HGPrhp6hp3s-2BQGSVvNSaRsQojbpJAi7kxyFcHZ8aaIeQ0LJlzM2cTXzCCeq8c-2FquCeg4mCmdPzUFdWUhBcgytnExm8LYbWctf-2B-2BcK49gD2uvdO0dVdoZGeFYKdAJZGcKrg-3D-3D
>
>   To manage Coverity Scan email notifications for
> "tom.rini@gmail.com", click
> https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXxWeIHzDeopm-2BEWQ6S6K-2FtUHv9ZTk8qZbuzkkz9sa-2BJFzf226DuRd-2B2ygQlLnerl-2BA3jN1AOYejXZ-2FNZ62waJHedPFGpqqjTx8fawy9KPJBno-3D0xWA_EEm8SbLgSDsaDZif-2Bv7ch8WqhKpLoKErHi4nXpwDNTu-2FviBcJy3TYnkbff9O1lpJ8fYfPhPSMWru8G5g0hjYD2lP6GfXdRYLDay-2BEZMB4nffrqxFwC3P84QsfDGYYlZb-2Fv2AYdsgvSvu2gEihe-2BP8O4Khh9gLeVsBYy-2Bps2buInswpEo43c-2B1-2FHNkYpmMXLe6-2FNHIyvt0clj7kDSbeyOqA-3D-3D
>

OK I have it on my list...currently trying to get the VBE stuff
finished for osfc in 10 days.

Regards,
Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
@ 2022-08-24 11:40 Tom Rini
  2022-09-08 18:19 ` Simon Glass
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2022-08-24 11:40 UTC (permalink / raw)
  To: u-boot, Simon Glass

[-- Attachment #1: Type: text/plain, Size: 5206 bytes --]

And here's the most recent one.

----- Forwarded message from Tom Rini <tom.rini@gmail.com> -----

Date: Wed, 24 Aug 2022 07:38:55 -0400
From: Tom Rini <tom.rini@gmail.com>
To: trini@konsulko.com
Subject: Fwd: New Defects reported by Coverity Scan for Das U-Boot

---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Aug 22, 2022 at 7:07 PM
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.

3 new defect(s) introduced to Das U-Boot found with Coverity Scan.
2 defect(s), reported by Coverity Scan earlier, were marked fixed in
the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 3 of 3 defect(s)


** CID 356244:  Null pointer dereferences  (FORWARD_NULL)


________________________________________________________________________________________________________
*** CID 356244:  Null pointer dereferences  (FORWARD_NULL)
/boot/vbe.c: 46 in vbe_find_first_device()
40     int vbe_find_first_device(struct udevice **devp)
41     {
42      uclass_find_first_device(UCLASS_BOOTMETH, devp);
43      if (*devp && is_vbe(*devp))
44              return 0;
45
>>>     CID 356244:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing "devp" to "vbe_find_next_device", which dereferences null "*devp".
46      return vbe_find_next_device(devp);
47     }
48
49     int vbe_list(void)
50     {
51      struct bootstd_priv *std;

** CID 356243:  Code maintainability issues  (UNUSED_VALUE)
/boot/vbe_simple.c: 237 in bootmeth_vbe_simple_ft_fixup()


________________________________________________________________________________________________________
*** CID 356243:  Code maintainability issues  (UNUSED_VALUE)
/boot/vbe_simple.c: 237 in bootmeth_vbe_simple_ft_fixup()
231             /*
232              * Ideally we would have driver model support for
fixups, but that does
233              * not exist yet. It is a step too far to try to do
this before VBE is
234              * in place.
235              */
236             for (ret = vbe_find_first_device(&dev); dev;
>>>     CID 356243:  Code maintainability issues  (UNUSED_VALUE)
>>>     Assigning value from "vbe_find_next_device(&dev)" to "ret" here, but that stored value is overwritten before it can be used.
237                  ret = vbe_find_next_device(&dev)) {
238                     struct simple_state state;
239
240                     if (strcmp("vbe_simple", dev->driver->name))
241                             continue;
242

** CID 356242:    (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 356242:    (TAINTED_SCALAR)
/test/dm/ofnode.c: 501 in make_ofnode_fdt()
495             ut_assertok(fdt_end_node(fdt));
496
497             ut_assert(fdt_begin_node(fdt, "new-mmc") >= 0);
498             ut_assertok(fdt_end_node(fdt));
499
500             ut_assertok(fdt_end_node(fdt));
>>>     CID 356242:    (TAINTED_SCALAR)
>>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
501             ut_assertok(fdt_finish(fdt));
502
503             return 0;
504     }
505
506     static int dm_test_ofnode_root(struct unit_test_state *uts)
/test/dm/ofnode.c: 501 in make_ofnode_fdt()
495             ut_assertok(fdt_end_node(fdt));
496
497             ut_assert(fdt_begin_node(fdt, "new-mmc") >= 0);
498             ut_assertok(fdt_end_node(fdt));
499
500             ut_assertok(fdt_end_node(fdt));
>>>     CID 356242:    (TAINTED_SCALAR)
>>>     Passing tainted expression "fdt->size_dt_strings" to "fdt_finish", which uses it as an offset.
501             ut_assertok(fdt_finish(fdt));
502
503             return 0;
504     }
505
506     static int dm_test_ofnode_root(struct unit_test_state *uts)


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit,
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yoA22WlOQ-2By3ieUvdbKmOyw68TMVT4Kip-2BBzfOGWXJ5yIiYplmPF9KAnKIja4Zd7tU-3Dl_S3_EEm8SbLgSDsaDZif-2Bv7ch8WqhKpLoKErHi4nXpwDNTu-2FviBcJy3TYnkbff9O1lpJB2a065UniCzfVIBu-2Brs6HGPrhp6hp3s-2BQGSVvNSaRsQojbpJAi7kxyFcHZ8aaIeQ0LJlzM2cTXzCCeq8c-2FquCeg4mCmdPzUFdWUhBcgytnExm8LYbWctf-2B-2BcK49gD2uvdO0dVdoZGeFYKdAJZGcKrg-3D-3D

  To manage Coverity Scan email notifications for
"tom.rini@gmail.com", click
https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXxWeIHzDeopm-2BEWQ6S6K-2FtUHv9ZTk8qZbuzkkz9sa-2BJFzf226DuRd-2B2ygQlLnerl-2BA3jN1AOYejXZ-2FNZ62waJHedPFGpqqjTx8fawy9KPJBno-3D0xWA_EEm8SbLgSDsaDZif-2Bv7ch8WqhKpLoKErHi4nXpwDNTu-2FviBcJy3TYnkbff9O1lpJ8fYfPhPSMWru8G5g0hjYD2lP6GfXdRYLDay-2BEZMB4nffrqxFwC3P84QsfDGYYlZb-2Fv2AYdsgvSvu2gEihe-2BP8O4Khh9gLeVsBYy-2Bps2buInswpEo43c-2B1-2FHNkYpmMXLe6-2FNHIyvt0clj7kDSbeyOqA-3D-3D



-- 
Tom

----- End forwarded message -----

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-12-21 16:08 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-24 11:40 [tom.rini@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot] Tom Rini
2022-08-24 11:40 Tom Rini
2022-09-08 18:19 ` Simon Glass
2022-12-06 14:51 Tom Rini
2023-01-31 15:02 Tom Rini
2023-03-27 19:19 Tom Rini
2023-05-29 20:04 Tom Rini
2023-07-27 17:38 Tom Rini
2023-10-24  1:18 Tom Rini
2023-10-24 15:05 ` Sughosh Ganu
2023-10-24 18:05   ` Tom Rini
2023-11-06 20:27 Tom Rini
2023-11-07 11:18 ` Ilias Apalodimas
2023-11-07 23:18 ` Johan Jonker
2023-11-08 13:54   ` Tom Rini
2023-11-08  3:24 ` Alexander Gendin
2023-11-08 14:29   ` Tom Rini
2023-12-18 16:26 Tom Rini
2023-12-21 14:48 ` Shantur Rathore
2023-12-21 16:08   ` Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).