On Thu, 2024-04-04 at 18:29 -0400, James Bottomley wrote:
> On Thu, 2024-04-04 at 15:04 -0700, Justin Stitt wrote:
[...]
> > It's interesting that fortify doesn't like if the len argument is
> > one byte larger than the source argument because while technically
> > it does overread the buffer strscpy will manually write a NUL-byte
> > to the destination buffer.
> >
> > The easy fix I see that doesn't limit the amount of representable
> > data is to increase these fields by 1 to match sas_expander_device.
> >
> > diff --git a/drivers/message/fusion/mptsas.c
> > b/drivers/message/fusion/mptsas.c
> > index 300f8e955a53..44b492698e06 100644
> > --- a/drivers/message/fusion/mptsas.c
> > +++ b/drivers/message/fusion/mptsas.c
> > @@ -2833,10 +2833,10 @@ struct rep_manu_reply{
> > u8 sas_format:1;
> > u8 reserved1:7;
> > u8 reserved2[3];
> > - u8 vendor_id[SAS_EXPANDER_VENDOR_ID_LEN];
> > - u8 product_id[SAS_EXPANDER_PRODUCT_ID_LEN];
> > - u8 product_rev[SAS_EXPANDER_PRODUCT_REV_LEN];
> > - u8 component_vendor_id[SAS_EXPANDER_COMPONENT_VENDOR_ID_LEN];
> > + u8 vendor_id[SAS_EXPANDER_VENDOR_ID_LEN+1];
> > + u8 product_id[SAS_EXPANDER_PRODUCT_ID_LEN+1];
> > + u8 product_rev[SAS_EXPANDER_PRODUCT_REV_LEN+1];
> > + u8 component_vendor_id[SAS_EXPANDER_COMPONENT_VENDOR_ID_LEN+1];
>
> This is a reply returned by the firmware ... we can't just
> arbitrarily change field sizes because then it won't match what the
> firmware is sending.
But additionally this is a common pattern in SCSI: using strncpy to
zero terminate fields that may be unterminated in the exchange protocol
so we can send them to sysfs or otherwise treat them as strings. That
means we might have this problem in other drivers you've converted ...
James
