On Thu, Dec 02, 2021 at 11:23:26AM -0800, Kees Cook wrote: > On Fri, Nov 05, 2021 at 12:24:13PM +0100, Steffen Maier wrote: > > On 11/5/21 10:11, Gustavo A. R. Silva wrote: > > > Use flexible-array members in struct fc_fdmi_attr_entry and > > > fs_fdmi_attrs instead of one-element arrays, and refactor the > > > code accordingly. > > > > > > Also, turn the one-element array _port_ in struct fc_fdmi_rpl > > > into a simple object of type struct fc_fdmi_port_name, as it > > > seems there is no more than just one port expected: > > > > > > $ git grep -nw numport drivers/scsi/ > > > drivers/scsi/csiostor/csio_lnode.c:447: reg_pl->numport = htonl(1); > > > drivers/scsi/libfc/fc_encode.h:232: put_unaligned_be32(1, &ct->payload.rhba.port.numport); > > > > > > Also, this helps with the ongoing efforts to globally enable > > > -Warray-bounds and get us closer to being able to tighten the > > > FORTIFY_SOURCE routines on memcpy(). > > > > > > https://github.com/KSPP/linux/issues/79 > > > Signed-off-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx> > > > --- > > > drivers/scsi/csiostor/csio_lnode.c | 2 +- > > > drivers/scsi/libfc/fc_encode.h | 4 ++-- > > > include/scsi/fc/fc_ms.h | 6 +++--- > > > 3 files changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/scsi/csiostor/csio_lnode.c b/drivers/scsi/csiostor/csio_lnode.c > > > index d5ac93897023..cf9dd79ee488 100644 > > > --- a/drivers/scsi/csiostor/csio_lnode.c > > > +++ b/drivers/scsi/csiostor/csio_lnode.c > > > @@ -445,7 +445,7 @@ csio_ln_fdmi_dprt_cbfn(struct csio_hw *hw, struct csio_ioreq *fdmi_req) > > > /* Register one port per hba */ > > > reg_pl = (struct fc_fdmi_rpl *)pld; > > > reg_pl->numport = htonl(1); > > > - memcpy(®_pl->port[0].portname, csio_ln_wwpn(ln), 8); > > > + memcpy(®_pl->port.portname, csio_ln_wwpn(ln), 8); > > > pld += sizeof(*reg_pl); > > > > > > /* Start appending HBA attributes hba */ > > > diff --git a/drivers/scsi/libfc/fc_encode.h b/drivers/scsi/libfc/fc_encode.h > > > index 74ae7fd15d8d..5806f99e4061 100644 > > > --- a/drivers/scsi/libfc/fc_encode.h > > > +++ b/drivers/scsi/libfc/fc_encode.h > > > @@ -232,7 +232,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport, > > > put_unaligned_be32(1, &ct->payload.rhba.port.numport); > > > /* Port Name */ > > > put_unaligned_be64(lport->wwpn, > > > - &ct->payload.rhba.port.port[0].portname); > > > + &ct->payload.rhba.port.port.portname); > > > > > > /* HBA Attributes */ > > > put_unaligned_be32(numattrs, > > > > > diff --git a/include/scsi/fc/fc_ms.h b/include/scsi/fc/fc_ms.h > > > index 00191695233a..44fbe84fa664 100644 > > > --- a/include/scsi/fc/fc_ms.h > > > +++ b/include/scsi/fc/fc_ms.h > > > > > @@ -174,7 +174,7 @@ struct fs_fdmi_attrs { > > > > /* > > * Registered Port List > > > > > */ > > > struct fc_fdmi_rpl { > > > __be32 numport; > > > - struct fc_fdmi_port_name port[1]; > > > + struct fc_fdmi_port_name port; > > > } __attribute__((__packed__)); > > > > While I'm not affected by the change, it feels to me as if these are > > protocol definitions originating in a T11 Fibre Channel standard FC-GS. It's > > a port *list*. Can you "modify" the standard here? > > > > The fact, that currently existing code users only ever seem to use one > > single port in the list, would be an independent thing to me. > > There are three changes made here, and I suspect it might make sense to > split them up. > > In a quick look, I see "struct fc_fdmi_attr_entry" has a sizeof() call > against it, so it's not clear if it's safe to switch it to a flexible > array without other changes. I'll take a look. > The change to struct fs_fdmi_attrs looks okay, since it appears to be > used only in casts, but it might make sense to use diffoscope on the > changed .o files to validate nothing weird has happened. OK. > For struct fc_dmi_rpl, as long as "numport" is always set/validated to > 1, I think this change is fine. OK. I'll split this up into a small series and keep struct fc_fdmi_port_name port[1]; as a one-element array. Thanks -- Gustavo
