From: "Huang\, Ying" <ying.huang@intel.com>
To: Rafael Aquini <aquini@redhat.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
akpm@linux-foundation.org
Subject: Re: [PATCH] mm: swapfile: avoid split_swap_cluster() NULL pointer dereference
Date: Wed, 23 Sep 2020 13:13:49 +0800 [thread overview]
Message-ID: <87sgb9oz1u.fsf@yhuang-dev.intel.com> (raw)
In-Reply-To: <20200923043459.GL795820@optiplex-lnx> (Rafael Aquini's message of "Wed, 23 Sep 2020 00:34:59 -0400")
Rafael Aquini <aquini@redhat.com> writes:
> On Wed, Sep 23, 2020 at 10:21:36AM +0800, Huang, Ying wrote:
>> Hi, Rafael,
>>
>> Rafael Aquini <aquini@redhat.com> writes:
>>
>> > The swap area descriptor only gets struct swap_cluster_info *cluster_info
>> > allocated if the swapfile is backed by non-rotational storage.
>> > When the swap area is laid on top of ordinary disk spindles, lock_cluster()
>> > will naturally return NULL.
>>
>> Thanks for reporting. But the bug looks strange. Because in a system
>> with only HDD swap devices, during THP swap out, the swap cluster
>> shouldn't be allocated, as in
>>
>> shrink_page_list()
>> add_to_swap()
>> get_swap_page()
>> get_swap_pages()
>> swap_alloc_cluster()
>>
>
> The underlying problem is that swap_info_struct.cluster_info is always NULL
> on the rotational storage case.
Yes.
> So, it's very easy to follow that constructions
> like this one, in split_swap_cluster
>
> ...
> ci = lock_cluster(si, offset);
> cluster_clear_huge(ci);
> ...
>
> will go for a NULL pointer dereference, in that case, given that lock_cluster
> reads:
>
> ...
> struct swap_cluster_info *ci;
> ci = si->cluster_info;
> if (ci) {
> ci += offset / SWAPFILE_CLUSTER;
> spin_lock(&ci->lock);
> }
> return ci;
> ...
But on HDD, we shouldn't call split_swap_cluster() at all, because we
will not allocate swap cluster firstly. So, if we run into this,
there should be some other bug, we need to figure it out.
Best Regards,
Huang, Ying
next prev parent reply other threads:[~2020-09-23 5:13 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-22 18:48 [PATCH] mm: swapfile: avoid split_swap_cluster() NULL pointer dereference Rafael Aquini
2020-09-22 19:47 ` Andrew Morton
2020-09-23 13:42 ` Rafael Aquini
2020-09-25 2:59 ` Andrew Morton
2020-09-25 3:06 ` Huang, Ying
2020-09-25 3:10 ` Andrew Morton
2020-09-23 2:21 ` Huang, Ying
2020-09-23 4:34 ` Rafael Aquini
2020-09-23 5:13 ` Huang, Ying [this message]
2020-09-23 13:01 ` Rafael Aquini
2020-09-24 0:59 ` Huang, Ying
2020-09-24 2:09 ` Rafael Aquini
2020-09-24 3:51 ` Huang, Ying
2020-09-24 6:30 ` Rafael Aquini
2020-09-24 6:57 ` Huang, Ying
2020-09-24 7:45 ` Huang, Ying
2020-09-24 15:08 ` Rafael Aquini
2020-09-25 3:21 ` Huang, Ying
2020-09-26 15:16 ` Rafael Aquini
2020-09-27 5:33 ` Huang, Ying
2020-10-01 14:31 ` Rafael Aquini
2020-10-05 13:39 ` Rafael Aquini
2020-10-09 0:18 ` Huang, Ying
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=87sgb9oz1u.fsf@yhuang-dev.intel.com \
--to=ying.huang@intel.com \
--cc=akpm@linux-foundation.org \
--cc=aquini@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.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;
as well as URLs for NNTP newsgroup(s).