lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 27 Jan 2022 15:57:39 +0000 From: "Michael Kelley (LINUX)" <mikelley@...rosoft.com> To: "Gustavo A. R. Silva" <gustavoars@...nel.org>, KY Srinivasan <kys@...rosoft.com>, Haiyang Zhang <haiyangz@...rosoft.com>, Stephen Hemminger <sthemmin@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>, "James E.J. Bottomley" <jejb@...ux.ibm.com>, "Martin K. Petersen" <martin.petersen@...cle.com> CC: "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>, "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org> Subject: RE: [PATCH][next] scsi: storvsc: Use struct_size() helper in storvsc_queuecommand() From: Gustavo A. R. Silva <gustavoars@...nel.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@...nel.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
Powered by blists - more mailing lists