Re: [PATCH v2 2/7] PCI: Set "untrusted" flag for truly external devices only

Greg Kroah-Hartman
2020-07-06T18:48:57Z

On Mon, Jul 06, 2020 at 11:41:26AM -0500, Bjorn Helgaas wrote:
> On Tue, Jun 30, 2020 at 09:55:54AM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Jun 29, 2020 at 09:49:38PM -0700, Rajat Jain wrote:
> > > The "ExternalFacing" devices (root ports) are still internal devices that
> > > sit on the internal system fabric and thus trusted. Currently they were
> > > being marked untrusted.
> > >
> > > This patch uses the platform flag to identify the external facing devices
> > > and then use it to mark any downstream devices as "untrusted". The
> > > external-facing devices themselves are left as "trusted". This was
> > > discussed here: https://lkml.org/lkml/2020/6/10/1049
> >
> > {sigh}
> >
> > First off, please use lore.kernel.org links, we don't control lkml.org
> > and it often times has been down.
> >
> > Also, you need to put all of the information in the changelog, referring
> > to another place isn't always the best thing, considering you will be
> > looking this up in 20+ years to try to figure out why people came up
> > with such a crazy design.
> >
> > But, the main point is, no, we did not decide on this. "trust" is a
> > policy decision to make by userspace, it is independant of "location",
> > while you are tieing it directly here, which is what I explicitly said
> > NOT to do.
> >
> > So again, no, I will NAK this patch as-is, sorry, you are mixing things
> > together in a way that it should not do at this point in time.
>
> What do you see being mixed together here? I acknowledge that the
> name of "pdev->untrusted" is probably a mistake. But this patch
> doesn't change anything there. It only changes the treatment of the
> edge case of the "ExternalFacing" ports. Previously we treated them
> as being external themselves, which does seem wrong.

I don't see the patch here, and it's been a while but I think there is a
mixture of "location" and "trust" happening here with a single value
when they should be separate.

Hopefully the next round of this patch series will be better.

thanks,

greg k-h