linux-kernel - Re: [PATCH v1 1/2] PCI/sysfs: Change read permissions for VPD attributes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241112214819.GA1862173@bhelgaas>
Date: Tue, 12 Nov 2024 15:48:19 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Leon Romanovsky <leon@...nel.org>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
	Stephen Hemminger <stephen@...workplumber.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	linux-pci@...r.kernel.org, Ariel Almog <ariela@...dia.com>,
	Aditya Prabhune <aprabhune@...dia.com>,
	Hannes Reinecke <hare@...e.de>, Arun Easi <aeasi@...vell.com>,
	Jonathan Chocron <jonnyc@...zon.com>,
	Bert Kenward <bkenward@...arflare.com>,
	Matt Carlson <mcarlson@...adcom.com>,
	Kai-Heng Feng <kai.heng.feng@...onical.com>,
	Jean Delvare <jdelvare@...e.de>,
	Alex Williamson <alex.williamson@...hat.com>,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v1 1/2] PCI/sysfs: Change read permissions for VPD
 attributes

On Tue, Nov 12, 2024 at 09:26:04AM +0200, Leon Romanovsky wrote:
> On Tue, Nov 12, 2024 at 07:44:09AM +0100, Heiner Kallweit wrote:
> > On 12.11.2024 01:34, Stephen Hemminger wrote:
> > > On Mon, 11 Nov 2024 14:41:04 -0600
> > > Bjorn Helgaas <helgaas@...nel.org> wrote:
> > > 
> > >> On Thu, Nov 07, 2024 at 08:56:56PM +0200, Leon Romanovsky wrote:
> > >>> From: Leon Romanovsky <leonro@...dia.com>
> > >>>
> > >>> The Vital Product Data (VPD) attribute is not readable by regular
> > >>> user without root permissions. Such restriction is not really needed
> > >>> for many devices in the world, as data presented in that VPD is not
> > >>> sensitive and access to the HW is safe and tested.
> > >>>
> > >>> This change aligns the permissions of the VPD attribute to be accessible
> > >>> for read by all users, while write being restricted to root only.
> > >>>
> > >>> For the driver, there is a need to opt-in in order to allow this
> > >>> functionality.  
> > >>
> > >> I don't think the use case is very strong (and not included at all
> > >> here).
> > >>
> > >> If we do need to do this, I think it's a property of the device, not
> > >> the driver.
> > > 
> > > I remember some broken PCI devices, which will crash if VPD is read.
> > > Probably not worth opening this can of worms.
> > 
> > These crashes shouldn't occur any longer. There are two problematic cases:
> > 1. Reading past end of VPD
> >    This used to crash certain devices and was fixed by stop reading at
> >    the VPD end tag.
> > 2. Accessing VPD if device firmware isn't correctly loaded and initialized
> >    This affects certain LSI devices, which are blacklisted so that PCI core
> >    prevents VPD access.
> 
> Thanks for the information.
> 
> Bjorn,
> 
> After this response, do you still think that v0 [1] is not the right way
> to change the read permission?
> 
> [1] https://lore.kernel.org/all/65791906154e3e5ea12ea49127cf7c707325ca56.1730102428.git.leonro@nvidia.com/

Yes, I still think it's unnecessarily risky to make VPD readable
by ordinary users.  This is a pretty niche use case.

Bjorn