[PATCH] mm:D-cache aliasing issue in cow_user_page

Dmitriy Monakhov

author

2006-10-17 09:15 UTC #0 lore raw

PATCH 1/1 mm:D-cache aliasing issue in cow_user_page

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 <[email protected]>

---

mm/memory.c +1 -0

index b5a4aad..156861f 100644
@@ -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;
 		
 	}

---

Andrew Morton

2006-10-19 06:33 UTC #1 lore raw

Quoted 25 lines · On Tue, 17 Oct 2006 13:15:37 +0400 Dmitriy Monakhov <[email protected]>…

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.

Quoted 7 lines

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Miller

2006-10-19 07:00 UTC #2 lore raw

Quoted 36 lines

Quoted 25 lines · On Tue, 17 Oct 2006 13:15:37 +0400 Dmitriy Monakhov <[email protected]>…
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.
Quoted 7 lines
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 }

Quoted 2 lines

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Andrew Morton

2006-10-19 07:17 UTC #3 lore raw

Quoted 14 lines · On Thu, 19 Oct 2006 00:00:27 -0700 (PDT) David Miller <[email protected]>…

Quoted 14 lines
Quoted 13 lines
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 }

Quoted 7 lines

Quoted 2 lines
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?

Quoted 3 lines

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).

Quoted 2 lines

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Miller

2006-10-19 07:22 UTC #4 lore raw

Quoted 9 lines

Quoted 4 lines · On Thu, 19 Oct 2006 00:00:27 -0700 (PDT) David Miller <[email protected]>…
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.

Quoted 7 lines

Quoted 3 lines
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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Andrew Morton

2006-10-19 07:41 UTC #5 lore raw

Quoted 11 lines · On Thu, 19 Oct 2006 00:22:37 -0700 (PDT) David Miller <[email protected]>…

Quoted 7 lines
Quoted 3 lines
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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>