2012-10-19 13:03:53

by Will Deacon

[permalink] [raw]
Subject: [PATCH v2 1/3] mm: highmem: export kmap_to_page for modules

Some virtio device drivers (9p) need to translate high virtual addresses
to physical addresses, which are inserted into the virtqueue for
processing by userspace.

This patch exports the kmap_to_page symbol, so that the affected drivers
can be compiled as modules.

Signed-off-by: Will Deacon <[email protected]>
---
mm/highmem.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/mm/highmem.c b/mm/highmem.c
index d517cd1..2a07f97 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -105,6 +105,7 @@ struct page *kmap_to_page(void *vaddr)

return virt_to_page(addr);
}
+EXPORT_SYMBOL(kmap_to_page);

static void flush_all_zero_pkmaps(void)
{
--
1.7.4.1


2012-10-19 13:03:52

by Will Deacon

[permalink] [raw]
Subject: [PATCH v2 2/3] virtio: 9p: correctly pass physical address to userspace for high pages

When using a virtio transport, the 9p net device may pass the physical
address of a kernel buffer to userspace via a scatterlist inside a
virtqueue. If the kernel buffer is mapped outside of the linear mapping
(e.g. highmem), then virt_to_page will return a bogus value and we will
populate the scatterlist with junk.

This patch uses kmap_to_page when populating the page array for a kernel
buffer.

Cc: Rusty Russell <[email protected]>
Cc: Sasha Levin <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
---
net/9p/trans_virtio.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 35b8911..fd05c81 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -39,6 +39,7 @@
#include <linux/inet.h>
#include <linux/idr.h>
#include <linux/file.h>
+#include <linux/highmem.h>
#include <linux/slab.h>
#include <net/9p/9p.h>
#include <linux/parser.h>
@@ -325,7 +326,7 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
int count = nr_pages;
while (nr_pages) {
s = rest_of_page(data);
- pages[index++] = virt_to_page(data);
+ pages[index++] = kmap_to_page(data);
data += s;
nr_pages--;
}
--
1.7.4.1

2012-10-19 13:03:51

by Will Deacon

[permalink] [raw]
Subject: [PATCH v2 3/3] virtio: force vring descriptors to be allocated from lowmem

Virtio devices may attempt to add descriptors to a virtqueue from atomic
context using GFP_ATOMIC allocation. This is problematic because such
allocations can fall outside of the lowmem mapping, causing virt_to_phys
to report bogus physical addresses which are subsequently passed to
userspace via the buffers for the virtual device.

This patch masks out __GFP_HIGH and __GFP_HIGHMEM from the requested
flags when allocating descriptors for a virtqueue. If an atomic
allocation is requested and later fails, we will return -ENOSPC which
will be handled by the driver.

Cc: Rusty Russell <[email protected]>
Cc: Sasha Levin <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
---
drivers/virtio/virtio_ring.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index e639584..286c30c 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -135,6 +135,13 @@ static int vring_add_indirect(struct vring_virtqueue *vq,
unsigned head;
int i;

+ /*
+ * We require lowmem mappings for the descriptors because
+ * otherwise virt_to_phys will give us bogus addresses in the
+ * virtqueue.
+ */
+ gfp &= ~(__GFP_HIGHMEM | __GFP_HIGH);
+
desc = kmalloc((out + in) * sizeof(struct vring_desc), gfp);
if (!desc)
return -ENOMEM;
--
1.7.4.1

2012-10-22 22:05:33

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] virtio: 9p: correctly pass physical address to userspace for high pages

On Fri, 19 Oct 2012 14:03:32 +0100
Will Deacon <[email protected]> wrote:

> When using a virtio transport, the 9p net device may pass the physical
> address of a kernel buffer to userspace via a scatterlist inside a
> virtqueue. If the kernel buffer is mapped outside of the linear mapping
> (e.g. highmem), then virt_to_page will return a bogus value and we will
> populate the scatterlist with junk.
>
> This patch uses kmap_to_page when populating the page array for a kernel
> buffer.
>
> ...
>
> --- a/net/9p/trans_virtio.c
> +++ b/net/9p/trans_virtio.c
> @@ -39,6 +39,7 @@
> #include <linux/inet.h>
> #include <linux/idr.h>
> #include <linux/file.h>
> +#include <linux/highmem.h>
> #include <linux/slab.h>
> #include <net/9p/9p.h>
> #include <linux/parser.h>
> @@ -325,7 +326,7 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
> int count = nr_pages;
> while (nr_pages) {
> s = rest_of_page(data);
> - pages[index++] = virt_to_page(data);
> + pages[index++] = kmap_to_page(data);
> data += s;
> nr_pages--;

I am suspecting that this code has been busted for a while on x86
highmem, but nobody noticed. True or false? If "true" then I expect
that a -stable backport is appropriate?

2012-10-22 22:08:20

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] mm: highmem: export kmap_to_page for modules

On Fri, 19 Oct 2012 14:03:31 +0100
Will Deacon <[email protected]> wrote:

> Some virtio device drivers (9p) need to translate high virtual addresses
> to physical addresses, which are inserted into the virtqueue for
> processing by userspace.
>
> This patch exports the kmap_to_page symbol, so that the affected drivers
> can be compiled as modules.
>
> Signed-off-by: Will Deacon <[email protected]>
> ---
> mm/highmem.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/mm/highmem.c b/mm/highmem.c
> index d517cd1..2a07f97 100644
> --- a/mm/highmem.c
> +++ b/mm/highmem.c
> @@ -105,6 +105,7 @@ struct page *kmap_to_page(void *vaddr)
>
> return virt_to_page(addr);
> }
> +EXPORT_SYMBOL(kmap_to_page);
>

Looks OK to me. Would generally prefer that an exported-to-modules
symbol have some documentation, but I guess this one is obvious enough.

Rusty, please include this if you grab the rest of the series.

2012-10-23 02:00:58

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] mm: highmem: export kmap_to_page for modules

Will Deacon <[email protected]> writes:

> Some virtio device drivers (9p) need to translate high virtual addresses
> to physical addresses, which are inserted into the virtqueue for
> processing by userspace.
>
> This patch exports the kmap_to_page symbol, so that the affected drivers
> can be compiled as modules.

Thanks Will!

I've applied these, and add cc:stable. I'll push them to Linus after a
little time in linux-next.

Cheers,
Rusty.

2012-10-23 10:34:28

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] mm: highmem: export kmap_to_page for modules

On Tue, Oct 23, 2012 at 12:55:57AM +0100, Rusty Russell wrote:
> Will Deacon <[email protected]> writes:
>
> > Some virtio device drivers (9p) need to translate high virtual addresses
> > to physical addresses, which are inserted into the virtqueue for
> > processing by userspace.
> >
> > This patch exports the kmap_to_page symbol, so that the affected drivers
> > can be compiled as modules.
>
> Thanks Will!
>
> I've applied these, and add cc:stable. I'll push them to Linus after a
> little time in linux-next.

Cheers Rusty. Andrew's right about this affecting highmem on x86 too (I
guess that's what Sasha saw with the 9p memory issues), so stable makes
sense.

Will