Ben Hutchings wrote:
> Matthew Wilcox wrote:
> > On Thu, Jun 26, 2008 at 09:09:25PM -0700, Benjamin Li wrote:
> > > For Broadcom 5706, 5708, 5709 rev. A nics, any read beyond the
> > > VPD end tag will hang the device. This problem was initially
> > > observed when a vpd entry was created in sysfs
> > > ('/sys/bus/pci/devices/<id>/vpd'). A read to this sysfs entry
> > > will dump 32k of data. Reading a full 32k will cause an access
> > > beyond the VPD end tag causing the device to hang. Once the device
> > > is hung, the bnx2 driver will not be able to reset the device.
> > > We believe that it is legal to read beyond the end tag and
> > > therefore the solution is to disable the VPD capability and
> > > not expose this data.
> >
> > Different scholars have differing opinions on whether it's legal to read
> > beyond the end tag.
>
> Depending on whether "Reading or writing data outside of the VPD space in
> the storage component is not allowed" is to be taken as restricting the
> implementation or the software using it?
>
> > Regardless of the law, there's devices in the field
> > that have this problem, so let's not throw the baby out with the
> > bathwater.
I'm concerned that there might be more of them out there. Given that I
exposed VPD as world-readable, this would result in a denial-of-service
vulnerability on those devices.
So I'm tempted to chicken out and ask for that change (commit
94e6108803469a37ee1e3c92dafdd1d59298602f) to be reverted for 2.6.26, or for
VPD to be made readable only by root for now.
> > I think with the below patch you can now do something like this:
> >
> > static void __devinit quirk_brcm_570x_shorten_vpd(struct pci_dev *dev)
> > {
> > dev->vpd->base.len = 8192;
>
> dev->vpd is a pointer to struct pci_vpd and may be NULL, so this should be:
> if (dev->vpd)
> dev->vpd.len = 8192;
>
> > }
> >
> > We might also want to consider parsing the top level of the VPD data
> > structures to find out how long they are.
> [...]
>
> Certainly a possibility. Let me see if I can do that.
I'll follow up with a patch that does that. However, that will introduce
VPD reads during initialisation, which we do not do now. This seems like
a risky addition close to the release of 2.6.26, but would be good for
-next.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
