linux-kernel - Re: [PATCH] swap: fix set_blocksize race during swapon/swapoff

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20131011120226.1f4bb32569f370b57b841e79@linux-foundation.org>
Date:	Fri, 11 Oct 2013 12:02:26 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
Cc:	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Weijie Yang <weijie.yang.kh@...il.com>,
	Bob Liu <bob.liu@...cle.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Shaohua Li <shli@...ionio.com>,
	Minchan Kim <minchan@...nel.org>,
	Hugh Dickins <hughd@...gle.com>
Subject: Re: [PATCH] swap: fix set_blocksize race during swapon/swapoff

(cc Hugh)

On Fri, 11 Oct 2013 11:54:22 +0200 Krzysztof Kozlowski <k.kozlowski@...sung.com> wrote:

> Swapoff used old_block_size from swap_info which could be overwritten by
> concurrent swapon.
> 
> Reported-by: Weijie Yang <weijie.yang.kh@...il.com>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@...sung.com>
> ---
>  mm/swapfile.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 3963fc2..de7c904 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1824,6 +1824,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>  	struct filename *pathname;
>  	int i, type, prev;
>  	int err;
> +	unsigned int old_block_size;
>  
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EPERM;
> @@ -1914,6 +1915,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>  	}
>  
>  	swap_file = p->swap_file;
> +	old_block_size = p->old_block_size;
>  	p->swap_file = NULL;
>  	p->max = 0;
>  	swap_map = p->swap_map;
> @@ -1938,7 +1940,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>  	inode = mapping->host;
>  	if (S_ISBLK(inode->i_mode)) {
>  		struct block_device *bdev = I_BDEV(inode);
> -		set_blocksize(bdev, p->old_block_size);
> +		set_blocksize(bdev, old_block_size);
>  		blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
>  	} else {
>  		mutex_lock(&inode->i_mutex);

I find it worrying that a swapon can run concurrently with any of this
swapoff code.  It just seem to be asking for trouble and the code
really isn't set up for this and races here will be poorly tested for

I'm wondering if we should just extend swapon_mutex a lot and eliminate
the concurrency?

--
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/