On Fri, 11 Oct 2013, Krzysztof Kozlowski wrote:
> Swapoff used old_block_size from swap_info which could be overwritten by
> concurrent swapon.
>
> Reported-by: Weijie Yang <weijie.yang.kh@xxxxxxxxx>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
Acked-by: Hugh Dickins <hughd@xxxxxxxxxx>
Yes, this is straightforward: it was relying on p->old_block_size
after it had given up its hold on *p: a use-after-free (though
those slots are not freed back to a lower-level allocator).
What is not obvious is why swapon needs to use set_blocksize() at all:
if I knew once upon a time, I've forgotten now: because a bdev starts
out with blocksize 0 and someone needs to set it non-0??
> ---
> 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);
> --
> 1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>