[PATCH 2/4] mce/iter: Check for copyin failure & return error up stack

From: Tony Luck
Date: Thu Mar 25 2021 - 20:03:26 EST


Check for failure from low level copy from user code. Doing this
requires some type changes from the unsigned "size_t" so some
signed type (so that "if (xxx < 0)" works!). I picked "loff_t"
but there may be some other more appropriate type.

Very likely more places need to be changed. These changes based
on a machine check copying user data for a write(2) system call
to an xfs file system where the stack trace looks like:

[ 149.445163] ? copyin+0x2d/0x40
[ 149.448687] ? iov_iter_copy_from_user_atomic+0xd6/0x3a0
[ 149.454625] ? iomap_write_actor+0xc7/0x190
[ 149.459319] ? iomap_apply+0x12a/0x440
[ 149.463508] ? iomap_write_begin+0x530/0x530
[ 149.468276] ? iomap_write_begin+0x530/0x530
[ 149.473041] ? iomap_file_buffered_write+0x62/0x90
[ 149.478390] ? iomap_write_begin+0x530/0x530
[ 149.483157] ? xfs_file_buffered_write+0xe0/0x340 [xfs]
[ 149.489393] ? new_sync_write+0x122/0x1b0
[ 149.493879] ? vfs_write+0x20a/0x350
[ 149.497888] ? ksys_write+0x5f/0xe0
[ 149.501784] ? do_syscall_64+0x33/0x40

---

Needs to be bundled with other patches for bisection safety
---
fs/iomap/buffered-io.c | 8 +++++++-
include/linux/uio.h | 2 +-
lib/iov_iter.c | 15 +++++++++++++--
3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 414769a6ad11..6cc28e3f32ff 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -757,7 +757,7 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
struct page *page;
unsigned long offset; /* Offset into pagecache page */
unsigned long bytes; /* Bytes to write to page */
- size_t copied; /* Bytes copied from user */
+ loff_t copied; /* Bytes copied from user */

offset = offset_in_page(pos);
bytes = min_t(unsigned long, PAGE_SIZE - offset,
@@ -791,6 +791,12 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,

copied = iov_iter_copy_from_user_atomic(page, i, offset, bytes);

+ if (copied < 0) {
+ unlock_page(page);
+ put_page(page);
+ return copied;
+ }
+
copied = iomap_write_end(inode, pos, bytes, copied, page, iomap,
srcmap);

diff --git a/include/linux/uio.h b/include/linux/uio.h
index 27ff8eb786dc..c2d82e8e309c 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -111,7 +111,7 @@ static inline struct iovec iov_iter_iovec(const struct iov_iter *iter)
};
}

-size_t iov_iter_copy_from_user_atomic(struct page *page,
+loff_t iov_iter_copy_from_user_atomic(struct page *page,
struct iov_iter *i, unsigned long offset, size_t bytes);
void iov_iter_advance(struct iov_iter *i, size_t bytes);
void iov_iter_revert(struct iov_iter *i, size_t bytes);
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index f66c62aa7154..57e2f81c51ee 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -16,13 +16,18 @@
#define PIPE_PARANOIA /* for now */

#define iterate_iovec(i, n, __v, __p, skip, STEP) { \
- size_t left; \
+ loff_t left; \
size_t wanted = n; \
__p = i->iov; \
__v.iov_len = min(n, __p->iov_len - skip); \
if (likely(__v.iov_len)) { \
__v.iov_base = __p->iov_base + skip; \
left = (STEP); \
+ if (left < 0) { \
+ wanted = left; \
+ n = 0; \
+ goto err; \
+ } \
__v.iov_len -= left; \
skip += __v.iov_len; \
n -= __v.iov_len; \
@@ -36,10 +41,16 @@
continue; \
__v.iov_base = __p->iov_base; \
left = (STEP); \
+ if (left < 0) { \
+ wanted = left; \
+ n = 0; \
+ break; \
+ } \
__v.iov_len -= left; \
skip = __v.iov_len; \
n -= __v.iov_len; \
} \
+err: \
n = wanted - n; \
}

@@ -975,7 +986,7 @@ size_t iov_iter_zero(size_t bytes, struct iov_iter *i)
}
EXPORT_SYMBOL(iov_iter_zero);

-size_t iov_iter_copy_from_user_atomic(struct page *page,
+loff_t iov_iter_copy_from_user_atomic(struct page *page,
struct iov_iter *i, unsigned long offset, size_t bytes)
{
char *kaddr = kmap_atomic(page), *p = kaddr + offset;
--
2.29.2