lkml.org 
[lkml]   [2012]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Repeated fork() causes SLAB to grow without bound
On Fri, Aug 17, 2012 at 11:46:18PM -0400, Rik van Riel wrote:

> On 08/17/2012 08:03 PM, Daniel Forrest wrote:
>
> >Based on your comments, I came up with the following patch. It boots
> >and the anon_vma/anon_vma_chain SLAB usage is stable, but I don't know
> >if I've overlooked something. I'm not a kernel hacker.
>
> The patch looks reasonable to me. There is one spot left
> for optimization, which I have pointed out below.
>
> Of course, that leaves the big question: do we want the
> overhead of having the atomic addition and decrement for
> every anonymous memory page, or is it easier to fix this
> issue in userspace?
>
> Given that malicious userspace could potentially run the
> system out of memory, without needing special privileges,
> and the OOM killer may not be able to reclaim it due to
> internal slab fragmentation, I guess this issue could be
> classified as a low impact denial of service vulnerability.
>
> Furthermore, there is already a fair amount of bookkeeping
> being done in the rmap code, so this patch is not likely
> to add a whole lot - some testing might be useful, though.
>
> >@@ -262,7 +264,10 @@ int anon_vma_clone(struct vm_area_struct
> > }
> > anon_vma = pavc->anon_vma;
> > root = lock_anon_vma_root(root, anon_vma);
> >- anon_vma_chain_link(dst, avc, anon_vma);
> >+ if (!atomic_read(&anon_vma->pagecount))
> >+ anon_vma_chain_free(avc);
> >+ else
> >+ anon_vma_chain_link(dst, avc, anon_vma);
> > }
> > unlock_anon_vma_root(root);
> > return 0;
>
> In this function, you can do the test before the code block
> where we try to allocate an anon_vma chain.
>
> In other words:
>
> list_for_each_entry_reverse(.....
> struct anon_vma *anon_vma;
>
> + if (!atomic_read(&anon_vma->pagecount))
> + continue;
> +
> avc = anon_vma_chain_alloc(...
> if (unlikely(!avc)) {
>
> The rest looks good.

I was being careful since I wasn't certain about the locking. Does
the test need to be protected by "lock_anon_vma_root"? That's why I
chose the overhead of the possible wasted "anon_vma_chain_alloc".

--
Daniel K. Forrest Space Science and
dan.forrest@ssec.wisc.edu Engineering Center
(608) 890 - 0558 University of Wisconsin, Madison


\
 
 \ /
  Last update: 2012-08-18 07:01    [W:0.381 / U:0.976 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site