lkml.org 
[lkml]   [2005]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Subject[patch 39/39] remap_file_pages protection support: wrong "historical" code for review - 2
From
Date

From: Ingo Molnar <mingo@elte.hu>, Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>

This "fast-path" was contained in the original
remap-file-pages-prot-2.6.4-rc1-mm1-A1.patch from Ingo Molnar; I think this
code is wrong, but I'm sending it for review anyway, because I'm unsure (and
in fact, in the end I found the reason for this).

I guess this code is intended for when we're called by sys_remap_file_page,
without altering pgoffset or protections (otherwise we'd refuse operation on a
private mapping). This cannot happen with mmap(MAP_POPULATE) because we clear
old mappings. And the code makes sense only if we COW'ed a page, because
otherwise the old mapping is already correct. I'm not sure whether we should
fail here - maybe skipping the PTE would be more appropriate. Or we could
anyway turn the nonblock param into a bitmask and pass O_TRUNC there.

However, this is wrong because both routines can be called from within
do_file_page, which is called when !pte_present(pte) && !pte_none(pte) &&
pte_file(pte). I.e. the pte is not zeroed, so it has been used, but the page
has been swapped out, or the page hasn't been loaded in first place (for
instance for MAP_NONBLOCK).

More accurately, in that situation ->populate is called with nonblock == 0, so
only install_page can be called there. If ->populate fails, the faulting
process will get an inappropriate SIGBUS.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
---

linux-2.6.git-paolo/mm/fremap.c | 15 +++++++++++++++
1 files changed, 15 insertions(+)

diff -puN mm/fremap.c~rfp-wrong mm/fremap.c
--- linux-2.6.git/mm/fremap.c~rfp-wrong 2005-08-12 18:42:23.000000000 +0200
+++ linux-2.6.git-paolo/mm/fremap.c 2005-08-12 18:42:23.000000000 +0200
@@ -90,6 +90,14 @@ int install_page(struct mm_struct *mm, s
if (!page->mapping || page->index >= size)
goto err_unlock;

+ /*
+ * Only install a new page for a non-shared mapping if it's
+ * not existent yet:
+ */
+ err = -EEXIST;
+ if (!pte_none(*pte) && !(vma->vm_flags & VM_SHARED))
+ goto err_unlock;
+
zap_pte(mm, vma, addr, pte);

inc_mm_counter(mm,rss);
@@ -145,6 +153,13 @@ int install_file_pte(struct mm_struct *m
pte = pte_alloc_map(mm, pmd, addr);
if (!pte)
goto err_unlock;
+ /*
+ * Only install a new page for a non-shared mapping if it's
+ * not existent yet:
+ */
+ err = -EEXIST;
+ if (!pte_none(*pte) && !(vma->vm_flags & VM_SHARED))
+ goto err_unlock;

zap_pte(mm, vma, addr, pte);

_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2005-08-12 20:57    [W:0.236 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site