[+cc LKML etc, should PCI VPD be readable by non-root?]
On Tue, Oct 29, 2024 at 07:04:50PM -0500, Bjorn Helgaas wrote:
> On Mon, Oct 28, 2024 at 10:05:33AM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxx>
> >
> > The Virtual Product Data (VPD) attribute is not readable by regular
("Vital Product Data" is the term in the spec)
> > user without root permissions. Such restriction is not really needed,
> > as data presented in that VPD is not sensitive at all.
> >
> > This change aligns the permissions of the VPD attribute to be accessible
> > for read by all users, while write being restricted to root only.
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: d93f8399053d ("PCI/sysfs: Convert "vpd" to static attribute")
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
>
> Applied to pci/vpd for v6.13, thanks!
I think this deserves a little more consideration than I gave it
initially.
Obviously somebody is interested in using this; can we include some
examples so we know there's an actual user?
Are we confident that VPD never contains anything sensitive? It may
contain arbitrary vendor-specific information, so we can't know what
might be in that part.
Reading VPD is fairly complicated and we've had problems in the past
(we have quirk_blacklist_vpd() for devices that behave
"unpredictably"), so it's worth considering whether allowing non-root
to do this could be exploited or could allow DOS attacks.
For reference, here are the fields defined in PCIe r6.0, sec 6.27.2
(although VPD can contain anything a manufacturer wants to put there):
PN Add-in Card Part Number
EC Engineering Change Level of the Add-in Card
FG Fabric Geography
LC Location
MN Manufacture ID
PG PCI Geography
SN Serial Number
TR Thermal Reporting
Vx Vendor Specific
CP Extended Capability
RV Checksum and Reserved
FF Form Factor
Yx System Specific
YA Asset Tag Identifier
RW Remaining Read/Write Area
The Conventional PCI spec, r3.0, sec 6.4, says:
Vital Product Data (VPD) is the information that uniquely defines
items such as the hardware, software, and microcode elements of a
system. The VPD provides the system with information on various FRUs
(Field Replaceable Unit) including Part Number, Serial Number, and
other detailed information. VPD also provides a mechanism for
storing information such as performance and failure data on the
device being monitored. The objective, from a system point of view,
is to collect this information by reading it from the hardware,
software, and microcode components.
Some of that, e.g., performance and failure data, might be considered
sensitive in some environments.
> > ---
> > I added stable@ as it was discovered during our hardware ennoblement
> > and it is important to be picked by distributions too.
> > ---
> > drivers/pci/vpd.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c
> > index e4300f5f304f..2537685cac90 100644
> > --- a/drivers/pci/vpd.c
> > +++ b/drivers/pci/vpd.c
> > @@ -317,7 +317,7 @@ static ssize_t vpd_write(struct file *filp, struct kobject *kobj,
> >
> > return ret;
> > }
> > -static BIN_ATTR(vpd, 0600, vpd_read, vpd_write, 0);
> > +static BIN_ATTR_RW(vpd, 0);
> >
> > static struct bin_attribute *vpd_attrs[] = {
> > &bin_attr_vpd,
> > --
> > 2.46.2
> >
