Re: [PATCH] PCI/sysfs: Fix read permissions for VPD attributes

[Leon Romanovsky <leon@kernel.org>]

On Tue, Nov 05, 2024 at 09:24:55AM -0600, Bjorn Helgaas wrote:
> On Tue, Nov 05, 2024 at 09:51:30AM +0200, Leon Romanovsky wrote:
> > On Mon, Nov 04, 2024 at 06:10:27PM -0600, Bjorn Helgaas wrote:
> > > On Sun, Nov 03, 2024 at 02:33:44PM +0200, Leon Romanovsky wrote:
> > > > On Fri, Nov 01, 2024 at 11:47:37AM -0500, Bjorn Helgaas wrote:
> > > > > On Fri, Nov 01, 2024 at 04:33:00PM +0200, Leon Romanovsky wrote:
> > > > > > On Thu, Oct 31, 2024 at 06:22:52PM -0500, Bjorn Helgaas wrote:
> > > > > > > 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@nvidia.com>
> > > > > > > > > 
> > > > > > > > > The Virtual Product Data (VPD) attribute is not
> > > > > > > > > readable by regular 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.
> > ...
> 
> > > What's the use case?  How does an unprivileged user use the VPD
> > > information?
> > 
> > We have to add new field keyword=value in VA section of VPD, which
> > will indicate very specific sub-model for devices used as a bridge.
> > 
> > > I can certainly imagine using VPD for bug reporting, but that
> > > would typically involve dmesg, dmidecode, lspci -vv, etc, all of
> > > which already require privilege, so it's not clear to me how
> > > public VPD info would help in that scenario.
> > 
> > I'm targeting other scenario - monitoring tool, which doesn't need
> > root permissions for reading data. It needs to distinguish between
> > NIC sub-models.
> 
> Maybe the driver could expose something in sysfs?  Maybe the driver
> needs to know the sub-model as well, and reading VPD once in the
> driver would make subsequent userspace sysfs reads trivial and fast.

Our PCI driver lays in netdev subsystem and they have long-standing
position do not allow any custom sysfs files. To be fair, we (RDMA)
don't allow custom sysfs files too.

Driver doesn't need to know this information as it is extra key=value in
existing [VA] field, while driver relies on multiple FW capabilities
to enable/disable functionality.

Current [VA] line:
"[VA] Vendor specific: MLX:MN=MLNX:CSKU=V2:UUID=V3:PCI=V0:MODL=CX713106A"
Future [VA] line:
"[VA] Vendor specific: MLX:MN=MLNX:CSKU=V2:UUID=V3:PCI=V0:MODL=CX713106A,SMDL=SOMETHING"

Also the idea that we will duplicate existing functionality doesn't
sound like a good approach to me, and there is no way that it is
possible to expose as subsystem specific file.

What about to allow existing VPD sysfs file to be readable for everyone for our devices?
And if this allow list grows to much, we will open it for all devices in the world?

Thanks

> 
> Bjorn