| Message ID | 20210528181337.792268-3-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | scsi: Fix a handful of memcpy() field overflows | expand |
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 > > }; >
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 };
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(-)