linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Jan Kara <jack@suse.cz>
Cc: linux-mm@kvack.org, Ross Zwisler <ross.zwisler@linux.intel.com>
Subject: Re: Use after free in workingset LRU handling
Date: Wed, 18 May 2016 02:03:48 -0400	[thread overview]
Message-ID: <20160518060348.GA31056@cmpxchg.org> (raw)
In-Reply-To: <20160512172722.GC30647@quack2.suse.cz>

Hi Jan,

sorry for the delay, I was cut off from email while traveling.

On Thu, May 12, 2016 at 07:27:22PM +0200, Jan Kara wrote:
> Hello,
> 
> when testing recent DAX fixes, I was puzzled by shadow_lru_isolate()
> barfing on radix tree nodes attached to DAX mappings (as DAX mappings have
> no shadow entries and I took care to not insert radix tree nodes for such
> mappings into workingset_shadow_nodes LRU list. After some investigation, I
> think there is a use after free issue in the handling of radix tree nodes
> by workingset code. The following seems to be possible:
> 
> Radix tree node is created, is has two page pointers for indices 0 and 1.
> 
> Page pointer for index 0 gets replaced with a shadow entry, radix tree
> node gets inserted into workingset_shadow_nodes
> 
> Truncate happens removing page at index 1, __radix_tree_delete_node() in
> page_cache_tree_delete() frees the radix tree node (as it has only single
> entry at index 0 and thus we can shrink the tree) while it is still in LRU
> list!

Due to the way shadow entries are counted, the tree is not actually
shrunk if there is one shadow at index 0.

		/*
		 * The candidate node has more than one child, or its child
		 * is not at the leftmost slot, or it is a multiorder entry,
		 * we cannot shrink.
		 */
		if (to_free->count != 1)
			break;

vs:

static inline void workingset_node_shadows_inc(struct radix_tree_node *node)
{
	node->count += 1U << RADIX_TREE_COUNT_SHIFT;
}

So the use-after-free scenario isn't possible here.

Admittedly, it really isn't pretty. The mess is caused by the page
cache mucking around with structures that should be private to the
radix tree implementation, but I can't think of a good way to solve
this without increasing struct radix_tree_node.

--
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>

  reply	other threads:[~2016-05-18  6:05 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-12 17:27 Jan Kara
2016-05-18  6:03 ` Johannes Weiner [this message]
2016-05-18  7:13   ` Jan Kara

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=20160518060348.GA31056@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=jack@suse.cz \
    --cc=linux-mm@kvack.org \
    --cc=ross.zwisler@linux.intel.com \
    /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