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 <[email protected]> > Signed-off-by: Krzysztof Kozlowski <[email protected]>
Acked-by: Hugh Dickins <[email protected]> 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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
