Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1240014 > unrolled thread

Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-06 00:50 +0200
    Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X Stephen Hemminger <stephen@networkplumber.org> - 2015-10-06 09:40 +0200
      Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X Avi Kivity <avi@scylladb.com> - 2015-10-06 14:20 +0200
        Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-06 16:10 +0200
          Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X Avi Kivity <avi@scylladb.com> - 2015-10-06 17:50 +0200
        Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X Thomas Monjalon <thomas.monjalon@6wind.com> - 2015-10-16 19:20 +0200
          Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X Stephen Hemminger <stephen@networkplumber.org> - 2015-10-16 19:30 +0200
      Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-06 15:50 +0200
    Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X Vlad Zolotarov <vladz@cloudius-systems.com> - 2015-10-06 10:30 +0200
      Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-06 16:00 +0200
        Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X Vlad Zolotarov <vladz@cloudius-systems.com> - 2015-10-06 16:50 +0200
          Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-06 17:10 +0200
            Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X Vlad Zolotarov <vladz@cloudius-systems.com> - 2015-10-06 18:50 +0200

#1240014 — Re: [dpdk-dev] [PATCH 2/2] uio: new driver to support PCI MSI-X

On Tue, Oct 06, 2015 at 01:09:55AM +0300, Vladislav Zolotarov wrote:
> How about instead of trying to invent the wheel just go and attack the problem
> directly just like i've proposed already a few times in the last days: instead
> of limiting the UIO limit the users that are allowed to use UIO to privileged
> users only (e.g. root). This would solve all clearly unresolvable issues u are
> raising here all together, wouldn't it?

No - root or no root, if the user can modify the addresses in the MSI-X
table and make the chip corrupt random memory, this is IMHO a non-starter.

And tainting kernel is not a solution - your patch adds a pile of
code that either goes completely unused or taints the kernel.
Not just that - it's a dedicated userspace API that either
goes completely unused or taints the kernel.

> >
> > --
> > MST
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1240210

Other than implementation objections, so far the two main arguments
against this reduce to:
  1. If you allow UIO ioctl then it opens an API hook for all the crap out
     of tree UIO drivers to do what they want.
  2. If you allow UIO MSI-X then you are expanding the usage of userspace
     device access in an insecure manner.

Another alternative which I explored was making a version of VFIO that
works without IOMMU. It solves #1 but actually increases the likely negative
response to arguent #2. This would keep same API, and avoid having to
modify UIO. But we would still have the same (if not more resistance)
from IOMMU developers who believe all systems have to be secure against
root.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240401

On 10/06/2015 10:33 AM, Stephen Hemminger wrote:
> Other than implementation objections, so far the two main arguments
> against this reduce to:
>    1. If you allow UIO ioctl then it opens an API hook for all the crap out
>       of tree UIO drivers to do what they want.
>    2. If you allow UIO MSI-X then you are expanding the usage of userspace
>       device access in an insecure manner.
>
> Another alternative which I explored was making a version of VFIO that
> works without IOMMU. It solves #1 but actually increases the likely negative
> response to arguent #2. This would keep same API, and avoid having to
> modify UIO. But we would still have the same (if not more resistance)
> from IOMMU developers who believe all systems have to be secure against
> root.

vfio's charter was explicitly aiming for modern setups with iommus.

This could be revisited, but I agree it will have even more resistance, 
justified IMO.

btw, (2) doesn't really add any insecurity.  The user could already poke 
at the msix tables (as well as perform DMA); they just couldn't get a 
useful interrupt out of them.

Maybe a module parameter "allow_insecure_dma" can be added to 
uio_pci_generic.  Without the parameter, bus mastering and msix is 
disabled, with the parameter it is allowed.  This requires the sysadmin 
to take a positive step in order to make use of their hardware.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240452

On Tue, Oct 06, 2015 at 03:15:57PM +0300, Avi Kivity wrote:
> btw, (2) doesn't really add any insecurity.  The user could already poke at
> the msix tables (as well as perform DMA); they just couldn't get a useful
> interrupt out of them.

Poking at msix tables won't cause memory corruption unless msix and bus
mastering is enabled.  It's true root can enable msix and bus mastering
through sysfs - but that's easy to block or detect. Even if you don't
buy a security story, it seems less likely to trigger as a result
of a userspace bug.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240614

On 10/06/2015 05:07 PM, Michael S. Tsirkin wrote:
> On Tue, Oct 06, 2015 at 03:15:57PM +0300, Avi Kivity wrote:
>> btw, (2) doesn't really add any insecurity.  The user could already poke at
>> the msix tables (as well as perform DMA); they just couldn't get a useful
>> interrupt out of them.
> Poking at msix tables won't cause memory corruption unless msix and bus
> mastering is enabled.

It's a given that bus mastering is enabled.  It's true that msix is 
unlikely to be enabled, unless msix support is added.

>    It's true root can enable msix and bus mastering
> through sysfs - but that's easy to block or detect. Even if you don't
> buy a security story, it seems less likely to trigger as a result
> of a userspace bug.

If you're doing DMA, that's the least of your worries.

Still, zero-mapping the msix space seems reasonable, and can protect 
userspace from silly stuff.  It can't be considered to have anything to 
do with security though, as long as users can simply DMA to every bit of 
RAM in the system they want to.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249011

To sum it up,
We want to remove the need of the out-of-tree module igb_uio.
3 possible implementations were discussed so far:
- new UIO driver
- extend uio_pci_generic
- VFIO without IOMMU

It is preferred to avoid creating yet another module to support.
That's why the uio_pci_generic extension would be nice.
In my understanding, there are currently 2 issues with the patches
from Vlad and Stephen:
- IRQ must be mapped to a fd without using a new ioctl
- MSI-X handling in userspace breaks the memory protection

I'm confident the first issue can be fixed with something like sysfs.
About the "security" concern, mainly expressed by MST, I think the idea
of Avi (below) deserves to be discussed.

2015-10-06 15:15, Avi Kivity:
> On 10/06/2015 10:33 AM, Stephen Hemminger wrote:
> > Other than implementation objections, so far the two main arguments
> > against this reduce to:
> >    1. If you allow UIO ioctl then it opens an API hook for all the crap out
> >       of tree UIO drivers to do what they want.
> >    2. If you allow UIO MSI-X then you are expanding the usage of userspace
> >       device access in an insecure manner.
[...]
> btw, (2) doesn't really add any insecurity.  The user could already poke 
> at the msix tables (as well as perform DMA); they just couldn't get a 
> useful interrupt out of them.
> 
> Maybe a module parameter "allow_insecure_dma" can be added to 
> uio_pci_generic.  Without the parameter, bus mastering and msix is 
> disabled, with the parameter it is allowed.  This requires the sysadmin 
> to take a positive step in order to make use of their hardware.

Giving the control of the memory protection level to the distribution or
the administrator looks a good idea.
When allowing insecure DMA, a log will make clear how it is supported
-or not- by the system provider.

From another thread:
2015-10-01 14:09, Michael S. Tsirkin:
> If Linux keeps enabling hacks, no one will bother doing the right thing.
> Upstream inclusion is the only carrot Linux has to make people do the
> right thing.

The "right thing" should be guided by the users needs at a given time.
The "carrot" for a better solution will be to have a well protected system.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249016

On Fri, 16 Oct 2015 19:11:35 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> To sum it up,
> We want to remove the need of the out-of-tree module igb_uio.
> 3 possible implementations were discussed so far:
> - new UIO driver
> - extend uio_pci_generic
> - VFIO without IOMMU

There is recent progress on VFIO without IOMMU. This looks the most promising
long term solution.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240439

On Tue, Oct 06, 2015 at 08:33:56AM +0100, Stephen Hemminger wrote:
> Other than implementation objections, so far the two main arguments
> against this reduce to:
>   1. If you allow UIO ioctl then it opens an API hook for all the crap out
>      of tree UIO drivers to do what they want.
>   2. If you allow UIO MSI-X then you are expanding the usage of userspace
>      device access in an insecure manner.

That's not all. Without MSI one can detect insecure usage by detecting
userspace enabling bus mastering.  This can be detected simply using
lspci.  Or one can also imagine a configuration where this ability is
disabled, is logged, or taints kernel.  This seems like something that
might be worth having for some locked-down systems.

OTOH enabling MSI requires enabling bus mastering so suddenly we have no
idea whether device can be/is used in a safe way.

> 
> Another alternative which I explored was making a version of VFIO that
> works without IOMMU. It solves #1 but actually increases the likely negative
> response to arguent #2.

No - because VFIO has limited protection against device misuse by
userspace, by limiting access to sub-ranges of device BARs and config
space.  For a device that doesn't do DMA, that will be enough to make it
secure to use.

That's a pretty weak excuse to support userspace drivers for PCI devices
without an IOMMU, but it's the best I heard so far.

Is that worth the security trade-off? I'm still not sure.

> This would keep same API, and avoid having to
> modify UIO. But we would still have the same (if not more resistance)
> from IOMMU developers who believe all systems have to be secure against
> root.

"Secure against root" is a confusing way to put it IMHO. We are talking
about memory protection.

So that's not IOMMU developers IIUC. I believe most kernel developers will
agree it's not a good idea to let userspace corrupt kernel memory.
Otherwise, the driver can't be supported, and maintaining upstream
drivers that can't be supported serves no useful purpose.  Anyone can
load out of tree ones just as well.

VFIO already supports MSI so VFIO developers already have a lot of
experience with these issues. Getting their input would be valuable.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240244

On 10/06/15 01:49, Michael S. Tsirkin wrote:
> On Tue, Oct 06, 2015 at 01:09:55AM +0300, Vladislav Zolotarov wrote:
>> How about instead of trying to invent the wheel just go and attack the problem
>> directly just like i've proposed already a few times in the last days: instead
>> of limiting the UIO limit the users that are allowed to use UIO to privileged
>> users only (e.g. root). This would solve all clearly unresolvable issues u are
>> raising here all together, wouldn't it?
> No - root or no root, if the user can modify the addresses in the MSI-X
> table and make the chip corrupt random memory, this is IMHO a non-starter.

Michael, how this or any other related patch is related to the problem u 
r describing? The above ability is there for years and if memory serves 
me well it was u who wrote uio_pci_generic with this "security flaw".  ;)

This patch in general only adds the ability to receive notifications per 
MSI-X interrupt and it has nothing to do with the ability to reprogram 
the MSI-X related registers from the user space which was always there.

>
> And tainting kernel is not a solution - your patch adds a pile of
> code that either goes completely unused or taints the kernel.
> Not just that - it's a dedicated userspace API that either
> goes completely unused or taints the kernel.
>
>>> --
>>> MST

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240451

On Tue, Oct 06, 2015 at 11:23:11AM +0300, Vlad Zolotarov wrote:
> Michael, how this or any other related patch is related to the problem u r
> describing?
> The above ability is there for years and if memory serves me
> well it was u who wrote uio_pci_generic with this "security flaw".  ;)

I answered all this already.

This patch enables bus mastering, enables MSI or MSI-X, and requires
userspace to map the MSI-X table and read/write the config space.
This means that a single userspace bug is enough to corrupt kernel
memory.

uio_pci_generic does not enable bus mastering or MSI, and
it might be a good idea to have uio_pci_generic block
access to MSI/MSI-X config.
-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240511

On 10/06/15 16:58, Michael S. Tsirkin wrote:
> On Tue, Oct 06, 2015 at 11:23:11AM +0300, Vlad Zolotarov wrote:
>> Michael, how this or any other related patch is related to the problem u r
>> describing?
>> The above ability is there for years and if memory serves me
>> well it was u who wrote uio_pci_generic with this "security flaw".  ;)
> I answered all this already.
>
> This patch enables bus mastering, enables MSI or MSI-X

This may be done from the user space right now without this patch...

> , and requires
> userspace to map the MSI-X table

Hmmm... I must have missed this requirement. Could u, pls., clarify? 
 From what I see, MSI/MSI-X table is configured completely in the kernel 
here...

> and read/write the config space.
> This means that a single userspace bug is enough to corrupt kernel
> memory.

Could u, pls., provide and example of this simple bug? Because it's 
absolutely not obvious...

>
> uio_pci_generic does not enable bus mastering or MSI, and
> it might be a good idea to have uio_pci_generic block
> access to MSI/MSI-X config.

Since device bars may be mapped bypassing the UIO/uio_pci_generic - this 
won't solve any issue.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240528

On Tue, Oct 06, 2015 at 05:49:21PM +0300, Vlad Zolotarov wrote:
> >and read/write the config space.
> >This means that a single userspace bug is enough to corrupt kernel
> >memory.
> 
> Could u, pls., provide and example of this simple bug? Because it's
> absolutely not obvious...

Stick a value that happens to match a kernel address in Msg Addr field
in an unmasked MSI-X entry.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1240725

On 10/06/15 18:00, Michael S. Tsirkin wrote:
> On Tue, Oct 06, 2015 at 05:49:21PM +0300, Vlad Zolotarov wrote:
>>> and read/write the config space.
>>> This means that a single userspace bug is enough to corrupt kernel
>>> memory.
>> Could u, pls., provide and example of this simple bug? Because it's
>> absolutely not obvious...
> Stick a value that happens to match a kernel address in Msg Addr field
> in an unmasked MSI-X entry.

This patch neither configures MSI-X entries in the user space nor 
provides additional means to do so therefore this "sticking" would be a 
matter of some extra code that is absolutely unrelated to this patch. 
So, this example seems absolutely irrelevant to this particular discussion.

thanks,
vlad

>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web