On Thu, 2024-04-04 at 15:04 -0700, Justin Stitt wrote: > Hi, > > On Thu, Apr 4, 2024 at 2:53 PM James Bottomley > <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > > > > On Thu, 2024-04-04 at 14:38 -0700, Justin Stitt wrote: > > [...] > > > I am not sure how my patch [1] is triggering this fortify panic. > > > I > > > didn't modify this printk or the string arguments (ioc->name), > > > also > > > the change from strncpy to strscpy did not introduce any > > > strnlen()'s > > > which seems to be the thing fortify is upset about: > > > "2024-04-01T19:18:28.000000+00:00 zGMT kernel - - - detected > > > buffer > > > overflow in strnlen" > > > or > > > "2024-04-01T22:23:45.000000+00:00 zGMT kernel - - - strnlen: > > > detected > > > buffer overflow: 9 byte read of buffer size 8" > > > > it's sitting in the definition of sized_strscpy in fortify-string.h > > I see now, I was jumping to the definition in lib/... > > > > > Since the fields in question aren't zero terminated there's a bad > > assumption that you can do strnlen on the source field. > > 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. James