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
| ||
|
Date: Sun, 12 Jan 2014 19:27:44 -0800 From: Andrew Morton <akpm@...ux-foundation.org> To: Weijie Yang <weijie.yang.kh@...il.com> Cc: Weijie Yang <weijie.yang@...sung.com>, linux-kernel <linux-kernel@...r.kernel.org>, Linux-MM <linux-mm@...ck.org>, Hugh Dickins <hughd@...gle.com>, Minchan Kim <minchan@...nel.org>, Shaohua Li <shli@...ionio.com>, Bob Liu <bob.liu@...cle.com>, stable@...r.kernel.org, Krzysztof Kozlowski <k.kozlowski@...sung.com> Subject: Re: [PATCH] mm/swap: fix race on swap_info reuse between swapoff and swapon On Mon, 13 Jan 2014 11:08:58 +0800 Weijie Yang <weijie.yang.kh@...il.com> wrote: > >> --- a/mm/swapfile.c > >> +++ b/mm/swapfile.c > >> @@ -1922,7 +1922,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) > >> p->swap_map = NULL; > >> cluster_info = p->cluster_info; > >> p->cluster_info = NULL; > >> - p->flags = 0; > >> frontswap_map = frontswap_map_get(p); > >> spin_unlock(&p->lock); > >> spin_unlock(&swap_lock); > >> @@ -1948,6 +1947,16 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) > >> mutex_unlock(&inode->i_mutex); > >> } > >> filp_close(swap_file, NULL); > >> + > >> + /* > >> + * clear SWP_USED flag after all resources freed > >> + * so that swapon can reuse this swap_info in alloc_swap_info() safely > >> + * it is ok to not hold p->lock after we cleared its SWP_WRITEOK > >> + */ > >> + spin_lock(&swap_lock); > >> + p->flags = 0; > >> + spin_unlock(&swap_lock); > >> + > >> err = 0; > >> atomic_inc(&proc_poll_event); > >> wake_up_interruptible(&proc_poll_wait); > > > > I didn't look too closely, but this patch might also address the race > > which Krzysztof addressed with > > http://ozlabs.org/~akpm/mmots/broken-out/swap-fix-setting-page_size-blocksize-during-swapoff-swapon-race.patch. > > Can we please check that out? > > > > I do prefer fixing all these swapon-vs-swapoff races with some large, > > simple, wide-scope exclusion scheme. Perhaps SWP_USED is that scheme. > > > > An alternative would be to add another mutex and just make sys_swapon() > > and sys_swapoff() 100% exclusive. But that is plastering yet another > > lock over this mess to hide the horrors which lurk within :( > > > > Hi, Andrew. Thanks for your suggestion. > > I checked Krzysztof's patch, it use the global swapon_mutex to protect > race condition among > swapon, swapoff and swap_start(). It is a kind of correct method, but > a heavy method. But do you agree that your http://ozlabs.org/~akpm/mmots/broken-out/mm-swap-fix-race-on-swap_info-reuse-between-swapoff-and-swapon.patch makes Krzysztof's http://ozlabs.org/~akpm/mmots/broken-out/swap-fix-setting-page_size-blocksize-during-swapoff-swapon-race.patch obsolete? I've been sitting on Krzysztof's swap-fix-setting-page_size-blocksize-during-swapoff-swapon-race.patch for several months - Hugh had issues with it so I put it on hold and nothing further happened. > I will try to resend a patchset to make lock usage in swapfile.c clear > and fine grit OK, thanks. In the meanwhile I'm planning on dropping Krzysztof's patch and merging your patch into 3.14-rc1, which is why I'd like confirmation that your patch addresses the issues which Krzysztof identified? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists