[PATCH 3/3] xfs: alignment check bio buffers

Dave Chinner
2019-08-21T08:38:29Z

From: Dave Chinner <dchinner@redhat.com>

Add memory buffer alignment validation checks to bios built in XFS
to catch bugs that will result in silent data corruption in block
drivers that cannot handle unaligned memory buffers but don't
validate the incoming buffer alignment is correct.

Known drivers with these issues are xenblk, brd and pmem.

Despite there being nothing XFS specific to xfs_bio_add_page(), this
function was created to do the required validation because the block
layer developers that keep telling us that is not possible to
validate buffer alignment in bio_add_page(), and even if it was
possible it would be too much overhead to do at runtime.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/xfs_bio_io.c | 32 +++++++++++++++++++++++++++++---
fs/xfs/xfs_buf.c | 2 +-
fs/xfs/xfs_linux.h | 3 +++
fs/xfs/xfs_log.c | 6 +++++-
4 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_bio_io.c b/fs/xfs/xfs_bio_io.c
index e2148f2d5d6b..fbaea643c000 100644
--- a/fs/xfs/xfs_bio_io.c
+++ b/fs/xfs/xfs_bio_io.c
@@ -9,6 +9,27 @@ static inline unsigned int bio_max_vecs(unsigned int count)
return min_t(unsigned, howmany(count, PAGE_SIZE), BIO_MAX_PAGES);
}

+int
+xfs_bio_add_page(
+ struct bio *bio,
+ struct page *page,
+ unsigned int len,
+ unsigned int offset)
+{
+ struct request_queue *q = bio->bi_disk->queue;
+ bool same_page = false;
+
+ if (WARN_ON_ONCE(!blk_rq_aligned(q, len, offset)))
+ return -EIO;
+
+ if (!__bio_try_merge_page(bio, page, len, offset, &same_page)) {
+ if (bio_full(bio, len))
+ return 0;
+ __bio_add_page(bio, page, len, offset);
+ }
+ return len;
+}
+
int
xfs_rw_bdev(
struct block_device *bdev,
@@ -20,7 +41,7 @@ xfs_rw_bdev(
{
unsigned int is_vmalloc = is_vmalloc_addr(data);
unsigned int left = count;
- int error;
+ int error, ret = 0;
struct bio *bio;

if (is_vmalloc && op == REQ_OP_WRITE)
@@ -36,9 +57,12 @@ xfs_rw_bdev(
unsigned int off = offset_in_page(data);
unsigned int len = min_t(unsigned, left, PAGE_SIZE - off);

- while (bio_add_page(bio, page, len, off) != len) {
+ while ((ret = xfs_bio_add_page(bio, page, len, off)) != len) {
struct bio *prev = bio;

+ if (ret < 0)
+ goto submit;
+
bio = bio_alloc(GFP_KERNEL, bio_max_vecs(left));
bio_copy_dev(bio, prev);
bio->bi_iter.bi_sector = bio_end_sector(prev);
@@ -48,14 +72,16 @@ xfs_rw_bdev(
submit_bio(prev);
}

+ ret = 0;
data += len;
left -= len;
} while (left > 0);

+submit:
error = submit_bio_wait(bio);
bio_put(bio);

if (is_vmalloc && op == REQ_OP_READ)
invalidate_kernel_vmap_range(data, count);
- return error;
+ return ret ? ret : error;
}
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 7bd1f31febfc..a2d499baee9c 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1294,7 +1294,7 @@ xfs_buf_ioapply_map(
if (nbytes > size)
nbytes = size;

- rbytes = bio_add_page(bio, bp->b_pages[page_index], nbytes,
+ rbytes = xfs_bio_add_page(bio, bp->b_pages[page_index], nbytes,
offset);
if (rbytes < nbytes)
break;
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index ca15105681ca..e71c7bf3e714 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -145,6 +145,9 @@ static inline void delay(long ticks)
schedule_timeout_uninterruptible(ticks);
}

+int xfs_bio_add_page(struct bio *bio, struct page *page, unsigned int len,
+ unsigned int offset);
+
/*
* XFS wrapper structure for sysfs support. It depends on external data
* structures and is embedded in various internal data structures to implement
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 1830d185d7fc..52f7d840d09e 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1673,8 +1673,12 @@ xlog_map_iclog_data(
struct page *page = kmem_to_page(data);
unsigned int off = offset_in_page(data);
size_t len = min_t(size_t, count, PAGE_SIZE - off);
+ int ret;

- WARN_ON_ONCE(bio_add_page(bio, page, len, off) != len);
+ ret = xfs_bio_add_page(bio, page, len, off);
+ WARN_ON_ONCE(ret != len);
+ if (ret < 0)
+ break;

data += len;
count -= len;
--
2.23.0.rc1