Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201016055235.440159-1-allen.lkml@gmail.com> <20201016062027.GB569795@kroah.com>
In-Reply-To: <20201016062027.GB569795@kroah.com>
From:   Allen <allen.lkml@gmail.com>
Date:   Mon, 19 Oct 2020 18:30:16 +0530
Message-ID: <CAOMdWSJDJ-uXpis1WbG3LnOG7bMiif5Q4Maafv_a=55Y_qypfQ@mail.gmail.com>
Subject: Re: [RFC] PCI: allow sysfs file owner to read the config space with CAP_SYS_RAWIO
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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

> >
> >  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