linux-kernel - Re: [PATCH] mm: Pin address_space before dereferencing it while isolating an LRU page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180123061338.cpsmhih2cvbo4gr6@bbox-2.seo.corp.google.com>
Date:   Tue, 23 Jan 2018 15:13:38 +0900
From:   Minchan Kim <minchan@...nel.org>
To:     Mel Gorman <mgorman@...hsingularity.net>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        "Huang, Ying" <ying.huang@...el.com>, Jan Kara <jack@...e.cz>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: Pin address_space before dereferencing it while
 isolating an LRU page

On Thu, Jan 04, 2018 at 10:25:12AM +0000, Mel Gorman wrote:
> Minchan Kim asked the following question -- what locks protects
> address_space destroying when race happens between inode trauncation and
> __isolate_lru_page? Jan Kara clarified by describing the race as follows
> 
> CPU1                                            CPU2
> 
> truncate(inode)                                 __isolate_lru_page()
>   ...
>   truncate_inode_page(mapping, page);
>     delete_from_page_cache(page)
>       spin_lock_irqsave(&mapping->tree_lock, flags);
>         __delete_from_page_cache(page, NULL)
>           page_cache_tree_delete(..)
>             ...                                   mapping = page_mapping(page);
>             page->mapping = NULL;
>             ...
>       spin_unlock_irqrestore(&mapping->tree_lock, flags);
>       page_cache_free_page(mapping, page)
>         put_page(page)
>           if (put_page_testzero(page)) -> false
> - inode now has no pages and can be freed including embedded address_space
> 
>                                                   if (mapping && !mapping->a_ops->migratepage)
> - we've dereferenced mapping which is potentially already free.
> 
> The race is theoritically possible but unlikely. Before the
> delete_from_page_cache, truncate_cleanup_page is called so the page is
> likely to be !PageDirty or PageWriteback which gets skipped by the only
> caller that checks the mappping in __isolate_lru_page. Even if the race
> occurs, a substantial amount of work has to happen during a tiny window
> with no preemption but it could potentially be done using a virtual machine
> to artifically slow one CPU or halt it during the critical window.
> 
> This patch should eliminate the race with truncation by try-locking the page
> before derefencing mapping and aborting if the lock was not acquired. There
> was a suggestion from Huang Ying to use RCU as a side-effect to prevent
> mapping being freed. However, I do not like the solution as it's an
> unconventional means of preserving a mapping and it's not a context where
> rcu_read_lock is obviously protecting rcu data.
> 
> Fixes: c82449352854 ("mm: compaction: make isolate_lru_page() filter-aware again")
> Signed-off-by: Mel Gorman <mgorman@...hsingularity.net>
Acked-by: Minchan Kim <minchan@...nel.org>

Thanks for the patch.