| Message ID | 20220125194213.GA74670@embeddedor |
|---|---|
| State | New |
| Headers | show |
| Series | [next] scsi: storvsc: Use struct_size() helper in storvsc_queuecommand() | expand |
From: Gustavo A. R. Silva <gustavoars@kernel.org> Sent: Tuesday, January 25, 2022 11:42 AM > > Make use of the struct_size() helper instead of an open-coded version, > in order to avoid any potential type mistakes or integer overflows that, > in the worst scenario, could lead to heap overflows. > > Also, address the following sparse warnings: > drivers/scsi/storvsc_drv.c:1843:39: warning: using sizeof on a flexible structure > > Link: https://github.com/KSPP/linux/issues/174 > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > drivers/scsi/storvsc_drv.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > index 9a0bba5a51a7..89c20dfc6609 100644 > --- a/drivers/scsi/storvsc_drv.c > +++ b/drivers/scsi/storvsc_drv.c > @@ -1755,7 +1755,7 @@ static int storvsc_queuecommand(struct Scsi_Host *host, > struct scsi_cmnd *scmnd) > struct scatterlist *sgl; > struct vmscsi_request *vm_srb; > struct vmbus_packet_mpb_array *payload; > - u32 payload_sz; > + size_t payload_sz; Does changing this variable to type size_t accomplish anything? It will be stored into cmd_request->payload_sz, which is still a u32. The code in storvsc_do_io() and storvsc_command_completion() will process it as a 32 bit value. > u32 length; > > if (vmstor_proto_version <= VMSTOR_PROTO_VERSION_WIN8) { > @@ -1839,8 +1839,8 @@ static int storvsc_queuecommand(struct Scsi_Host *host, > struct scsi_cmnd *scmnd) > > if (hvpg_count > MAX_PAGE_BUFFER_COUNT) { > > - payload_sz = (hvpg_count * sizeof(u64) + > - sizeof(struct vmbus_packet_mpb_array)); > + payload_sz = struct_size(payload, range.pfn_array, > + hvpg_count); > payload = kzalloc(payload_sz, GFP_ATOMIC); > if (!payload) > return SCSI_MLQUEUE_DEVICE_BUSY; > -- > 2.27.0 Overall this is an incremental improvement. But the code is still implicitly dependent on scsi_bufflen() returning a 32-bit value into the local variable "length", and hence hvpg_count will always produce a payload_sz that fits in 32-bits. I would be OK with taking this, minus the change to the type of the local variable payload_sz. Michael
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 9a0bba5a51a7..89c20dfc6609 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -1755,7 +1755,7 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd) struct scatterlist *sgl; struct vmscsi_request *vm_srb; struct vmbus_packet_mpb_array *payload; - u32 payload_sz; + size_t payload_sz; u32 length; if (vmstor_proto_version <= VMSTOR_PROTO_VERSION_WIN8) { @@ -1839,8 +1839,8 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd) if (hvpg_count > MAX_PAGE_BUFFER_COUNT) { - payload_sz = (hvpg_count * sizeof(u64) + - sizeof(struct vmbus_packet_mpb_array)); + payload_sz = struct_size(payload, range.pfn_array, + hvpg_count); payload = kzalloc(payload_sz, GFP_ATOMIC); if (!payload) return SCSI_MLQUEUE_DEVICE_BUSY;
Make use of the struct_size() helper instead of an open-coded version, in order to avoid any potential type mistakes or integer overflows that, in the worst scenario, could lead to heap overflows. Also, address the following sparse warnings: drivers/scsi/storvsc_drv.c:1843:39: warning: using sizeof on a flexible structure Link: https://github.com/KSPP/linux/issues/174 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/scsi/storvsc_drv.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)