Re: [PATCH] swap: fix swapfile read/write offset

Re: [PATCH] swap: fix swapfile read/write offset

[Anthony Iliopoulos]

On Tue, Mar 02, 2021 at 03:36:19PM -0700, Jens Axboe wrote:
> We're not factoring in the start of the file for where to write and
> read the swapfile, which leads to very unfortunate side effects of
> writing where we should not be...
> 
> Fixes: 48d15436fde6 ("mm: remove get_swap_bio")

Presumably the usage of swap_page_sector was already affecting swap on
blockdevs that implement rw_page (currently brd, zram, btt, pmem), so it
may worth adding:

Fixes: dd6bd0d9c7db ("swap: use bdev_read_page() / bdev_write_page()")
Cc: <stable@vger.kernel.org> # v3.16+

for backporting, since it also affects stable.

Re: [PATCH] swap: fix swapfile read/write offset

[Jens Axboe]

On 3/3/21 8:46 AM, Anthony Iliopoulos wrote:
> On Tue, Mar 02, 2021 at 03:36:19PM -0700, Jens Axboe wrote:
>> We're not factoring in the start of the file for where to write and
>> read the swapfile, which leads to very unfortunate side effects of
>> writing where we should not be...
>>
>> Fixes: 48d15436fde6 ("mm: remove get_swap_bio")
> 
> Presumably the usage of swap_page_sector was already affecting swap on
> blockdevs that implement rw_page (currently brd, zram, btt, pmem), so it
> may worth adding:
> 
> Fixes: dd6bd0d9c7db ("swap: use bdev_read_page() / bdev_write_page()")
> Cc: <stable@vger.kernel.org> # v3.16+
> 
> for backporting, since it also affects stable.

yes indeed, in fact that is the source of the original issue (copy/paste
from that broken path).

Fix is already upstream, but would be nice if someone would turn it into
something that could be applied to stable.

-- 
Jens Axboe

Re: [PATCH] swap: fix swapfile read/write offset

[Anthony Iliopoulos]

On Wed, Mar 03, 2021 at 09:24:13AM -0700, Jens Axboe wrote:
> On 3/3/21 8:46 AM, Anthony Iliopoulos wrote:
> > On Tue, Mar 02, 2021 at 03:36:19PM -0700, Jens Axboe wrote:
> >> We're not factoring in the start of the file for where to write and
> >> read the swapfile, which leads to very unfortunate side effects of
> >> writing where we should not be...
> >>
> >> Fixes: 48d15436fde6 ("mm: remove get_swap_bio")
> > 
> > Presumably the usage of swap_page_sector was already affecting swap on
> > blockdevs that implement rw_page (currently brd, zram, btt, pmem), so it
> > may worth adding:
> > 
> > Fixes: dd6bd0d9c7db ("swap: use bdev_read_page() / bdev_write_page()")
> > Cc: <stable@vger.kernel.org> # v3.16+
> > 
> > for backporting, since it also affects stable.
> 
> yes indeed, in fact that is the source of the original issue (copy/paste
> from that broken path).
> 
> Fix is already upstream, but would be nice if someone would turn it into
> something that could be applied to stable.

Sure, I have the following for v5.10+ and can post this later today to
stable for review along with backports for the rest of the longterm
kernels:

From f0d75f9a18e5184670ea11b87bf513b0225b6826 Mon Sep 17 00:00:00 2001
From: Anthony Iliopoulos <ailiop@suse.com>
Date: Wed, 3 Mar 2021 20:07:05 +0100
Subject: [PATCH] swap: fix swapfile page offset mapping

Fix block device sector offset calculation for swap page io on top of
blockdevs that provide a rw_page operation and do page-sized io directly
(without the block layer).

Currently swap_page_sector() maps a swap page into a blockdev sector by
obtaining the swap page offset (swap map slot), but ignores the swapfile
starting offset into the blockdev.

In setups where swapfiles are sitting on top of a filesystem, this
results into swapping out activity potentially overwriting filesystem
blocks that fall outside the swapfile region.

[This issue only affects swapfiles on filesystems on top of blockdevs
that implement rw_page ops (brd, zram, btt, pmem), and not on top of any
other regular block devices.]

Fixes: dd6bd0d9c7db ("swap: use bdev_read_page() / bdev_write_page()")
Cc: <stable@vger.kernel.org> # v5.10+

Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
---
 mm/page_io.c  | 12 ++++--------
 mm/swapfile.c |  2 +-
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/mm/page_io.c b/mm/page_io.c
index 9bca17ecc4df..d2d4d1b3db10 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -37,7 +37,6 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
 
 		bio->bi_iter.bi_sector = map_swap_page(page, &bdev);
 		bio_set_dev(bio, bdev);
-		bio->bi_iter.bi_sector <<= PAGE_SHIFT - 9;
 		bio->bi_end_io = end_io;
 
 		bio_add_page(bio, page, thp_size(page), 0);
@@ -273,11 +272,6 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
 	return ret;
 }
 
-static sector_t swap_page_sector(struct page *page)
-{
-	return (sector_t)__page_file_index(page) << (PAGE_SHIFT - 9);
-}
-
 static inline void count_swpout_vm_event(struct page *page)
 {
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
@@ -355,7 +349,8 @@ int __swap_writepage(struct page *page, struct writeback_control *wbc,
 		return ret;
 	}
 
-	ret = bdev_write_page(sis->bdev, swap_page_sector(page), page, wbc);
+	ret = bdev_write_page(sis->bdev, map_swap_page(page, &sis->bdev),
+			      page, wbc);
 	if (!ret) {
 		count_swpout_vm_event(page);
 		return 0;
@@ -414,7 +409,8 @@ int swap_readpage(struct page *page, bool synchronous)
 	}
 
 	if (sis->flags & SWP_SYNCHRONOUS_IO) {
-		ret = bdev_read_page(sis->bdev, swap_page_sector(page), page);
+		ret = bdev_read_page(sis->bdev, map_swap_page(page, &sis->bdev),
+				     page);
 		if (!ret) {
 			if (trylock_page(page)) {
 				swap_slot_free_notify(page);
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 9fffc5af29d1..47524a4d5e90 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -2308,7 +2308,7 @@ sector_t map_swap_page(struct page *page, struct block_device **bdev)
 {
 	swp_entry_t entry;
 	entry.val = page_private(page);
-	return map_swap_entry(entry, bdev);
+	return map_swap_entry(entry, bdev) << (PAGE_SHIFT - 9);
 }
 
 /*
-- 
2.30.1