| Message ID | Y+LJz/r6+UeLqnV3@work (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3,next] scsi: smartpqi: Replace one-element array with flexible-array member | expand |
On Tue, Feb 07, 2023 at 03:59:43PM -0600, Gustavo A. R. Silva wrote: > One-element arrays are deprecated, and we are replacing them with flexible > array members instead. So, replace one-element array with flexible-array > member in struct report_log_lun_list. > > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines > on memcpy(). > > Link: https://github.com/KSPP/linux/issues/79 > Link: https://github.com/KSPP/linux/issues/204 > Acked-by: Don Brace <don.brace@microchip.com> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Looks good to me -- both before and after, there is 1 extra all-zero entry at the end. But now the struct is using a flex array correctly. Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > Changes in v3: > - Amplify the existing comment instead of adding a new one. By Don > Brace[1]. > - Add Don's Acked-by tag to the changelog. > Link: https://lore.kernel.org/linux-hardening/d881ad98-a291-1c9b-53dc-199f23f1625e@embeddedor.com/ [1] > > Changes in v2: > - In v1 we thought that the original code was allocating one too-many > entries for the list. However, Don Brace commented that the allocation > was actually intentional[2]. So, I added a code comment with his feedback. > Link: https://lore.kernel.org/linux-hardening/16e6c434-44af-2efb-d4bc-a253e93e5590@embeddedor.com/ [2] > > v1: > - Link: https://lore.kernel.org/linux-hardening/c80c0979933e0c05e80d95792ef167a28640a14b.1663816572.git.gustavoars@kernel.org/ > > drivers/scsi/smartpqi/smartpqi.h | 2 +- > drivers/scsi/smartpqi/smartpqi_init.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h > index af27bb0f3133..228838eb3686 100644 > --- a/drivers/scsi/smartpqi/smartpqi.h > +++ b/drivers/scsi/smartpqi/smartpqi.h > @@ -954,7 +954,7 @@ struct report_log_lun { > > struct report_log_lun_list { > struct report_lun_header header; > - struct report_log_lun lun_entries[1]; > + struct report_log_lun lun_entries[]; > }; > > struct report_phys_lun_8byte_wwid { > diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c > index d0446d4d4465..49a8f91810b6 100644 > --- a/drivers/scsi/smartpqi/smartpqi_init.c > +++ b/drivers/scsi/smartpqi/smartpqi_init.c > @@ -1259,7 +1259,8 @@ static int pqi_get_device_lists(struct pqi_ctrl_info *ctrl_info, > "report logical LUNs failed\n"); > > /* > - * Tack the controller itself onto the end of the logical device list. > + * Tack the controller itself onto the end of the logical device list > + * by adding a list entry that is all zeros. > */ > > logdev_data = *logdev_list; > -- > 2.34.1 >
On 2/7/23 17:01, Kees Cook wrote: > On Tue, Feb 07, 2023 at 03:59:43PM -0600, Gustavo A. R. Silva wrote: >> One-element arrays are deprecated, and we are replacing them with flexible >> array members instead. So, replace one-element array with flexible-array >> member in struct report_log_lun_list. >> >> This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines >> on memcpy(). >> >> Link: https://github.com/KSPP/linux/issues/79 >> Link: https://github.com/KSPP/linux/issues/204 >> Acked-by: Don Brace <don.brace@microchip.com> >> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > Looks good to me -- both before and after, there is 1 extra all-zero > entry at the end. But now the struct is using a flex array correctly. Yep. :) > > Reviewed-by: Kees Cook <keescook@chromium.org> Thanks! -- Gustavo > > -Kees > >> --- >> Changes in v3: >> - Amplify the existing comment instead of adding a new one. By Don >> Brace[1]. >> - Add Don's Acked-by tag to the changelog. >> Link: https://lore.kernel.org/linux-hardening/d881ad98-a291-1c9b-53dc-199f23f1625e@embeddedor.com/ [1] >> >> Changes in v2: >> - In v1 we thought that the original code was allocating one too-many >> entries for the list. However, Don Brace commented that the allocation >> was actually intentional[2]. So, I added a code comment with his feedback. >> Link: https://lore.kernel.org/linux-hardening/16e6c434-44af-2efb-d4bc-a253e93e5590@embeddedor.com/ [2] >> >> v1: >> - Link: https://lore.kernel.org/linux-hardening/c80c0979933e0c05e80d95792ef167a28640a14b.1663816572.git.gustavoars@kernel.org/ >> >> drivers/scsi/smartpqi/smartpqi.h | 2 +- >> drivers/scsi/smartpqi/smartpqi_init.c | 3 ++- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h >> index af27bb0f3133..228838eb3686 100644 >> --- a/drivers/scsi/smartpqi/smartpqi.h >> +++ b/drivers/scsi/smartpqi/smartpqi.h >> @@ -954,7 +954,7 @@ struct report_log_lun { >> >> struct report_log_lun_list { >> struct report_lun_header header; >> - struct report_log_lun lun_entries[1]; >> + struct report_log_lun lun_entries[]; >> }; >> >> struct report_phys_lun_8byte_wwid { >> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c >> index d0446d4d4465..49a8f91810b6 100644 >> --- a/drivers/scsi/smartpqi/smartpqi_init.c >> +++ b/drivers/scsi/smartpqi/smartpqi_init.c >> @@ -1259,7 +1259,8 @@ static int pqi_get_device_lists(struct pqi_ctrl_info *ctrl_info, >> "report logical LUNs failed\n"); >> >> /* >> - * Tack the controller itself onto the end of the logical device list. >> + * Tack the controller itself onto the end of the logical device list >> + * by adding a list entry that is all zeros. >> */ >> >> logdev_data = *logdev_list; >> -- >> 2.34.1 >> >
Gustavo, > One-element arrays are deprecated, and we are replacing them with > flexible array members instead. So, replace one-element array with > flexible-array member in struct report_log_lun_list. Applied to 6.3/scsi-staging, thanks!
On Tue, 07 Feb 2023 15:59:43 -0600, Gustavo A. R. Silva wrote: > One-element arrays are deprecated, and we are replacing them with flexible > array members instead. So, replace one-element array with flexible-array > member in struct report_log_lun_list. > > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines > on memcpy(). > > [...] Applied to 6.3/scsi-queue, thanks! [1/1] scsi: smartpqi: Replace one-element array with flexible-array member https://git.kernel.org/mkp/scsi/c/ead821268c14
diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h index af27bb0f3133..228838eb3686 100644 --- a/drivers/scsi/smartpqi/smartpqi.h +++ b/drivers/scsi/smartpqi/smartpqi.h @@ -954,7 +954,7 @@ struct report_log_lun { struct report_log_lun_list { struct report_lun_header header; - struct report_log_lun lun_entries[1]; + struct report_log_lun lun_entries[]; }; struct report_phys_lun_8byte_wwid { diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index d0446d4d4465..49a8f91810b6 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -1259,7 +1259,8 @@ static int pqi_get_device_lists(struct pqi_ctrl_info *ctrl_info, "report logical LUNs failed\n"); /* - * Tack the controller itself onto the end of the logical device list. + * Tack the controller itself onto the end of the logical device list + * by adding a list entry that is all zeros. */ logdev_data = *logdev_list;