* [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