Re: Memory leak in 2.4.27 kernel, using mmap raw packet sockets

[Marcelo Tosatti <marcelo.tosatti@cyclades.com>]

On Sun, Oct 17, 2004 at 03:39:26AM +0100, Alan Cox wrote:
> On Gwe, 2004-10-15 at 19:23, Marcelo Tosatti wrote:
> > I prefer doing the "if (PageReserved(page)) put_page_testzero(page)" as
> > you propose instead of changing get_user_pages(), as there are several
> > users which rely on its behaviour.
> > 
> > I have applied your fix to the 2.4 BK tree.
> 
> That isnt sufficient. Consider anything else taking a reference to the
> page and the refcount going negative. And yes 2.6.x has this problem and
> far worse in some ways, but it also has the mechanism to fix it.
> 
> 2.6.x uses VM_IO as a VMA flag which tells the kernel two things
> a) get_user_pages fails on it
> b) core dumping of it is forbidden
> 
> 2.6.x is missing a whole pile of these (fixed in the 2.6.9-ac tree I'm
> putting together). I *think* remap_page_range() in 2.6.x can just set
> VM_IO, but older kernels didn't pass the vma so all the users would need
> fixing (OSS audio, media/video, usb audio, usb video, frame buffer
> etc).

I dont see any practical problem with 2.4.x right now.

get_user_pages() wont be called on driver created VMA's with PageReserved 
pages because of the VM_IO bit which is set at remap_page_range(). 

Its not possible to have any vma mapped by a driver without VM_IO set.

But the network packet mmap was an isolated case, so I'll apply Andrea's 
fix just for safety, although I can't find any offender in the tree.


--- memory.c    2004-10-22 15:58:28.000000000 -0200
+++ memory.c  2004-10-28 14:32:26.585813200 -0200
@@ -499,7 +499,7 @@
                                /* FIXME: call the correct function,
                                 * depending on the type of the found page
                                 */
-                               if (!pages[i])
+                               if (!pages[i] || PageReserved(pages[i]))
                                        goto bad_page;
                                page_cache_get(pages[i]);
                        }