All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Arcangeli <aarcange@redhat.com>
To: Hugh Dickins <hughd@google.com>
Cc: Chris Wright <chrisw@sous-sol.org>,
	Andrea Righi <andrea@betterlinux.com>,
	CAI Qian <caiqian@redhat.com>, Rik van Riel <riel@redhat.com>,
	Mel Gorman <mel@csn.ul.ie>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [BUG 3.0.0-rc1] ksm: NULL pointer dereference in ksm_do_scan()
Date: Thu, 2 Jun 2011 19:43:05 +0200	[thread overview]
Message-ID: <20110602174305.GH19505@random.random> (raw)
In-Reply-To: <alpine.LSU.2.00.1106021011300.1277@sister.anvils>

On Thu, Jun 02, 2011 at 10:29:39AM -0700, Hugh Dickins wrote:
> AndreaA, I didn't study the patch you posted half an hour ago,
> since by that time I'd worked it out and was preparing patch below.
> I think your patch would be for a different bug, hopefully one we
> don't have, it looks more complicated than we should need for this.

I didn't expect two different bugs leading to double free.

If you've time please review my other patch too because mmput runs
with no mmap_sem hold and I think the ksm scan code runs under the
assumption that __ksm_exit is waiting in down_write() when
ksm_mmlist_lock is released (before freeing the mm_slot), and that
assumption is wrong. ksm_test_exit may very well be true despite
__ksm_exit didn't run yet, and ksm scan will proceed freeing after
changing the mm_slot and ksm_exit will be free to run and free again
immediately after the ksm scan releases the ksm_mmlist_lock and before
it clears the MMF_VM_MERGEABLE (because the mm_slot has been changed
before releasing the ksm_mmlist_lock).

The rmap_list being null will kind of hide it, the fact there's so
little time between the unlock of the ksm_mmlist_lock and the clearing
of MMF_VM_MERGEABLE (that will stop ksm_exit from calling __ksm_exit
at all) will also hide it. At least in
unmerge_and_remove_all_rmap_items remove_trailing_rmap_items will nuke
the rmap_list just before this race runs so making it more likely
possible.

WARNING: multiple messages have this Message-ID (diff)
From: Andrea Arcangeli <aarcange@redhat.com>
To: Hugh Dickins <hughd@google.com>
Cc: Chris Wright <chrisw@sous-sol.org>,
	Andrea Righi <andrea@betterlinux.com>,
	CAI Qian <caiqian@redhat.com>, Rik van Riel <riel@redhat.com>,
	Mel Gorman <mel@csn.ul.ie>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [BUG 3.0.0-rc1] ksm: NULL pointer dereference in ksm_do_scan()
Date: Thu, 2 Jun 2011 19:43:05 +0200	[thread overview]
Message-ID: <20110602174305.GH19505@random.random> (raw)
In-Reply-To: <alpine.LSU.2.00.1106021011300.1277@sister.anvils>

On Thu, Jun 02, 2011 at 10:29:39AM -0700, Hugh Dickins wrote:
> AndreaA, I didn't study the patch you posted half an hour ago,
> since by that time I'd worked it out and was preparing patch below.
> I think your patch would be for a different bug, hopefully one we
> don't have, it looks more complicated than we should need for this.

I didn't expect two different bugs leading to double free.

If you've time please review my other patch too because mmput runs
with no mmap_sem hold and I think the ksm scan code runs under the
assumption that __ksm_exit is waiting in down_write() when
ksm_mmlist_lock is released (before freeing the mm_slot), and that
assumption is wrong. ksm_test_exit may very well be true despite
__ksm_exit didn't run yet, and ksm scan will proceed freeing after
changing the mm_slot and ksm_exit will be free to run and free again
immediately after the ksm scan releases the ksm_mmlist_lock and before
it clears the MMF_VM_MERGEABLE (because the mm_slot has been changed
before releasing the ksm_mmlist_lock).

The rmap_list being null will kind of hide it, the fact there's so
little time between the unlock of the ksm_mmlist_lock and the clearing
of MMF_VM_MERGEABLE (that will stop ksm_exit from calling __ksm_exit
at all) will also hide it. At least in
unmerge_and_remove_all_rmap_items remove_trailing_rmap_items will nuke
the rmap_list just before this race runs so making it more likely
possible.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-06-02 17:44 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-01 22:20 [BUG 3.0.0-rc1] ksm: NULL pointer dereference in ksm_do_scan() Andrea Righi
2011-06-01 22:20 ` Andrea Righi
2011-06-02  1:53 ` Chris Wright
2011-06-02  1:53   ` Chris Wright
2011-06-02  7:09 ` CAI Qian
2011-06-02  7:09   ` CAI Qian
2011-06-02 14:19   ` Andrea Righi
2011-06-02 14:19     ` Andrea Righi
2011-06-02 16:48     ` Chris Wright
2011-06-02 16:48       ` Chris Wright
2011-06-02 17:29       ` Hugh Dickins
2011-06-02 17:29         ` Hugh Dickins
2011-06-02 17:43         ` Andrea Arcangeli [this message]
2011-06-02 17:43           ` Andrea Arcangeli
2011-06-03 17:06           ` Hugh Dickins
2011-06-03 17:06             ` Hugh Dickins
2011-06-03 18:13             ` Andrea Arcangeli
2011-06-03 18:13               ` Andrea Arcangeli
2011-06-02 17:35       ` [PATCH] ksm: fix race between ksmd and exiting task Chris Wright
2011-06-02 17:35         ` Chris Wright
2011-06-02 20:12         ` Andrea Righi
2011-06-02 20:12           ` Andrea Righi
2011-06-02 21:23           ` Chris Wright
2011-06-02 21:23             ` Chris Wright
2011-06-03 16:37         ` Hugh Dickins
2011-06-03 16:37           ` Hugh Dickins
2011-06-04  0:54           ` [PATCH] ksm: fix NULL pointer dereference in scan_get_next_rmap_item Chris Wright
2011-06-04  0:54             ` Chris Wright
2011-06-02 20:10       ` [BUG 3.0.0-rc1] ksm: NULL pointer dereference in ksm_do_scan() Andrea Righi
2011-06-02 20:10         ` Andrea Righi
2011-06-03  4:42     ` CAI Qian
2011-06-03  4:42       ` CAI Qian
2011-06-02 14:31   ` Chris Wright
2011-06-02 14:31     ` Chris Wright
2011-06-02 14:36     ` Andrea Arcangeli
2011-06-02 14:36       ` Andrea Arcangeli
2011-06-02 15:36       ` Chris Wright
2011-06-02 15:36         ` Chris Wright
2011-06-02 16:44         ` Andrea Arcangeli
2011-06-02 16:44           ` Andrea Arcangeli
2011-06-02 20:15           ` Andrea Righi
2011-06-02 20:15             ` Andrea Righi
2011-06-02 21:35             ` Andrea Arcangeli
2011-06-02 21:35               ` Andrea Arcangeli
2011-06-03  4:50       ` CAI Qian
2011-06-03  4:50         ` CAI Qian
2011-06-03  4:44     ` CAI Qian
2011-06-03  4:44       ` CAI Qian

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=20110602174305.GH19505@random.random \
    --to=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrea@betterlinux.com \
    --cc=caiqian@redhat.com \
    --cc=chrisw@sous-sol.org \
    --cc=hughd@google.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mel@csn.ul.ie \
    --cc=riel@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.