Dear Alan, dear Greg,
here is a quick report from the static analysis tool clang-analyzer on
./drivers/usb/storage/transport.c:
When usb_stor_bulk_transfer_sglist() returns with USB_STOR_XFER_ERROR, it
returns without writing to its parameter *act_len.
Further, the two callers of usb_stor_bulk_transfer_sglist():
usb_stor_bulk_srb() and
usb_stor_bulk_transfer_sg(),
use the passed variable partial without checking the return value. Hence,
the uninitialized value of partial is then used in the further execution
of those two functions.
Clang-analyzer detects this potential control and data flow and warns:
drivers/usb/storage/transport.c:469:40: warning: The right operand of '-'
is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
scsi_set_resid(srb, scsi_bufflen(srb) - partial);
^
drivers/usb/storage/transport.c:495:15: warning: Assigned value is garbage
or undefined [clang-analyzer-core.uninitialized.Assign]
length_left -= partial;
^
The tool is right; unfortunately, I do not know anything about the
intended function here. What is the further operation of those two
functions supposed to be when USB_STOR_XFER_ERROR is returned from
usb_stor_bulk_transfer_sglist()? Should the passed arguments remain
untouched, so setting *act_len to zero for the error paths would be
a suitable fix to achieve that.
A quick hint on that point and I can prepare a patch for you to pick up...
Given that this code is pretty stable for years and probably in wider
use, the overall functionality is probably resilient to having this local
data being filled with arbitrary undefined data in the error case... but
who knows...
Thanks and best regards,
Lukas
