linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Hugh Dickins <hugh@veritas.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>,
	Erez Zadok <ezk@cs.sunysb.edu>, Ryan Finnie <ryan@finnie.org>,
	Michael Halcrow <mhalcrow@us.ibm.com>,
	cjwatson@ubuntu.com, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	stable@kernel.org
Subject: [PATCH] fix tmpfs BUG and AOP_WRITEPAGE_ACTIVATE
Date: Wed, 24 Oct 2007 22:02:15 +0100 (BST)	[thread overview]
Message-ID: <Pine.LNX.4.64.0710242152020.13001@blonde.wat.veritas.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0710222101420.23513@blonde.wat.veritas.com>

It's possible to provoke unionfs (not yet in mainline, though in mm
and some distros) to hit shmem_writepage's BUG_ON(page_mapped(page)).
I expect it's possible to provoke the 2.6.23 ecryptfs in the same way
(but the 2.6.24 ecryptfs no longer calls lower level's ->writepage).

This came to light with the recent find that AOP_WRITEPAGE_ACTIVATE
could leak from tmpfs via write_cache_pages and unionfs to userspace.
There's already a fix (e423003028183df54f039dfda8b58c49e78c89d7 -
writeback: don't propagate AOP_WRITEPAGE_ACTIVATE) in the tree for
that, and it's okay so far as it goes; but insufficient because it
doesn't address the underlying issue, that shmem_writepage expects
to be called only by vmscan (relying on backing_dev_info capabilities
to prevent the normal writeback path from ever approaching it).

That's an increasingly fragile expectation, and ramdisk_writepage
(the other source of AOP_WRITEPAGE_ACTIVATEs) is already careful
to check wbc->for_reclaim before returning it.  Make the same check
in shmem_writepage, thereby sidestepping the page_mapped BUG also.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
Unionfs intends its own, third fix to these issues, checking
backing_dev_info capabilities as the normal writeback path does.
And I intend a fourth fix, getting rid of AOP_WRITEPAGE_ACTIVATE
entirely (mainly to put a stop to everybody asking what it means
and when it happens and how to handle it) - but that's a slightly
bigger patch, needing a little more testing, probably for 2.6.25.

I've CC'ed this to stable as you did for the write_cache_pages
fix: it's probably required for ecryptfs (but unionfs was much
easier to set up and test), and helpful to distros using unionfs
and checking stable for fixes.  Does this make the write_cache_pages
fix redundant?  Probably, but let's have both in for safety.

 mm/shmem.c |    5 +++++
 1 file changed, 5 insertions(+)

--- 2.6.24-rc1/mm/shmem.c	2007-10-24 07:16:04.000000000 +0100
+++ linux/mm/shmem.c	2007-10-24 20:24:31.000000000 +0100
@@ -915,6 +915,11 @@ static int shmem_writepage(struct page *
 	struct inode *inode;
 
 	BUG_ON(!PageLocked(page));
+	if (!wbc->for_reclaim) {
+		set_page_dirty(page);
+		unlock_page(page);
+		return 0;
+	}
 	BUG_ON(page_mapped(page));
 
 	mapping = page->mapping;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2007-10-24 21:02 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200710071920.l97JKJX5018871@agora.fsl.cs.sunysb.edu>
2007-10-11 21:47 ` msync(2) bug(?), returns AOP_WRITEPAGE_ACTIVATE to userland Andrew Morton
2007-10-11 22:12   ` Ryan Finnie
2007-10-12  0:38     ` Hugh Dickins
2007-10-12 21:45       ` Pekka Enberg
2007-10-14  8:44         ` Hugh Dickins
2007-10-14 17:09           ` Pekka Enberg
2007-10-14 17:23             ` Erez Zadok
2007-10-14 17:50               ` Pekka J Enberg
2007-10-14 22:32                 ` Erez Zadok
2007-10-15 11:47                   ` Pekka Enberg
2007-10-16 18:02                     ` Erez Zadok
2007-10-22 20:16                     ` Hugh Dickins
2007-10-22 20:48                       ` Pekka Enberg
2007-10-25 15:36                         ` Hugh Dickins
2007-10-25 16:44                           ` Erez Zadok
2007-10-25 18:23                             ` Hugh Dickins
2007-10-26  2:00                           ` Neil Brown
2007-10-26  8:09                             ` Pekka Enberg
2007-10-26 11:26                             ` Hugh Dickins
2007-10-26  8:05                           ` Pekka Enberg
2007-10-22 21:04                       ` Erez Zadok
2007-10-25 16:40                         ` Hugh Dickins
2007-10-24 21:02                       ` Hugh Dickins [this message]
2007-10-24 21:08                         ` [PATCH] fix tmpfs BUG and AOP_WRITEPAGE_ACTIVATE Andrew Morton
2007-10-24 21:37                           ` [PATCH+comment] " Hugh Dickins
2007-10-25  5:37                             ` Pekka Enberg
2007-10-25  6:30                               ` Hugh Dickins
2007-10-25  7:24                                 ` Pekka Enberg
2007-10-25 16:01                                 ` Erez Zadok
2007-10-25 20:51                                   ` H. Peter Anvin
2007-10-22 20:01                   ` msync(2) bug(?), returns AOP_WRITEPAGE_ACTIVATE to userland Hugh Dickins
2007-10-22 20:40                     ` Pekka Enberg
2007-10-22 19:42               ` Hugh Dickins
2007-10-22 21:38                 ` Erez Zadok
2007-10-25 18:03                   ` Hugh Dickins
2007-10-27 20:47                     ` Erez Zadok
2007-10-28 20:23                     ` Erez Zadok
2007-10-29 20:33                       ` Hugh Dickins
2007-10-31 23:53                         ` Erez Zadok
2007-11-05 15:40                           ` Hugh Dickins
2007-11-05 16:38                             ` Dave Hansen
2007-11-05 18:57                               ` Hugh Dickins
2007-11-09  2:47                               ` Erez Zadok
2007-11-09  6:05                             ` Erez Zadok
2007-11-12  5:41                               ` Hugh Dickins
2007-11-12 17:01                               ` Hugh Dickins
2007-11-13 10:18                                 ` Erez Zadok
2007-11-17 21:24                                   ` Hugh Dickins
2007-11-20  1:30                                     ` Erez Zadok

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0710242152020.13001@blonde.wat.veritas.com \
    --to=hugh@veritas.com \
    --cc=akpm@linux-foundation.org \
    --cc=cjwatson@ubuntu.com \
    --cc=ezk@cs.sunysb.edu \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhalcrow@us.ibm.com \
    --cc=penberg@cs.helsinki.fi \
    --cc=ryan@finnie.org \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox