Re: startup BUG at lib/string_helpers.c from scsi fusion mptsas

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux