* [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows
@ 2021-05-28 18:13 Kees Cook
2021-05-28 18:13 ` [PATCH 1/3] scsi: fcoe: Statically initialize flogi_maddr Kees Cook
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Kees Cook @ 2021-05-28 18:13 UTC (permalink / raw)
To: Martin K . Petersen
Cc: Kees Cook, Hannes Reinecke, James E.J. Bottomley,
Gustavo A. R. Silva, Bradley Grove, Artur Paszkiewicz,
linux-kernel, linux-scsi, linux-hardening
While working on improving FORTIFY_SOURCE's memcpy() coverage, there are
a few fixes that don't require any helper changes, etc.
-Kees
Kees Cook (3):
scsi: fcoe: Statically initialize flogi_maddr
scsi: esas2r: Switch to flexible array member
scsi: isci: Use correctly sized target buffer for memcpy()
drivers/scsi/esas2r/atioctl.h | 2 +-
drivers/scsi/fcoe/fcoe.c | 6 ++----
drivers/scsi/isci/task.c | 4 ++--
3 files changed, 5 insertions(+), 7 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] scsi: fcoe: Statically initialize flogi_maddr
2021-05-28 18:13 [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows Kees Cook
@ 2021-05-28 18:13 ` Kees Cook
2021-05-28 18:28 ` Joe Perches
2021-05-28 18:13 ` [PATCH 2/3] scsi: esas2r: Switch to flexible array member Kees Cook
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2021-05-28 18:13 UTC (permalink / raw)
To: Martin K . Petersen
Cc: Kees Cook, Hannes Reinecke, James E.J. Bottomley,
Gustavo A. R. Silva, Bradley Grove, Artur Paszkiewicz,
linux-kernel, linux-scsi, linux-hardening
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy() using memcpy() with an inline const
buffer and instead just statically initialize the destination array
directly.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/scsi/fcoe/fcoe.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 89ec735929c3..8991990e6639 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -293,7 +293,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
struct fcoe_ctlr *fip = fcoe_to_ctlr(fcoe);
struct netdev_hw_addr *ha;
struct net_device *real_dev;
- u8 flogi_maddr[ETH_ALEN];
+ u8 flogi_maddr[ETH_ALEN] = FC_FCOE_FLOGI_MAC;
const struct net_device_ops *ops;
fcoe->netdev = netdev;
@@ -336,7 +336,6 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
* or enter promiscuous mode if not capable of listening
* for multiple unicast MACs.
*/
- memcpy(flogi_maddr, (u8[6]) FC_FCOE_FLOGI_MAC, ETH_ALEN);
dev_uc_add(netdev, flogi_maddr);
if (fip->spma)
dev_uc_add(netdev, fip->ctl_src_addr);
@@ -442,7 +441,7 @@ static void fcoe_interface_remove(struct fcoe_interface *fcoe)
{
struct net_device *netdev = fcoe->netdev;
struct fcoe_ctlr *fip = fcoe_to_ctlr(fcoe);
- u8 flogi_maddr[ETH_ALEN];
+ u8 flogi_maddr[ETH_ALEN] = FC_FCOE_FLOGI_MAC;
const struct net_device_ops *ops;
/*
@@ -458,7 +457,6 @@ static void fcoe_interface_remove(struct fcoe_interface *fcoe)
synchronize_net();
/* Delete secondary MAC addresses */
- memcpy(flogi_maddr, (u8[6]) FC_FCOE_FLOGI_MAC, ETH_ALEN);
dev_uc_del(netdev, flogi_maddr);
if (fip->spma)
dev_uc_del(netdev, fip->ctl_src_addr);
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] scsi: esas2r: Switch to flexible array member
2021-05-28 18:13 [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows Kees Cook
2021-05-28 18:13 ` [PATCH 1/3] scsi: fcoe: Statically initialize flogi_maddr Kees Cook
@ 2021-05-28 18:13 ` Kees Cook
2021-05-28 20:26 ` Gustavo A. R. Silva
2021-05-28 18:13 ` [PATCH 3/3] scsi: isci: Use correctly sized target buffer for memcpy() Kees Cook
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2021-05-28 18:13 UTC (permalink / raw)
To: Martin K . Petersen
Cc: Kees Cook, Hannes Reinecke, James E.J. Bottomley,
Gustavo A. R. Silva, Bradley Grove, Artur Paszkiewicz,
linux-kernel, linux-scsi, linux-hardening
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), avoid intentionally writing across
neighboring array fields.
Remove old-style 1-byte array in favor of a flexible array[1] to avoid
future false-positive cross-field memcpy() warning in:
esas2r_vda.c:
memcpy(vi->cmd.gsv.version_info, esas2r_vdaioctl_versions, ...)
The change in struct size doesn't change other structure sizes (it is
already maxed out to 256 bytes, for example here:
union {
struct atto_ioctl_vda_scsi_cmd scsi;
struct atto_ioctl_vda_flash_cmd flash;
struct atto_ioctl_vda_diag_cmd diag;
struct atto_ioctl_vda_cli_cmd cli;
struct atto_ioctl_vda_smp_cmd smp;
struct atto_ioctl_vda_cfg_cmd cfg;
struct atto_ioctl_vda_mgt_cmd mgt;
struct atto_ioctl_vda_gsv_cmd gsv;
u8 cmd_info[256];
} cmd;
No sizes are calculated using the enclosing structure, so no other
updates are needed.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/scsi/esas2r/atioctl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/esas2r/atioctl.h b/drivers/scsi/esas2r/atioctl.h
index 4aca3d52c851..ff2ad9b38575 100644
--- a/drivers/scsi/esas2r/atioctl.h
+++ b/drivers/scsi/esas2r/atioctl.h
@@ -1141,7 +1141,7 @@ struct __packed atto_ioctl_vda_gsv_cmd {
u8 rsp_len;
u8 reserved[7];
- u8 version_info[1];
+ u8 version_info[];
#define ATTO_VDA_VER_UNSUPPORTED 0xFF
};
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] scsi: isci: Use correctly sized target buffer for memcpy()
2021-05-28 18:13 [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows Kees Cook
2021-05-28 18:13 ` [PATCH 1/3] scsi: fcoe: Statically initialize flogi_maddr Kees Cook
2021-05-28 18:13 ` [PATCH 2/3] scsi: esas2r: Switch to flexible array member Kees Cook
@ 2021-05-28 18:13 ` Kees Cook
2021-05-28 20:29 ` Gustavo A. R. Silva
2021-06-02 3:38 ` [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows Martin K. Petersen
2021-06-08 3:05 ` Martin K. Petersen
4 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2021-05-28 18:13 UTC (permalink / raw)
To: Martin K . Petersen
Cc: Kees Cook, Hannes Reinecke, James E.J. Bottomley,
Gustavo A. R. Silva, Bradley Grove, Artur Paszkiewicz,
linux-kernel, linux-scsi, linux-hardening
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), avoid intentionally writing across
neighboring array fields.
Switch from rsp_ui to resp_buf, since resp_ui isn't SSP_RESP_IU_MAX_SIZE
bytes in length. This avoids future compile-time warnings.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/scsi/isci/task.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 62062ed6cd9a..eeaec26ac324 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -709,8 +709,8 @@ isci_task_request_complete(struct isci_host *ihost,
tmf->status = completion_status;
if (tmf->proto == SAS_PROTOCOL_SSP) {
- memcpy(&tmf->resp.resp_iu,
- &ireq->ssp.rsp,
+ memcpy(tmf->resp.rsp_buf,
+ ireq->ssp.rsp_buf,
SSP_RESP_IU_MAX_SIZE);
} else if (tmf->proto == SAS_PROTOCOL_SATA) {
memcpy(&tmf->resp.d2h_fis,
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] scsi: fcoe: Statically initialize flogi_maddr
2021-05-28 18:13 ` [PATCH 1/3] scsi: fcoe: Statically initialize flogi_maddr Kees Cook
@ 2021-05-28 18:28 ` Joe Perches
2021-05-28 18:29 ` Kees Cook
0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2021-05-28 18:28 UTC (permalink / raw)
To: Kees Cook, Martin K . Petersen
Cc: Hannes Reinecke, James E.J. Bottomley, Gustavo A. R. Silva,
Bradley Grove, Artur Paszkiewicz, linux-kernel, linux-scsi,
linux-hardening
On Fri, 2021-05-28 at 11:13 -0700, Kees Cook wrote:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memcpy() using memcpy() with an inline const
> buffer and instead just statically initialize the destination array
> directly.
[]
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
[]
> @@ -293,7 +293,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
> struct fcoe_ctlr *fip = fcoe_to_ctlr(fcoe);
> struct netdev_hw_addr *ha;
> struct net_device *real_dev;
> - u8 flogi_maddr[ETH_ALEN];
> + u8 flogi_maddr[ETH_ALEN] = FC_FCOE_FLOGI_MAC;
static const
> @@ -442,7 +441,7 @@ static void fcoe_interface_remove(struct fcoe_interface *fcoe)
> {
> struct net_device *netdev = fcoe->netdev;
> struct fcoe_ctlr *fip = fcoe_to_ctlr(fcoe);
> - u8 flogi_maddr[ETH_ALEN];
> + u8 flogi_maddr[ETH_ALEN] = FC_FCOE_FLOGI_MAC;
etc...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] scsi: fcoe: Statically initialize flogi_maddr
2021-05-28 18:28 ` Joe Perches
@ 2021-05-28 18:29 ` Kees Cook
0 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2021-05-28 18:29 UTC (permalink / raw)
To: Joe Perches
Cc: Martin K . Petersen, Hannes Reinecke, James E.J. Bottomley,
Gustavo A. R. Silva, Bradley Grove, Artur Paszkiewicz,
linux-kernel, linux-scsi, linux-hardening
On Fri, May 28, 2021 at 11:28:59AM -0700, Joe Perches wrote:
> On Fri, 2021-05-28 at 11:13 -0700, Kees Cook wrote:
> > In preparation for FORTIFY_SOURCE performing compile-time and run-time
> > field bounds checking for memcpy() using memcpy() with an inline const
> > buffer and instead just statically initialize the destination array
> > directly.
> []
> > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> []
> > @@ -293,7 +293,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
> > struct fcoe_ctlr *fip = fcoe_to_ctlr(fcoe);
> > struct netdev_hw_addr *ha;
> > struct net_device *real_dev;
> > - u8 flogi_maddr[ETH_ALEN];
> > + u8 flogi_maddr[ETH_ALEN] = FC_FCOE_FLOGI_MAC;
>
> static const
>
> > @@ -442,7 +441,7 @@ static void fcoe_interface_remove(struct fcoe_interface *fcoe)
> > {
> > struct net_device *netdev = fcoe->netdev;
> > struct fcoe_ctlr *fip = fcoe_to_ctlr(fcoe);
> > - u8 flogi_maddr[ETH_ALEN];
> > + u8 flogi_maddr[ETH_ALEN] = FC_FCOE_FLOGI_MAC;
>
> etc...
Hm, good point.
--
Kees Cook
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] scsi: esas2r: Switch to flexible array member
2021-05-28 18:13 ` [PATCH 2/3] scsi: esas2r: Switch to flexible array member Kees Cook
@ 2021-05-28 20:26 ` Gustavo A. R. Silva
0 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-28 20:26 UTC (permalink / raw)
To: Kees Cook, Martin K . Petersen
Cc: Hannes Reinecke, James E.J. Bottomley, Gustavo A. R. Silva,
Bradley Grove, Artur Paszkiewicz, linux-kernel, linux-scsi,
linux-hardening
On 5/28/21 13:13, Kees Cook wrote:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memcpy(), avoid intentionally writing across
> neighboring array fields.
>
> Remove old-style 1-byte array in favor of a flexible array[1] to avoid
> future false-positive cross-field memcpy() warning in:
>
> esas2r_vda.c:
> memcpy(vi->cmd.gsv.version_info, esas2r_vdaioctl_versions, ...)
>
> The change in struct size doesn't change other structure sizes (it is
> already maxed out to 256 bytes, for example here:
>
> union {
> struct atto_ioctl_vda_scsi_cmd scsi;
> struct atto_ioctl_vda_flash_cmd flash;
> struct atto_ioctl_vda_diag_cmd diag;
> struct atto_ioctl_vda_cli_cmd cli;
> struct atto_ioctl_vda_smp_cmd smp;
> struct atto_ioctl_vda_cfg_cmd cfg;
> struct atto_ioctl_vda_mgt_cmd mgt;
> struct atto_ioctl_vda_gsv_cmd gsv;
> u8 cmd_info[256];
> } cmd;
>
> No sizes are calculated using the enclosing structure, so no other
> updates are needed.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks
--
Gustavo
> ---
> drivers/scsi/esas2r/atioctl.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/esas2r/atioctl.h b/drivers/scsi/esas2r/atioctl.h
> index 4aca3d52c851..ff2ad9b38575 100644
> --- a/drivers/scsi/esas2r/atioctl.h
> +++ b/drivers/scsi/esas2r/atioctl.h
> @@ -1141,7 +1141,7 @@ struct __packed atto_ioctl_vda_gsv_cmd {
>
> u8 rsp_len;
> u8 reserved[7];
> - u8 version_info[1];
> + u8 version_info[];
> #define ATTO_VDA_VER_UNSUPPORTED 0xFF
>
> };
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] scsi: isci: Use correctly sized target buffer for memcpy()
2021-05-28 18:13 ` [PATCH 3/3] scsi: isci: Use correctly sized target buffer for memcpy() Kees Cook
@ 2021-05-28 20:29 ` Gustavo A. R. Silva
0 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-28 20:29 UTC (permalink / raw)
To: Kees Cook, Martin K . Petersen
Cc: Hannes Reinecke, James E.J. Bottomley, Gustavo A. R. Silva,
Bradley Grove, Artur Paszkiewicz, linux-kernel, linux-scsi,
linux-hardening
On 5/28/21 13:13, Kees Cook wrote:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memcpy(), avoid intentionally writing across
> neighboring array fields.
>
> Switch from rsp_ui to resp_buf, since resp_ui isn't SSP_RESP_IU_MAX_SIZE
> bytes in length. This avoids future compile-time warnings.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks
--
Gustavo
> ---
> drivers/scsi/isci/task.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
> index 62062ed6cd9a..eeaec26ac324 100644
> --- a/drivers/scsi/isci/task.c
> +++ b/drivers/scsi/isci/task.c
> @@ -709,8 +709,8 @@ isci_task_request_complete(struct isci_host *ihost,
> tmf->status = completion_status;
>
> if (tmf->proto == SAS_PROTOCOL_SSP) {
> - memcpy(&tmf->resp.resp_iu,
> - &ireq->ssp.rsp,
> + memcpy(tmf->resp.rsp_buf,
> + ireq->ssp.rsp_buf,
> SSP_RESP_IU_MAX_SIZE);
> } else if (tmf->proto == SAS_PROTOCOL_SATA) {
> memcpy(&tmf->resp.d2h_fis,
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows
2021-05-28 18:13 [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows Kees Cook
` (2 preceding siblings ...)
2021-05-28 18:13 ` [PATCH 3/3] scsi: isci: Use correctly sized target buffer for memcpy() Kees Cook
@ 2021-06-02 3:38 ` Martin K. Petersen
2021-06-02 18:00 ` Kees Cook
2021-06-08 3:05 ` Martin K. Petersen
4 siblings, 1 reply; 11+ messages in thread
From: Martin K. Petersen @ 2021-06-02 3:38 UTC (permalink / raw)
To: Kees Cook
Cc: Martin K . Petersen, Hannes Reinecke, James E.J. Bottomley,
Gustavo A. R. Silva, Bradley Grove, Artur Paszkiewicz,
linux-kernel, linux-scsi, linux-hardening
Kees,
> While working on improving FORTIFY_SOURCE's memcpy() coverage, there are
> a few fixes that don't require any helper changes, etc.
Applied patches 2 and 3 to 5.14/scsi-staging, please update patch 1.
Thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows
2021-06-02 3:38 ` [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows Martin K. Petersen
@ 2021-06-02 18:00 ` Kees Cook
0 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2021-06-02 18:00 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Hannes Reinecke, James E.J. Bottomley, Gustavo A. R. Silva,
Bradley Grove, Artur Paszkiewicz, linux-kernel, linux-scsi,
linux-hardening
On Tue, Jun 01, 2021 at 11:38:47PM -0400, Martin K. Petersen wrote:
> Kees,
>
> > While working on improving FORTIFY_SOURCE's memcpy() coverage, there are
> > a few fixes that don't require any helper changes, etc.
>
> Applied patches 2 and 3 to 5.14/scsi-staging, please update patch 1.
>
> Thanks!
Awesome; thanks! I've sent v2 for patch 1 now.
--
Kees Cook
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows
2021-05-28 18:13 [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows Kees Cook
` (3 preceding siblings ...)
2021-06-02 3:38 ` [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows Martin K. Petersen
@ 2021-06-08 3:05 ` Martin K. Petersen
4 siblings, 0 replies; 11+ messages in thread
From: Martin K. Petersen @ 2021-06-08 3:05 UTC (permalink / raw)
To: Kees Cook
Cc: Martin K . Petersen, Artur Paszkiewicz, Gustavo A. R. Silva,
linux-scsi, Hannes Reinecke, Bradley Grove, James E.J. Bottomley,
linux-hardening, linux-kernel
On Fri, 28 May 2021 11:13:34 -0700, Kees Cook wrote:
> While working on improving FORTIFY_SOURCE's memcpy() coverage, there are
> a few fixes that don't require any helper changes, etc.
>
> -Kees
>
> Kees Cook (3):
> scsi: fcoe: Statically initialize flogi_maddr
> scsi: esas2r: Switch to flexible array member
> scsi: isci: Use correctly sized target buffer for memcpy()
>
> [...]
Applied to 5.14/scsi-queue, thanks!
[2/3] scsi: esas2r: Switch to flexible array member
https://git.kernel.org/mkp/scsi/c/66fc475bd9e1
[3/3] scsi: isci: Use correctly sized target buffer for memcpy()
https://git.kernel.org/mkp/scsi/c/5250db63d140
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-06-08 3:06 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-28 18:13 [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows Kees Cook
2021-05-28 18:13 ` [PATCH 1/3] scsi: fcoe: Statically initialize flogi_maddr Kees Cook
2021-05-28 18:28 ` Joe Perches
2021-05-28 18:29 ` Kees Cook
2021-05-28 18:13 ` [PATCH 2/3] scsi: esas2r: Switch to flexible array member Kees Cook
2021-05-28 20:26 ` Gustavo A. R. Silva
2021-05-28 18:13 ` [PATCH 3/3] scsi: isci: Use correctly sized target buffer for memcpy() Kees Cook
2021-05-28 20:29 ` Gustavo A. R. Silva
2021-06-02 3:38 ` [PATCH 0/3] scsi: Fix a handful of memcpy() field overflows Martin K. Petersen
2021-06-02 18:00 ` Kees Cook
2021-06-08 3:05 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.