Re: [PATCH] mm: Limit pgd range freeing to mm->task_size
From: Hugh Dickins
Date: Thu Feb 14 2013 - 16:24:10 EST
On Wed, 13 Feb 2013, Andrew Morton wrote:
> On Wed, 13 Feb 2013 11:39:29 +0000
> Catalin Marinas <catalin.marinas@xxxxxxx> wrote:
> > ARM processors with LPAE enabled use 3 levels of page tables, with an
> > entry in the top level (pgd) covering 1GB of virtual space. Because of
> > the branch relocation limitations on ARM, the loadable modules are
> > mapped 16MB below PAGE_OFFSET, making the corresponding 1GB pgd shared
> > between kernel modules and user space.
> > Since free_pgtables() is called with ceiling == 0, free_pgd_range() (and
> > subsequently called functions) also frees the page table
> > shared between user space and kernel modules (which is normally handled
> > by the ARM-specific pgd_free() function).
> > This patch changes the ceiling argument to mm->task_size for the
> > free_pgtables() and free_pgd_range() function calls. We cannot use
> > TASK_SIZE since this macro may not be a run-time constant on 64-bit
> > systems supporting compat applications.
> I'm trying to work out why we're using 0 in there at all, rather than
> ->task_size. But that's lost in the mists of time.
> As you've discovered, handling of task_size and TASK_SIZE is somewhat
> inconsistent across architectures and with compat tasks. I guess we
> toss it in there and see if anything breaks...
... and an x86_64 kernel quickly shows,
with either 64-bit or 32-bit userspace, that exit_mmap() breaks at
WARN_ON(mm->nr_ptes > (FIRST_USER_ADDRESS+PMD_SIZE-1)>>PMD_SHIFT);
We couldn't think of using mm->task_size in 2.6.12 because it didn't
exist then; but although it sounds plausible, and on many architectures
(x86_32?) it should be fine, in general it's not quite the right thing
to use. 0 is an easy rounded-up-whatever-the-increment version of
TASK_SIZE (okay, it's missing an implicit 1 before all its 0s).
The ceiling passed to free_pgtables() says how far up it can go in
freeing pts and pmds and puds and pgds: when doing munmap(), you have
to be careful not to stray beyond the range you're freeing; when doing
exit_mmap(), you have to be careful to free all the areas you might
have had to avoid before.
mm->task_size does not necessarily fall on a nice boundary: use it
instead of 0 and exit_mmap() is liable to leave unfreed page tables
at several levels.
I'm sure that Catalin is right that he needs to adjust that ceiling arg
to free_pgtables() to cope with a level shared between user and kernel.
I met the same problem two years ago, when doing a patch (which worked
but went nowhere: x86 people kept on changing the early pagetable setup)
to make CONFIG_VMSPLIT_2G_OPT and 3G_OPT compatible with CONFIG_X86_PAE.
That shared a level beween user and kernel too: everything could be
handled down in the arch code, except this free_pgtables() ceiling arg.
(I did not make any change to the free_pgd_range() calls in fs/exec.c,
I'm not familiar with those at all: my patch appeared to work fine
without touching them, but now I wonder.)
Here's the mm/mmap.c part of my patch (but it now looks like the
default should go into include/asm-generic):
@@ -38,6 +38,16 @@
+ * On almost all architectures and configurations, 0 can be used as the
+ * upper ceiling to free_pgtables(): on many architectures it has the same
+ * effect as using TASK_SIZE. However, there is one configuration which
+ * must impose a more careful limit, to avoid freeing kernel pgtables.
+#define USER_PGTABLES_CEILING 0UL
#define arch_mmap_check(addr, len, flags) (0)
@@ -1888,8 +1898,8 @@ static void unmap_region(struct mm_struct *mm,
unmap_vmas(&tlb, vma, start, end, &nr_accounted, NULL);
- free_pgtables(tlb, vma, prev? prev->vm_end: FIRST_USER_ADDRESS,
- next? next->vm_start: 0);
+ free_pgtables(tlb, vma, prev ? prev->vm_end : FIRST_USER_ADDRESS,
+ next ? next->vm_start : USER_PGTABLES_CEILING);
tlb_finish_mmu(tlb, start, end);
@@ -2221,7 +2231,7 @@ void exit_mmap(struct mm_struct *mm)
end = unmap_vmas(&tlb, vma, 0, -1, &nr_accounted, NULL);
- free_pgtables(tlb, vma, FIRST_USER_ADDRESS, 0);
+ free_pgtables(tlb, vma, FIRST_USER_ADDRESS, USER_PGTABLES_CEILING);
tlb_finish_mmu(tlb, 0, end);
Then arch/x86/include/asm/pgtable-3level_types.h had to
#define USER_PGTABLES_CEILING PAGE_OFFSET
in the special configuration.
In other words: to be safe, I believe you have to keep using 0 for the
ceiling on all the architectures and configurations that you're not
adding special new code to handle the user/kernel shared pagetables.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/