* [PATCH][next] scsi: storvsc: Use struct_size() helper in storvsc_queuecommand()
@ 2022-01-25 19:42 Gustavo A. R. Silva
2022-01-27 15:57 ` Michael Kelley (LINUX)
0 siblings, 1 reply; 2+ messages in thread
From: Gustavo A. R. Silva @ 2022-01-25 19:42 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, James E.J. Bottomley, Martin K. Petersen
Cc: linux-hyperv, linux-scsi, linux-kernel, Gustavo A. R. Silva,
linux-hardening
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;
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* RE: [PATCH][next] scsi: storvsc: Use struct_size() helper in storvsc_queuecommand()
2022-01-25 19:42 [PATCH][next] scsi: storvsc: Use struct_size() helper in storvsc_queuecommand() Gustavo A. R. Silva
@ 2022-01-27 15:57 ` Michael Kelley (LINUX)
0 siblings, 0 replies; 2+ messages in thread
From: Michael Kelley (LINUX) @ 2022-01-27 15:57 UTC (permalink / raw)
To: Gustavo A. R. Silva, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, Dexuan Cui, James E.J. Bottomley,
Martin K. Petersen
Cc: linux-hyperv, linux-scsi, linux-kernel, linux-hardening
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-01-27 15:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-25 19:42 [PATCH][next] scsi: storvsc: Use struct_size() helper in storvsc_queuecommand() Gustavo A. R. Silva
2022-01-27 15:57 ` Michael Kelley (LINUX)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).