| Message ID | 20250429120232.3874362-1-jerome.forissier@linaro.org |
|---|---|
| State | Accepted |
| Commit | 0352eab7d373664c1c01b23154025b83ec0eadca |
| Headers | show |
| Series | cmd: spawn: reject 0 as an invalid job ID | expand |
On Tue, 29 Apr 2025 14:02:18 +0200, Jerome Forissier wrote: > Job IDs are positive integers greater than 1. 0 is not a valid job ID, > therefore fix the comparison in do_wait(). > > Fixes Coverity defects: > > *** CID 550296: Control flow issues (NO_EFFECT) > /cmd/spawn.c: 172 in do_wait() > 166 for (i = 0; i < CONFIG_CMD_SPAWN_NUM_JOBS; i++) > 167 if (job[i]) > 168 ret = wait_job(i); > 169 } else { > 170 for (i = 1; i < argc; i++) { > 171 id = dectoul(argv[i], NULL); > >>> CID 550296: Control flow issues (NO_EFFECT) > >>> This less-than-zero comparison of an unsigned value is never true. > "id < 0UL". > 172 if (id < 0 || id > > CONFIG_CMD_SPAWN_NUM_JOBS) > 173 return CMD_RET_USAGE; > 174 idx = (int)id - 1; > 175 ret = wait_job(idx); > 176 } > 177 } > > [...] Applied to u-boot/master, thanks! [1/1] cmd: spawn: reject 0 as an invalid job ID commit: 0352eab7d373664c1c01b23154025b83ec0eadca
diff --git a/cmd/spawn.c b/cmd/spawn.c index eddbcb792b3..37737b8627c 100644 --- a/cmd/spawn.c +++ b/cmd/spawn.c @@ -169,7 +169,7 @@ static int do_wait(struct cmd_tbl *cmdtp, int flag, int argc, } else { for (i = 1; i < argc; i++) { id = dectoul(argv[i], NULL); - if (id < 0 || id > CONFIG_CMD_SPAWN_NUM_JOBS) + if (id < 1 || id > CONFIG_CMD_SPAWN_NUM_JOBS) return CMD_RET_USAGE; idx = (int)id - 1; ret = wait_job(idx);
Job IDs are positive integers greater than 1. 0 is not a valid job ID, therefore fix the comparison in do_wait(). Fixes Coverity defects: *** CID 550296: Control flow issues (NO_EFFECT) /cmd/spawn.c: 172 in do_wait() 166 for (i = 0; i < CONFIG_CMD_SPAWN_NUM_JOBS; i++) 167 if (job[i]) 168 ret = wait_job(i); 169 } else { 170 for (i = 1; i < argc; i++) { 171 id = dectoul(argv[i], NULL); >>> CID 550296: Control flow issues (NO_EFFECT) >>> This less-than-zero comparison of an unsigned value is never true. "id < 0UL". 172 if (id < 0 || id > CONFIG_CMD_SPAWN_NUM_JOBS) 173 return CMD_RET_USAGE; 174 idx = (int)id - 1; 175 ret = wait_job(idx); 176 } 177 } *** CID 550297: Integer handling issues (INTEGER_OVERFLOW) /cmd/spawn.c: 174 in do_wait() 168 ret = wait_job(i); 169 } else { 170 for (i = 1; i < argc; i++) { 171 id = dectoul(argv[i], NULL); 172 if (id < 0 || id > CONFIG_CMD_SPAWN_NUM_JOBS) 173 return CMD_RET_USAGE; >>> CID 550297: Integer handling issues (INTEGER_OVERFLOW) >>> Expression "idx", where "(int)id - 1" is known to be equal to -1, overflows the type of "idx", which is type "unsigned int". 174 idx = (int)id - 1; 175 ret = wait_job(idx); 176 } 177 } Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> CC: Tom Rini <trini@konsulko.com> --- cmd/spawn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)