linux-kernel - Re: [PATCH -V5b -mm] mm, swap: Fix race between swapoff and some swap operations

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: <87d12zsjn1.fsf@yhuang-dev.intel.com>
Date:   Thu, 28 Dec 2017 09:53:22 +0800
From:   "Huang\, Ying" <ying.huang@...el.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>,
        Hugh Dickins <hughd@...gle.com>,
        "Paul E . McKenney" <paulmck@...ux.vnet.ibm.com>,
        Minchan Kim <minchan@...nel.org>,
        "Johannes Weiner" <hannes@...xchg.org>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Shaohua Li <shli@...com>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Jérôme Glisse 
        <jglisse@...hat.com>, Michal Hocko <mhocko@...e.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        David Rientjes <rientjes@...gle.com>,
        Rik van Riel <riel@...hat.com>, Jan Kara <jack@...e.cz>,
        Dave Jiang <dave.jiang@...el.com>,
        Aaron Lu <aaron.lu@...el.com>
Subject: Re: [PATCH -V5b -mm] mm, swap: Fix race between swapoff and some swap operations

"Huang, Ying" <ying.huang@...el.com> writes:

> From: Huang Ying <ying.huang@...el.com>
>
> When the swapin is performed, after getting the swap entry information
> from the page table, system will swap in the swap entry, without any
> lock held to prevent the swap device from being swapoff.  This may
> cause the race like below,
>
> CPU 1				CPU 2
> -----				-----
> 				do_swap_page
> 				  swapin_readahead
> 				    __read_swap_cache_async
> swapoff				      swapcache_prepare
>   p->swap_map = NULL		        __swap_duplicate
> 					  p->swap_map[?] /* !!! NULL pointer access */
>
> Because swapoff is usually done when system shutdown only, the race
> may not hit many people in practice.  But it is still a race need to
> be fixed.
>
> To fix the race, get_swap_device() is added to check whether the
> specified swap entry is valid in its swap device.  If so, it will keep
> the swap entry valid via preventing the swap device from being
> swapoff, until put_swap_device() is called.
>
> Because swapoff() is very rare code path, to make the normal path runs
> as fast as possible, disabling preemption + stop_machine() instead of
> reference count is used to implement get/put_swap_device().  From
> get_swap_device() to put_swap_device(), the preemption is disabled, so
> stop_machine() in swapoff() will wait until put_swap_device() is
> called.
>
> In addition to swap_map, cluster_info, etc. data structure in the
> struct swap_info_struct, the swap cache radix tree will be freed after
> swapoff, so this patch fixes the race between swap cache looking up
> and swapoff too.
>
> Races between some other swap cache usages protected via disabling
> preemption and swapoff are fixed too via calling stop_machine()
> between clearing PageSwapCache() and freeing swap cache data
> structure.
>
> Alternative implementation could be replacing disable preemption with
> rcu_read_lock_sched and stop_machine() with synchronize_sched().
>
> Cc: Hugh Dickins <hughd@...gle.com>
> Cc: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> Cc: Minchan Kim <minchan@...nel.org>
> Cc: Johannes Weiner <hannes@...xchg.org>
> Cc: Tim Chen <tim.c.chen@...ux.intel.com>
> Cc: Shaohua Li <shli@...com>
> Cc: Mel Gorman <mgorman@...hsingularity.net>
> Cc: "Jérôme Glisse" <jglisse@...hat.com>
> Cc: Michal Hocko <mhocko@...e.com>
> Cc: Andrea Arcangeli <aarcange@...hat.com>
> Cc: David Rientjes <rientjes@...gle.com>
> Cc: Rik van Riel <riel@...hat.com>
> Cc: Jan Kara <jack@...e.cz>
> Cc: Dave Jiang <dave.jiang@...el.com>
> Cc: Aaron Lu <aaron.lu@...el.com>
> Signed-off-by: "Huang, Ying" <ying.huang@...el.com>
>
> Changelog:
>
> v5:
>
> - Replace RCU with stop_machine()

2 versions (-V5a and -V5b) have been sent, one is implemented with
stop_machine(), the other is implemented with RCU-sched.  RCU-sched
based version is better for real time users.  Both are OK for me.

Best Regards,
Huang, Ying