netdev - Re: [PATCH v2] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241121121301.GA160612@unreal>
Date: Thu, 21 Nov 2024 14:13:01 +0200
From: Leon Romanovsky <leon@...nel.org>
To: Jean Delvare <jdelvare@...e.de>, Bjorn Helgaas <helgaas@...nel.org>
Cc: 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>,
	Heiner Kallweit <hkallweit1@...il.com>,
	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>,
	Alex Williamson <alex.williamson@...hat.com>,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	Jakub Kicinski <kuba@...nel.org>,
	Thomas Weißschuh <linux@...ssschuh.net>,
	Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH v2] PCI/sysfs: Change read permissions for VPD attributes

On Thu, Nov 21, 2024 at 01:01:27PM +0100, Jean Delvare wrote:
> Hi Leon,
> 
> On Wed, 13 Nov 2024 14:59:58 +0200, Leon Romanovsky wrote:
> > --- a/drivers/pci/vpd.c
> > +++ b/drivers/pci/vpd.c
> > @@ -332,6 +332,14 @@ static umode_t vpd_attr_is_visible(struct kobject *kobj,
> >  	if (!pdev->vpd.cap)
> >  		return 0;
> >  
> > +	/*
> > +	 * Mellanox devices have implementation that allows VPD read by
> > +	 * unprivileged users, so just add needed bits to allow read.
> > +	 */
> > +	WARN_ON_ONCE(a->attr.mode != 0600);
> > +	if (unlikely(pdev->vendor == PCI_VENDOR_ID_MELLANOX))
> > +		return a->attr.mode + 0044;
> 
> When manipulating bitfields, | is preferred. This would make the
> operation safe regardless of the initial value, so you can even get rid
> of the WARN_ON_ONCE() above.

The WARN_ON_ONCE() is intended to catch future changes in VPD sysfs
attributes. My intention is that once that WARN will trigger, the
author will be forced to reevaluate the latter if ( ... PCI_VENDOR_ID_MELLANOX)
condition and maybe we won't need it anymore. Without WARN_ON_ONCE, it
is easy to miss that code.

I still didn't lost hope that at some point VPD will be open for read to
all kernel devices.

Bjorn, are you ok with this patch? If yes, I'll resend the patch with
the suggested change after the merge window.

Thanks

> 
> > +
> >  	return a->attr.mode;
> >  }
> >  
> 
> -- 
> Jean Delvare
> SUSE L3 Support
>