linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm:D-cache aliasing issue in cow_user_page
@ 2006-10-17  9:15 Dmitriy Monakhov
  2006-10-19  6:33 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitriy Monakhov @ 2006-10-17  9:15 UTC (permalink / raw)
  To: Linux Kernel; +Cc: Linux Memory Management

[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]

 from mm/memory.c:
  1434  static inline void cow_user_page(struct page *dst, struct page *src, unsigned long va)
  1435  {
  1436          /*
  1437           * If the source page was a PFN mapping, we don't have
  1438           * a "struct page" for it. We do a best-effort copy by
  1439           * just copying from the original user address. If that
  1440           * fails, we just zero-fill it. Live with it.
  1441           */
  1442          if (unlikely(!src)) {
  1443                  void *kaddr = kmap_atomic(dst, KM_USER0);
  1444                  void __user *uaddr = (void __user *)(va & PAGE_MASK);
  1445  
  1446                  /*
  1447                   * This really shouldn't fail, because the page is there
  1448                   * in the page tables. But it might just be unreadable,
  1449                   * in which case we just give up and fill the result with
  1450                   * zeroes.
  1451                   */
  1452                  if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE))
  1453                          memset(kaddr, 0, PAGE_SIZE);
  1454                  kunmap_atomic(kaddr, KM_USER0);
  #### D-cache have to be flushed here.
  #### It seems it is just forgotten.

  1455                  return;
  1456                  
  1457          }
  1458          copy_user_highpage(dst, src, va);
  #### Ok here. flush_dcache_page() called from this func if arch need it 
  1459  }

Following is the patch  fix this issue:
Signed-off-by: Dmitriy Monakhov <dmonakhov@openvz.org>
---

[-- Attachment #2: d-cache-aliasing-issue-in-cow-user-page.patch --]
[-- Type: text/plain, Size: 341 bytes --]

diff --git a/mm/memory.c b/mm/memory.c
index b5a4aad..156861f 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1452,6 +1452,7 @@ static inline void cow_user_page(struct 
 		if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE))
 			memset(kaddr, 0, PAGE_SIZE);
 		kunmap_atomic(kaddr, KM_USER0);
+		flush_dcache_page(dst);
 		return;
 		
 	}

[-- Attachment #3: Type: text/plain, Size: 4 bytes --]

---

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm:D-cache aliasing issue in cow_user_page
  2006-10-17  9:15 [PATCH] mm:D-cache aliasing issue in cow_user_page Dmitriy Monakhov
@ 2006-10-19  6:33 ` Andrew Morton
  2006-10-19  7:00   ` David Miller, Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2006-10-19  6:33 UTC (permalink / raw)
  To: Dmitriy Monakhov; +Cc: Linux Kernel, Linux Memory Management

On Tue, 17 Oct 2006 13:15:37 +0400
Dmitriy Monakhov <dmonakhov@openvz.org> wrote:

>  from mm/memory.c:
>   1434  static inline void cow_user_page(struct page *dst, struct page *src, unsigned long va)
>   1435  {
>   1436          /*
>   1437           * If the source page was a PFN mapping, we don't have
>   1438           * a "struct page" for it. We do a best-effort copy by
>   1439           * just copying from the original user address. If that
>   1440           * fails, we just zero-fill it. Live with it.
>   1441           */
>   1442          if (unlikely(!src)) {
>   1443                  void *kaddr = kmap_atomic(dst, KM_USER0);
>   1444                  void __user *uaddr = (void __user *)(va & PAGE_MASK);
>   1445  
>   1446                  /*
>   1447                   * This really shouldn't fail, because the page is there
>   1448                   * in the page tables. But it might just be unreadable,
>   1449                   * in which case we just give up and fill the result with
>   1450                   * zeroes.
>   1451                   */
>   1452                  if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE))
>   1453                          memset(kaddr, 0, PAGE_SIZE);
>   1454                  kunmap_atomic(kaddr, KM_USER0);
>   #### D-cache have to be flushed here.
>   #### It seems it is just forgotten.
> 
>   1455                  return;
>   1456                  
>   1457          }
>   1458          copy_user_highpage(dst, src, va);
>   #### Ok here. flush_dcache_page() called from this func if arch need it 
>   1459  }
> 

This page has just been allocated and is private to the caller - there can
be no userspace mappings of it.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm:D-cache aliasing issue in cow_user_page
  2006-10-19  6:33 ` Andrew Morton
@ 2006-10-19  7:00   ` David Miller, Andrew Morton
  2006-10-19  7:17     ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller, Andrew Morton @ 2006-10-19  7:00 UTC (permalink / raw)
  To: akpm; +Cc: dmonakhov, linux-kernel, linux-mm

> On Tue, 17 Oct 2006 13:15:37 +0400
> Dmitriy Monakhov <dmonakhov@openvz.org> wrote:
> 
> >  from mm/memory.c:
> >   1434  static inline void cow_user_page(struct page *dst, struct page *src, unsigned long va)
> >   1435  {
> >   1436          /*
> >   1437           * If the source page was a PFN mapping, we don't have
> >   1438           * a "struct page" for it. We do a best-effort copy by
> >   1439           * just copying from the original user address. If that
> >   1440           * fails, we just zero-fill it. Live with it.
> >   1441           */
> >   1442          if (unlikely(!src)) {
> >   1443                  void *kaddr = kmap_atomic(dst, KM_USER0);
> >   1444                  void __user *uaddr = (void __user *)(va & PAGE_MASK);
> >   1445  
> >   1446                  /*
> >   1447                   * This really shouldn't fail, because the page is there
> >   1448                   * in the page tables. But it might just be unreadable,
> >   1449                   * in which case we just give up and fill the result with
> >   1450                   * zeroes.
> >   1451                   */
> >   1452                  if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE))
> >   1453                          memset(kaddr, 0, PAGE_SIZE);
> >   1454                  kunmap_atomic(kaddr, KM_USER0);
> >   #### D-cache have to be flushed here.
> >   #### It seems it is just forgotten.
> > 
> >   1455                  return;
> >   1456                  
> >   1457          }
> >   1458          copy_user_highpage(dst, src, va);
> >   #### Ok here. flush_dcache_page() called from this func if arch need it 
> >   1459  }
> > 
> 
> This page has just been allocated and is private to the caller - there can
> be no userspace mappings of it.

Unfortunately, the kernel has just touched the page and thus there are
active cache lines for the kernel side mapping.  When we map this into
user space, userspace might see stale cachelines instead of the
memset() stores.

Architectures typically take care of this in copy_user_page() and
clear_user_page().  The absolutely depend upon those two routines
being used for anonymous pages, and handle the D-cache issues there.
But this code is going outside of that scope, and therefore needs
an explicit D-cache flush.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm:D-cache aliasing issue in cow_user_page
  2006-10-19  7:00   ` David Miller, Andrew Morton
@ 2006-10-19  7:17     ` Andrew Morton
  2006-10-19  7:22       ` David Miller, Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2006-10-19  7:17 UTC (permalink / raw)
  To: David Miller; +Cc: dmonakhov, linux-kernel, linux-mm

On Thu, 19 Oct 2006 00:00:27 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> > >   1452                  if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE))
> > >   1453                          memset(kaddr, 0, PAGE_SIZE);
> > >   1454                  kunmap_atomic(kaddr, KM_USER0);
> > >   #### D-cache have to be flushed here.
> > >   #### It seems it is just forgotten.
> > > 
> > >   1455                  return;
> > >   1456                  
> > >   1457          }
> > >   1458          copy_user_highpage(dst, src, va);
> > >   #### Ok here. flush_dcache_page() called from this func if arch need it 
> > >   1459  }
> > > 
> > 
> > This page has just been allocated and is private to the caller - there can
> > be no userspace mappings of it.
> 
> Unfortunately, the kernel has just touched the page and thus there are
> active cache lines for the kernel side mapping.  When we map this into
> user space, userspace might see stale cachelines instead of the
> memset() stores.

hm.  Has it always been that way or did something change?

> Architectures typically take care of this in copy_user_page() and
> clear_user_page().  The absolutely depend upon those two routines
> being used for anonymous pages, and handle the D-cache issues there.

Only anonymous pages?  There are zillions of places where we modify
pagecache without a flush, especially against the blockdev mapping (fs
metadata).

> But this code is going outside of that scope, and therefore needs
> an explicit D-cache flush.

OK, I'll add the patch.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm:D-cache aliasing issue in cow_user_page
  2006-10-19  7:17     ` Andrew Morton
@ 2006-10-19  7:22       ` David Miller, Andrew Morton
  2006-10-19  7:41         ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller, Andrew Morton @ 2006-10-19  7:22 UTC (permalink / raw)
  To: akpm; +Cc: dmonakhov, linux-kernel, linux-mm

> On Thu, 19 Oct 2006 00:00:27 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
> 
> > Unfortunately, the kernel has just touched the page and thus there are
> > active cache lines for the kernel side mapping.  When we map this into
> > user space, userspace might see stale cachelines instead of the
> > memset() stores.
> 
> hm.  Has it always been that way or did something change?

Always.

> > Architectures typically take care of this in copy_user_page() and
> > clear_user_page().  The absolutely depend upon those two routines
> > being used for anonymous pages, and handle the D-cache issues there.
> 
> Only anonymous pages?  There are zillions of places where we modify
> pagecache without a flush, especially against the blockdev mapping (fs
> metadata).

It's cpu stores that matter, not device DMA and the like, and we have
flush_dcache_page() calls in the correct spots.  You can see that
we take care of this even in places such as the loop driver :-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm:D-cache aliasing issue in cow_user_page
  2006-10-19  7:22       ` David Miller, Andrew Morton
@ 2006-10-19  7:41         ` Andrew Morton
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2006-10-19  7:41 UTC (permalink / raw)
  To: David Miller; +Cc: dmonakhov, linux-kernel, linux-mm

On Thu, 19 Oct 2006 00:22:37 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> > > Architectures typically take care of this in copy_user_page() and
> > > clear_user_page().  The absolutely depend upon those two routines
> > > being used for anonymous pages, and handle the D-cache issues there.
> > 
> > Only anonymous pages?  There are zillions of places where we modify
> > pagecache without a flush, especially against the blockdev mapping (fs
> > metadata).
> 
> It's cpu stores that matter, not device DMA and the like, and we have
> flush_dcache_page() calls in the correct spots.  You can see that
> we take care of this even in places such as the loop driver :-)

grep b_data fs/*/*.c

All of them ;)

It's possible to mmap /dev/hda, but it's a rather odd thing to do (INND
can do it, but not a live filesystem).

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2006-10-19  7:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-17  9:15 [PATCH] mm:D-cache aliasing issue in cow_user_page Dmitriy Monakhov
2006-10-19  6:33 ` Andrew Morton
2006-10-19  7:00   ` David Miller, Andrew Morton
2006-10-19  7:17     ` Andrew Morton
2006-10-19  7:22       ` David Miller, Andrew Morton
2006-10-19  7:41         ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox