Linux Virtualization - Oct 2021 - [PATCH v5 12/16] PCI: Add pci_iomap_host_shared(), pci_iomap_host_shared_range()

On Sun, Oct 10, 2021 at 03:22:39PM -0700, Andi Kleen
wrote:
> 
> > To which Andi replied
> > 	One problem with removing the ioremap opt-in is that
> > 	it's still possible for drivers to get at devices without going
through probe.
> > 
> > To which Greg replied:
> > https://lore.kernel.org/all/YVXBNJ431YIWwZdQ at kroah.com/
> > 	If there are in-kernel PCI drivers that do not do this, they need to
be
> > 	fixed today.
> > 
> > Can you guys resolve the differences here?
> 
> 
> I addressed this in my other mail, but we may need more discussion.
Hopefully Greg will reply to that one.
> 
> > 
> > And once they are resolved, mention this in the commit log so
> > I don't get to re-read the series just to find out nothing
> > changed in this respect?
> > 
> > I frankly do not believe we are anywhere near being able to harden
> > an arbitrary kernel config against attack.
> 
> Why not? Device filter and the opt-ins together are a fairly strong
> mechanism.
Because it does not end with I/O operations, that's a trivial example.
module unloading is famous for being racy: I just re-read that part of
virtio drivers and sure enough we have bugs there, this is after
they have presumably been audited, so a TDX guest is better off
just disabling hot-unplug completely, and hotplug isn't far behind.
Malicious filesystems can exploit many linux systems unless
you take pains to limit what is mounted and how.
Networking devices tend to get into the default namespaces and can
do more or less whatever CAP_NET_ADMIN can.
Etc.
I am not saying this makes the effort worthless, I am saying userspace
better know very well what it's doing, and kernel better be
configured in a very specific way.
> And it's not that they're a lot of code or super complicated
either.
> 
> You're essentially objecting to a single line change in your subsystem
here.
Well I commented on the API patch, not the virtio patch.
If it's a way for a driver to say "I am hardened
and audited" then I guess it should at least say so. It has nothing
to do with host or sharing, that's an implementation detail,
and it obscures the actual limitations of the approach,
in that eventually in an ideal world all drivers would be secure
and use this API.

Yes, if that's the API that PCI gains then virtio will use it.

> > How about creating a defconfig that makes sense for TDX then?
> 
> TDX can be used in many different ways, I don't think a defconfig is
> practical.
> 
> In theory you could do some Kconfig dependency (at the pain point of having
> separate kernel binariees), but why not just do it at run time then if you
> maintain the list anyways. That's much easier and saner for everyone.
In the
> past we usually always ended up with runtime mechanism for similar things
> anyways.
> 
> Also it turns out that the filter mechanisms are needed for some arch
> drivers which are not even configurable, so alone it's probably not
enough,

I guess they aren't really needed though right, or you won't try to
filter them? So make them configurable?
> 
> > Anyone deviating from that better know what they are doing,
> > this API tweaking is just putting policy into the kernel  ...
> 
> Hardening drivers is kernel policy. It cannot be done anywhere else.
> 
> 
> -Andi
To clarify, the policy is which drivers to load into the kernel.

-- 
MST

> Because it does not end with I/O operations, that's a trivial example.
> module unloading is famous for being racy: I just re-read that part of
> virtio drivers and sure enough we have bugs there, this is after
> they have presumably been audited, so a TDX guest is better off
> just disabling hot-unplug completely, and hotplug isn't far behind.
These all shouldn't matter for a confidential guest. The only way it can 
be attacked is through IO, everything else is protected by hardware.


Also it would all require doing something at the guest level, which we 
assume is not malicious.

> Malicious filesystems can exploit many linux systems unless
> you take pains to limit what is mounted and how.
That's expected to be handled by authenticated dmcrypt and similar. 
Hardening at this level has been done for many years.

> Networking devices tend to get into the default namespaces and can
> do more or less whatever CAP_NET_ADMIN can.
> Etc.

Networking should be already hardened, otherwise you would have much 
worse problems today.


> hange in your subsystem here.
> Well I commented on the API patch, not the virtio patch.
> If it's a way for a driver to say "I am hardened
> and audited" then I guess it should at least say so.

This is handled by the central allow list. We intentionally didn't want 
each driver to declare itself, but have a central list where changes 
will get more scrutiny than random driver code.

But then there are the additional opt-ins for the low level firewall. 
These are in the API. I don't see how it could be done at the driver 
level, unless you want to pass in a struct device everywhere?
>>> How about creating a defconfig that makes sense for TDX then?
>> TDX can be used in many different ways, I don't think a defconfig
is
>> practical.
>>
>> In theory you could do some Kconfig dependency (at the pain point of
having
>> separate kernel binariees), but why not just do it at run time then if
you
>> maintain the list anyways. That's much easier and saner for
everyone. In the
>> past we usually always ended up with runtime mechanism for similar
things
>> anyways.
>>
>> Also it turns out that the filter mechanisms are needed for some arch
>> drivers which are not even configurable, so alone it's probably not
enough,
>
> I guess they aren't really needed though right, or you won't try to
> filter them?
We're addressing most of them with the device filter for platform 
drivers. But since we cannot stop them doing ioremap IO in their init 
code they also need the low level firewall.

Some others that cannot be addressed have explicit disables.

> So make them configurable?
Why not just fix the runtime? It's much saner for everyone. Proposing to 
do things at build time sounds like we're in Linux 0.99 days.

-Andi

On Mon, Oct 11, 2021 at 07:59:17AM -0400, Michael S. Tsirkin
wrote:
> On Sun, Oct 10, 2021 at 03:22:39PM -0700, Andi Kleen wrote:
> > 
> > > To which Andi replied
> > > 	One problem with removing the ioremap opt-in is that
> > > 	it's still possible for drivers to get at devices without
going through probe.
> > > 
> > > To which Greg replied:
> > > https://lore.kernel.org/all/YVXBNJ431YIWwZdQ at kroah.com/
> > > 	If there are in-kernel PCI drivers that do not do this, they
need to be
> > > 	fixed today.
> > > 
> > > Can you guys resolve the differences here?
> > 
> > 
> > I addressed this in my other mail, but we may need more discussion.
> 
> Hopefully Greg will reply to that one.
Note, when wanting Greg to reply, someone should at the very least cc:
him.

{sigh}

greg k-h