* [PATCH] scsi: myrs: avoid stack overflow warning
@ 2018-11-02 15:44 Arnd Bergmann
2018-11-03 8:47 ` Hannes Reinecke
2018-11-06 3:36 ` Martin K. Petersen
0 siblings, 2 replies; 3+ messages in thread
From: Arnd Bergmann @ 2018-11-02 15:44 UTC (permalink / raw)
To: Hannes Reinecke, James E.J. Bottomley, Martin K. Petersen
Cc: Arnd Bergmann, James Bottomley, linux-scsi, linux-kernel
Putting a 1024 byte data structure on the stack is generally a bad idea.
On 32-bit systems, it also triggers a compile-time warning when building
with -Og:
drivers/scsi/myrs.c: In function 'myrs_get_ctlr_info':
drivers/scsi/myrs.c:212:1: error: the frame size of 1028 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
We only really need three members of the structure, so just read them
manually here instead of copying the entire structure.
Fixes: 77266186397c ("scsi: myrs: Add Mylex RAID controller (SCSI interface)")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/scsi/myrs.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c
index 0264a2e2bc19..b8d54ef8cf6d 100644
--- a/drivers/scsi/myrs.c
+++ b/drivers/scsi/myrs.c
@@ -163,9 +163,12 @@ static unsigned char myrs_get_ctlr_info(struct myrs_hba *cs)
dma_addr_t ctlr_info_addr;
union myrs_sgl *sgl;
unsigned char status;
- struct myrs_ctlr_info old;
+ unsigned short ldev_present, ldev_critical, ldev_offline;
+
+ ldev_present = cs->ctlr_info->ldev_present;
+ ldev_critical = cs->ctlr_info->ldev_critical;
+ ldev_offline = cs->ctlr_info->ldev_offline;
- memcpy(&old, cs->ctlr_info, sizeof(struct myrs_ctlr_info));
ctlr_info_addr = dma_map_single(&cs->pdev->dev, cs->ctlr_info,
sizeof(struct myrs_ctlr_info),
DMA_FROM_DEVICE);
@@ -198,9 +201,9 @@ static unsigned char myrs_get_ctlr_info(struct myrs_hba *cs)
cs->ctlr_info->rbld_active +
cs->ctlr_info->exp_active != 0)
cs->needs_update = true;
- if (cs->ctlr_info->ldev_present != old.ldev_present ||
- cs->ctlr_info->ldev_critical != old.ldev_critical ||
- cs->ctlr_info->ldev_offline != old.ldev_offline)
+ if (cs->ctlr_info->ldev_present != ldev_present ||
+ cs->ctlr_info->ldev_critical != ldev_critical ||
+ cs->ctlr_info->ldev_offline != ldev_offline)
shost_printk(KERN_INFO, cs->host,
"Logical drive count changes (%d/%d/%d)\n",
cs->ctlr_info->ldev_critical,
--
2.18.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] scsi: myrs: avoid stack overflow warning
2018-11-02 15:44 [PATCH] scsi: myrs: avoid stack overflow warning Arnd Bergmann
@ 2018-11-03 8:47 ` Hannes Reinecke
2018-11-06 3:36 ` Martin K. Petersen
1 sibling, 0 replies; 3+ messages in thread
From: Hannes Reinecke @ 2018-11-03 8:47 UTC (permalink / raw)
To: Arnd Bergmann, Hannes Reinecke, James E.J. Bottomley, Martin K. Petersen
Cc: James Bottomley, linux-scsi, linux-kernel
On 11/2/18 4:44 PM, Arnd Bergmann wrote:
> Putting a 1024 byte data structure on the stack is generally a bad idea.
> On 32-bit systems, it also triggers a compile-time warning when building
> with -Og:
>
> drivers/scsi/myrs.c: In function 'myrs_get_ctlr_info':
> drivers/scsi/myrs.c:212:1: error: the frame size of 1028 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> We only really need three members of the structure, so just read them
> manually here instead of copying the entire structure.
>
> Fixes: 77266186397c ("scsi: myrs: Add Mylex RAID controller (SCSI interface)")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/scsi/myrs.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c
> index 0264a2e2bc19..b8d54ef8cf6d 100644
> --- a/drivers/scsi/myrs.c
> +++ b/drivers/scsi/myrs.c
> @@ -163,9 +163,12 @@ static unsigned char myrs_get_ctlr_info(struct myrs_hba *cs)
> dma_addr_t ctlr_info_addr;
> union myrs_sgl *sgl;
> unsigned char status;
> - struct myrs_ctlr_info old;
> + unsigned short ldev_present, ldev_critical, ldev_offline;
> +
> + ldev_present = cs->ctlr_info->ldev_present;
> + ldev_critical = cs->ctlr_info->ldev_critical;
> + ldev_offline = cs->ctlr_info->ldev_offline;
>
> - memcpy(&old, cs->ctlr_info, sizeof(struct myrs_ctlr_info));
> ctlr_info_addr = dma_map_single(&cs->pdev->dev, cs->ctlr_info,
> sizeof(struct myrs_ctlr_info),
> DMA_FROM_DEVICE);
> @@ -198,9 +201,9 @@ static unsigned char myrs_get_ctlr_info(struct myrs_hba *cs)
> cs->ctlr_info->rbld_active +
> cs->ctlr_info->exp_active != 0)
> cs->needs_update = true;
> - if (cs->ctlr_info->ldev_present != old.ldev_present ||
> - cs->ctlr_info->ldev_critical != old.ldev_critical ||
> - cs->ctlr_info->ldev_offline != old.ldev_offline)
> + if (cs->ctlr_info->ldev_present != ldev_present ||
> + cs->ctlr_info->ldev_critical != ldev_critical ||
> + cs->ctlr_info->ldev_offline != ldev_offline)
> shost_printk(KERN_INFO, cs->host,
> "Logical drive count changes (%d/%d/%d)\n",
> cs->ctlr_info->ldev_critical,
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] scsi: myrs: avoid stack overflow warning
2018-11-02 15:44 [PATCH] scsi: myrs: avoid stack overflow warning Arnd Bergmann
2018-11-03 8:47 ` Hannes Reinecke
@ 2018-11-06 3:36 ` Martin K. Petersen
1 sibling, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2018-11-06 3:36 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Hannes Reinecke, James E.J. Bottomley, Martin K. Petersen,
James Bottomley, linux-scsi, linux-kernel
Arnd,
> Putting a 1024 byte data structure on the stack is generally a bad idea.
> On 32-bit systems, it also triggers a compile-time warning when building
> with -Og:
Applied to 4.20/scsi-fixes, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-06 3:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-02 15:44 [PATCH] scsi: myrs: avoid stack overflow warning Arnd Bergmann
2018-11-03 8:47 ` Hannes Reinecke
2018-11-06 3:36 ` Martin K. Petersen
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).