* [RESEND PATCH 1/1] vfio/nvlink: Remove exec permission to avoid SELinux AVCs
@ 2020-05-18 15:05 Leonardo Bras
2020-05-26 20:43 ` Alex Williamson
0 siblings, 1 reply; 4+ messages in thread
From: Leonardo Bras @ 2020-05-18 15:05 UTC (permalink / raw)
To: Alex Williamson, Alexey Kardashevskiy; +Cc: Leonardo Bras, qemu-devel, dgibson
If SELinux is setup without 'execmem' permission for qemu, all mmap
with (PROT_WRITE | PROT_EXEC) will fail and print a warning in
SELinux log.
If "nvlink2-mr" memory allocation fails (fist diff), it will cause
guest NUMA nodes to not be correctly configured (V100 memory will
not be visible for guest, nor its NUMA nodes).
Not having 'execmem' permission is intesting for virtual machines to
avoid buffer-overflow based attacks, and it's adopted in distros
like RHEL.
So, removing the PROT_EXEC flag seems the right thing to do.
Browsing some other code that mmaps memory for usage with
memory_region_init_ram_device_ptr, I could notice it's usual to
not have PROT_EXEC (only PROT_READ | PROT_WRITE), so it should be
no problem around this.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
- Alexey's review is here: https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg00006.html
hw/vfio/pci-quirks.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 2d348f8237..124d4f57e1 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1620,7 +1620,7 @@ int vfio_pci_nvidia_v100_ram_init(VFIOPCIDevice *vdev, Error **errp)
}
cap = (void *) hdr;
- p = mmap(NULL, nv2reg->size, PROT_READ | PROT_WRITE | PROT_EXEC,
+ p = mmap(NULL, nv2reg->size, PROT_READ | PROT_WRITE,
MAP_SHARED, vdev->vbasedev.fd, nv2reg->offset);
if (p == MAP_FAILED) {
ret = -errno;
@@ -1680,7 +1680,7 @@ int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error **errp)
/* Some NVLink bridges may not have assigned ATSD */
if (atsdreg->size) {
- p = mmap(NULL, atsdreg->size, PROT_READ | PROT_WRITE | PROT_EXEC,
+ p = mmap(NULL, atsdreg->size, PROT_READ | PROT_WRITE,
MAP_SHARED, vdev->vbasedev.fd, atsdreg->offset);
if (p == MAP_FAILED) {
ret = -errno;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RESEND PATCH 1/1] vfio/nvlink: Remove exec permission to avoid SELinux AVCs
2020-05-18 15:05 [RESEND PATCH 1/1] vfio/nvlink: Remove exec permission to avoid SELinux AVCs Leonardo Bras
@ 2020-05-26 20:43 ` Alex Williamson
2020-05-27 0:42 ` David Gibson
0 siblings, 1 reply; 4+ messages in thread
From: Alex Williamson @ 2020-05-26 20:43 UTC (permalink / raw)
To: Leonardo Bras; +Cc: Alexey Kardashevskiy, dgibson, qemu-devel
On Mon, 18 May 2020 12:05:24 -0300
Leonardo Bras <leobras.c@gmail.com> wrote:
> If SELinux is setup without 'execmem' permission for qemu, all mmap
> with (PROT_WRITE | PROT_EXEC) will fail and print a warning in
> SELinux log.
>
> If "nvlink2-mr" memory allocation fails (fist diff), it will cause
> guest NUMA nodes to not be correctly configured (V100 memory will
> not be visible for guest, nor its NUMA nodes).
>
> Not having 'execmem' permission is intesting for virtual machines to
> avoid buffer-overflow based attacks, and it's adopted in distros
> like RHEL.
>
> So, removing the PROT_EXEC flag seems the right thing to do.
>
> Browsing some other code that mmaps memory for usage with
> memory_region_init_ram_device_ptr, I could notice it's usual to
> not have PROT_EXEC (only PROT_READ | PROT_WRITE), so it should be
> no problem around this.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> ---
Seems David Gibson might be in a position to send a pull request
including this before I can, so:
Acked-by: Alex Williamson <alex.williamson@redhat.com>
> - Alexey's review is here: https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg00006.html
>
> hw/vfio/pci-quirks.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index 2d348f8237..124d4f57e1 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -1620,7 +1620,7 @@ int vfio_pci_nvidia_v100_ram_init(VFIOPCIDevice *vdev, Error **errp)
> }
> cap = (void *) hdr;
>
> - p = mmap(NULL, nv2reg->size, PROT_READ | PROT_WRITE | PROT_EXEC,
> + p = mmap(NULL, nv2reg->size, PROT_READ | PROT_WRITE,
> MAP_SHARED, vdev->vbasedev.fd, nv2reg->offset);
> if (p == MAP_FAILED) {
> ret = -errno;
> @@ -1680,7 +1680,7 @@ int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error **errp)
>
> /* Some NVLink bridges may not have assigned ATSD */
> if (atsdreg->size) {
> - p = mmap(NULL, atsdreg->size, PROT_READ | PROT_WRITE | PROT_EXEC,
> + p = mmap(NULL, atsdreg->size, PROT_READ | PROT_WRITE,
> MAP_SHARED, vdev->vbasedev.fd, atsdreg->offset);
> if (p == MAP_FAILED) {
> ret = -errno;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RESEND PATCH 1/1] vfio/nvlink: Remove exec permission to avoid SELinux AVCs
2020-05-26 20:43 ` Alex Williamson
@ 2020-05-27 0:42 ` David Gibson
0 siblings, 0 replies; 4+ messages in thread
From: David Gibson @ 2020-05-27 0:42 UTC (permalink / raw)
To: Alex Williamson; +Cc: Alexey Kardashevskiy, Leonardo Bras, qemu-devel, dgibson
[-- Attachment #1: Type: text/plain, Size: 2807 bytes --]
On Tue, May 26, 2020 at 02:43:43PM -0600, Alex Williamson wrote:
> On Mon, 18 May 2020 12:05:24 -0300
> Leonardo Bras <leobras.c@gmail.com> wrote:
>
> > If SELinux is setup without 'execmem' permission for qemu, all mmap
> > with (PROT_WRITE | PROT_EXEC) will fail and print a warning in
> > SELinux log.
> >
> > If "nvlink2-mr" memory allocation fails (fist diff), it will cause
> > guest NUMA nodes to not be correctly configured (V100 memory will
> > not be visible for guest, nor its NUMA nodes).
> >
> > Not having 'execmem' permission is intesting for virtual machines to
> > avoid buffer-overflow based attacks, and it's adopted in distros
> > like RHEL.
> >
> > So, removing the PROT_EXEC flag seems the right thing to do.
> >
> > Browsing some other code that mmaps memory for usage with
> > memory_region_init_ram_device_ptr, I could notice it's usual to
> > not have PROT_EXEC (only PROT_READ | PROT_WRITE), so it should be
> > no problem around this.
> >
> > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >
> > ---
>
> Seems David Gibson might be in a position to send a pull request
> including this before I can, so:
Merged to ppc-for-5.1, thanks.
>
> Acked-by: Alex Williamson <alex.williamson@redhat.com>
>
>
> > - Alexey's review is here: https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg00006.html
> >
> > hw/vfio/pci-quirks.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> > index 2d348f8237..124d4f57e1 100644
> > --- a/hw/vfio/pci-quirks.c
> > +++ b/hw/vfio/pci-quirks.c
> > @@ -1620,7 +1620,7 @@ int vfio_pci_nvidia_v100_ram_init(VFIOPCIDevice *vdev, Error **errp)
> > }
> > cap = (void *) hdr;
> >
> > - p = mmap(NULL, nv2reg->size, PROT_READ | PROT_WRITE | PROT_EXEC,
> > + p = mmap(NULL, nv2reg->size, PROT_READ | PROT_WRITE,
> > MAP_SHARED, vdev->vbasedev.fd, nv2reg->offset);
> > if (p == MAP_FAILED) {
> > ret = -errno;
> > @@ -1680,7 +1680,7 @@ int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error **errp)
> >
> > /* Some NVLink bridges may not have assigned ATSD */
> > if (atsdreg->size) {
> > - p = mmap(NULL, atsdreg->size, PROT_READ | PROT_WRITE | PROT_EXEC,
> > + p = mmap(NULL, atsdreg->size, PROT_READ | PROT_WRITE,
> > MAP_SHARED, vdev->vbasedev.fd, atsdreg->offset);
> > if (p == MAP_FAILED) {
> > ret = -errno;
> >
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* [RESEND PATCH 1/1] vfio/nvlink: Remove exec permission to avoid SELinux AVCs
@ 2020-05-01 8:11 Leonardo Bras
0 siblings, 0 replies; 4+ messages in thread
From: Leonardo Bras @ 2020-05-01 8:11 UTC (permalink / raw)
To: Alex Williamson, Alexey Kardashevskiy; +Cc: Leonardo Bras, qemu-devel
If SELinux is setup without 'execmem' permission for qemu, all mmap
with (PROT_WRITE | PROT_EXEC) will fail and print a warning in
SELinux log.
If "nvlink2-mr" memory allocation fails (fist diff), it will cause
guest NUMA nodes to not be correctly configured (V100 memory will
not be visible for guest, nor its NUMA nodes).
Not having 'execmem' permission is intesting for virtual machines to
avoid buffer-overflow based attacks, and it's adopted in distros
like RHEL.
So, removing the PROT_EXEC flag seems the right thing to do.
Browsing some other code that mmaps memory for usage with
memory_region_init_ram_device_ptr, I could notice it's usual to
not have PROT_EXEC (only PROT_READ | PROT_WRITE), so it should be
no problem around this.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
- Resend because it, for some reason, did not hit the list.
- Alexey's review is here: https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg00006.html
hw/vfio/pci-quirks.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 2d348f8237..124d4f57e1 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1620,7 +1620,7 @@ int vfio_pci_nvidia_v100_ram_init(VFIOPCIDevice *vdev, Error **errp)
}
cap = (void *) hdr;
- p = mmap(NULL, nv2reg->size, PROT_READ | PROT_WRITE | PROT_EXEC,
+ p = mmap(NULL, nv2reg->size, PROT_READ | PROT_WRITE,
MAP_SHARED, vdev->vbasedev.fd, nv2reg->offset);
if (p == MAP_FAILED) {
ret = -errno;
@@ -1680,7 +1680,7 @@ int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error **errp)
/* Some NVLink bridges may not have assigned ATSD */
if (atsdreg->size) {
- p = mmap(NULL, atsdreg->size, PROT_READ | PROT_WRITE | PROT_EXEC,
+ p = mmap(NULL, atsdreg->size, PROT_READ | PROT_WRITE,
MAP_SHARED, vdev->vbasedev.fd, atsdreg->offset);
if (p == MAP_FAILED) {
ret = -errno;
--
2.25.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-27 3:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-18 15:05 [RESEND PATCH 1/1] vfio/nvlink: Remove exec permission to avoid SELinux AVCs Leonardo Bras
2020-05-26 20:43 ` Alex Williamson
2020-05-27 0:42 ` David Gibson
-- strict thread matches above, loose matches on Subject: below --
2020-05-01 8:11 Leonardo Bras
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).