The SOFT_DIRTY bit shows that the content of memory was changed
after a defined point in the past. mprotect() doesn't change the
content of memory, so it must not change the SOFT_DIRTY bit.
This patch does nothing with _PAGE_SWP_SOFT_DIRTY, becase pte_modify()
is called only for present pages.
Fixes: 0f8975ec4db2 (mm: soft-dirty bits for user memory changes tracking)
Acked-by: Cyrill Gorcunov <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Pavel Emelyanov <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Wen Congyang <[email protected]>
Signed-off-by: Andrey Vagin <[email protected]>
---
arch/x86/include/asm/pgtable_types.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index a83aa44..1aa9ccd 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -121,7 +121,8 @@
/* Set of bits not changed in pte_modify */
#define _PAGE_CHG_MASK (PTE_PFN_MASK | _PAGE_PCD | _PAGE_PWT | \
- _PAGE_SPECIAL | _PAGE_ACCESSED | _PAGE_DIRTY)
+ _PAGE_SPECIAL | _PAGE_ACCESSED | _PAGE_DIRTY | \
+ _PAGE_SOFT_DIRTY)
#define _HPAGE_CHG_MASK (_PAGE_CHG_MASK | _PAGE_PSE)
#define _PAGE_CACHE_MASK (_PAGE_PCD | _PAGE_PWT)
--
1.8.5.3
On Thu, 30 Jan 2014 00:04:59 +0400 Andrey Vagin <[email protected]> wrote:
> The SOFT_DIRTY bit shows that the content of memory was changed
> after a defined point in the past. mprotect() doesn't change the
> content of memory, so it must not change the SOFT_DIRTY bit.
>
> This patch does nothing with _PAGE_SWP_SOFT_DIRTY, becase pte_modify()
> is called only for present pages.
Standard complaint: when fixing a bug, please describe the end-user
visible effects of that bug.
afaict the effects are minor: snapshotting will save/copy more pages
than it needs to, but there will be no malfunction. Hence a -stable
backport is unneeded. Agree?
On Wed, Jan 29, 2014 at 12:18:59PM -0800, Andrew Morton wrote:
> On Thu, 30 Jan 2014 00:04:59 +0400 Andrey Vagin <[email protected]> wrote:
>
> > The SOFT_DIRTY bit shows that the content of memory was changed
> > after a defined point in the past. mprotect() doesn't change the
> > content of memory, so it must not change the SOFT_DIRTY bit.
> >
> > This patch does nothing with _PAGE_SWP_SOFT_DIRTY, becase pte_modify()
> > is called only for present pages.
>
> Standard complaint: when fixing a bug, please describe the end-user
> visible effects of that bug.
>
> afaict the effects are minor: snapshotting will save/copy more pages
> than it needs to, but there will be no malfunction. Hence a -stable
> backport is unneeded. Agree?
There will be malfunction. On the first iteration all pages are dumped.
On other iterations only pages with the SOFT_DIRTY bit are dumped. So if
the SOFT_DIRTY bit is cleared from a page by mistake, the page is not
dumped and its content will be restored incorrectly.
This patch should be cc'ed to stable. Sorry forgot to do this in first
place.
The following program is able to detect the problem:
#include <sys/mman.h>
#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#define PME_SOFT_DIRTY (1ULL << 55)
int main()
{
unsigned long long v;
void *p;
int fd;
p = mmap(NULL, 4096, PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
if (p == MAP_FAILED)
return -1;
fd = open("/proc/self/clear_refs", O_WRONLY);
if (fd < 0)
return 1;
if (write(fd, "4\n", 2) < 2)
return 1;
close(fd);
*((int *) p) = 5;
fd = open("/proc/self/pagemap", O_RDONLY);
if (fd < 0)
return 1;
mprotect(p, 4096, PROT_READ);
lseek(fd, (unsigned long) p / 4096 * 8, SEEK_SET);
read(fd, &v, sizeof(v));
close(fd);
printf("%8llx\n", v);
if (v & PME_SOFT_DIRTY) {
printf("PASS\n");
return 0;
}
printf("FAIL\n");
return 1;
}