Hello Dave Penkler,
Commit 9dde4559e939 ("staging: gpib: Add GPIB common core driver")
from Sep 18, 2024 (linux-next), leads to the following Smatch static
checker warning:
drivers/staging/gpib/common/gpib_os.c:541 dvrsp()
warn: no lower bound on 'sad' rl='s32min-30'
drivers/staging/gpib/common/gpib_os.c
525 int dvrsp(gpib_board_t *board, unsigned int pad, int sad,
526 unsigned int usec_timeout, uint8_t *result)
527 {
528 int status = ibstatus(board);
529 int retval;
530
531 if ((status & CIC) == 0) {
532 pr_err("gpib: not CIC during serial poll\n");
533 return -1;
534 }
535
536 if (pad > gpib_addr_max || sad > gpib_addr_max) {
sad comes from the user in serial_poll_ioctl() and it can be any int.
537 pr_err("gpib: bad address for serial poll");
538 return -1;
539 }
540
--> 541 retval = serial_poll_single(board, pad, sad, usec_timeout, result);
Passing a negative doesn't do anything harmful and it looks like probably it's
intentional to mean there is no "sad". But instead of saying that "any negative
is valid" it would be better to say that "-1" means there is no sad.
if (pad > gpib_addr_max ||
(sad < -1 || sad > gpib_addr_max)) {
...
Presumably the user space code is already passing -1 as the no sad value but I
don't maybe it's passing INT_MIN or something weird. This is an API change.
We're allowed to change the API so long as we don't break userspace. In
staging, we have a bit more leeway to break userspace as well honestly.
542 if (io_timed_out(board))
543 retval = -ETIMEDOUT;
544
545 return retval;
546 }
regards,
dan carpenter
