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

Powered by blists - more mailing lists