From: Xiaohui Zhang <[email protected]> Sent: Tuesday, December 8, 2020 5:19 AM > > From: Zhang Xiaohui <[email protected]> > > storvsc_queuecommand() calls memcpy() without checking > the destination size may trigger a buffer overflower, > which a local user could use to cause denial of service > or the execution of arbitrary code. > Fix it by putting the length check before calling memcpy(). > > Signed-off-by: Zhang Xiaohui <[email protected]> > --- > drivers/scsi/storvsc_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > index 0c65fbd41..09b60a4c0 100644 > --- a/drivers/scsi/storvsc_drv.c > +++ b/drivers/scsi/storvsc_drv.c > @@ -1729,6 +1729,8 @@ static int storvsc_queuecommand(struct Scsi_Host *host, > struct > scsi_cmnd *scmnd) > > vm_srb->cdb_length = scmnd->cmd_len; > > + if (vm_srb->cdb_length > STORVSC_MAX_CMD_LEN) > + vm_srb->cdb_length = STORVSC_MAX_CMD_LEN; > memcpy(vm_srb->cdb, scmnd->cmnd, vm_srb->cdb_length); > > sgl = (struct scatterlist *)scsi_sglist(scmnd); > -- > 2.17.1
At first glance, this new test isn't necessary. storvsc_queuecommand() gets called from scsi_dispatch_cmd(), where just before the queuecommand function is called, the cmd_len field is checked against the maximum command length defined for the SCSI controller. In the case of storvsc, that maximum command length is STORVSC_MAX_CMD_LEN as set in storvsc_probe(). There's a comment in scsi_dispatch_cmd() that covers this exact case. You are correct that we need to make sure there's no buffer overflow. Are you seeing any other path where storvsc_queuecommand() could be called without the cmd_len being checked? Michael
