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

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

[Will Deacon]

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 <will.deacon@arm.com>
---
 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

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

[Will Deacon]

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 <will.deacon@arm.com>
---
 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

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

[Will Deacon]

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 <rusty@rustcorp.com.au>
Cc: Sasha Levin <levinsasha928@gmail.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 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

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

[Will Deacon]

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 <rusty@rustcorp.com.au>
Cc: Sasha Levin <levinsasha928@gmail.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 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

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

[Andrew Morton]

On Fri, 19 Oct 2012 14:03:32 +0100
Will Deacon <will.deacon@arm.com> 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?

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

[Andrew Morton]

On Fri, 19 Oct 2012 14:03:32 +0100
Will Deacon <will.deacon@arm.com> 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?

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

[Will Deacon]

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 <rusty@rustcorp.com.au>
Cc: Sasha Levin <levinsasha928@gmail.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 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

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

[Will Deacon]

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 <rusty@rustcorp.com.au>
Cc: Sasha Levin <levinsasha928@gmail.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 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

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

[Andrew Morton]

On Fri, 19 Oct 2012 14:03:31 +0100
Will Deacon <will.deacon@arm.com> 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 <will.deacon@arm.com>
> ---
>  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.

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

[Andrew Morton]

On Fri, 19 Oct 2012 14:03:31 +0100
Will Deacon <will.deacon@arm.com> 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 <will.deacon@arm.com>
> ---
>  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.

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

[Rusty Russell]

Will Deacon <will.deacon@arm.com> 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.

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

[Rusty Russell]

Will Deacon <will.deacon@arm.com> 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.

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

[Will Deacon]

On Tue, Oct 23, 2012 at 12:55:57AM +0100, Rusty Russell wrote:
> Will Deacon <will.deacon@arm.com> 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

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

[Will Deacon]

On Tue, Oct 23, 2012 at 12:55:57AM +0100, Rusty Russell wrote:
> Will Deacon <will.deacon@arm.com> 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