Re: [RFC] PCI: allow sysfs file owner to read the config space with CAP_SYS_RAWIO

From: Allen <allen.lkml@gmail.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: linux-pci@vger.kernel.org, bhelgaas@google.com, ast@kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Allen Pais <apais@linux.microsoft.com>,
	Allen Pais <allen.pais@lkml.com>
Subject: Re: [RFC] PCI: allow sysfs file owner to read the config space with CAP_SYS_RAWIO
Date: Mon, 19 Oct 2020 18:30:16 +0530	[thread overview]
Message-ID: <CAOMdWSJDJ-uXpis1WbG3LnOG7bMiif5Q4Maafv_a=55Y_qypfQ@mail.gmail.com> (raw)
In-Reply-To: <20201016062027.GB569795@kroah.com>

> >
> >  Access to pci config space is explictly checked with CAP_SYS_ADMIN
> > in order to read configuration space past the frist 64B.
> >
> >  Since the path is only for reading, could we use CAP_SYS_RAWIO?
>
> Why?  What needs this reduced capability?

Thanks for the review.

We need read access to /sys/bus/pci/devices/,  We need write access to config,
remove, rescan & enable files under the device directory for each PCIe
functions & the downstream PCIe port.

We need r/w access to sysfs to unbind and rebind the root complex.

>
> > This patch contains a simpler fix, I would love to hear from the
> > Maintainers on the approach.
> >
> >  The other approach that I considered was to introduce and API
> > which would check for multiple capabilities, something similar to
> > perfmon_capable()/bpf_capable(). But I could not find more users
> > for the API and hence dropped it.
> >
> >  The problem I am trying to solve is to avoid handing out
> > CAP_SYS_ADMIN for extended reads of the PCI config space.
>
> Who is reading this config space that doesn't have admin rights?  And
> what are they doing with it?
>
> One big problem is that some devices will crash if you do this wrong,
> which is why we restricted it to root.  Hopefully all of those devices
> are now gone, but I don't think you can count on it.
>
> The "guaranteed safe" fields in the config space are already exported by
> sysfs for all users to read, are they not sufficient?
>
> thanks,
>
> greg k-h

-- 
       - Allen