It's possible that calling sendfile() to copy the data from a memfd to itself may result in doing a memcpy() with overlapping arguments. To avoid undefined behavior here, replace memcpy() with memmove() and rename memcpy_to_page()/memcpy_from_page() accordingly.
This problem has been detected with KMSAN and syzkaller. Signed-off-by: Alexander Potapenko <gli...@google.com> --- For the record, the following program: =================================== // autogenerated by syzkaller (http://github.com/google/syzkaller) #ifndef __NR_memfd_create #define __NR_memfd_create 319 #endif #include <sys/mman.h> #include <sys/time.h> #include <sys/types.h> #include <sys/wait.h> #include <pthread.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <unistd.h> static uint64_t current_time_ms() { struct timespec ts; if (clock_gettime(CLOCK_MONOTONIC, &ts)) _exit(1); return (uint64_t)ts.tv_sec * 1000 + (uint64_t)ts.tv_nsec / 1000000; } int fd; void *page; void* thr(void* arg) { sendfile(fd, fd, page, 0xfec); return 0; } void test() { int i; pthread_t th[2]; page = mmap(0x20000000, 0x1000, 0x3, 0x32, -1, 0); char buf[1] = "\0"; fd = syscall(__NR_memfd_create, buf, 0); write(fd, buf, 1); for (i = 0; i < 2; i++) { pthread_create(&th[i], 0, thr, NULL); usleep(10000); } usleep(100000); } int main() { int iter; for (iter = 0; iter < 10; iter++) { int pid = fork(); if (pid < 0) _exit(1); if (pid == 0) { test(); _exit(0); } int status = 0; uint64_t start = current_time_ms(); for (;;) { int res; if (current_time_ms() - start > 5 * 1000) { kill(-pid, SIGKILL); kill(pid, SIGKILL); while (waitpid(-1, &status, __WALL) != pid) { } return 0; } usleep(1000); } } return 0; } =================================== triggers calls to memcpy() with overlapping arguments: ================================================================== BUG: memcpy-param-overlap in generic_perform_write+0x551/0xa20 __msan_memcpy(ffff88013c6e3001, ffff88013c6e3000, 105) CPU: 0 PID: 1040 Comm: probe Not tainted 4.11.0-rc5+ #2562 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:16 dump_stack+0x143/0x1b0 lib/dump_stack.c:52 __msan_memcpy+0x91/0x1e0 mm/kmsan/kmsan_instr.c:359 memcpy_from_page lib/iov_iter.c:433 iov_iter_copy_from_user_atomic+0x98e/0x1320 lib/iov_iter.c:721 generic_perform_write+0x551/0xa20 mm/filemap.c:2843 __generic_file_write_iter+0x3fa/0x8f0 mm/filemap.c:2960 generic_file_write_iter+0x705/0xa80 mm/filemap.c:2988 call_write_iter ./include/linux/fs.h:1733 vfs_iter_write+0x481/0x580 fs/read_write.c:391 iter_file_splice_write+0xa87/0x12f0 fs/splice.c:771 do_splice_from fs/splice.c:873 direct_splice_actor+0x1ae/0x210 fs/splice.c:1040 splice_direct_to_actor+0x683/0xd40 fs/splice.c:995 do_splice_direct+0x327/0x430 fs/splice.c:1083 do_sendfile+0x8a3/0x12e0 fs/read_write.c:1379 SYSC_sendfile64+0x1ab/0x2b0 fs/read_write.c:1434 SyS_sendfile64+0x9a/0xc0 fs/read_write.c:1426 do_syscall_64+0x72/0xa0 arch/x86/entry/common.c:285 entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:246 RIP: 0033:0x43f4da RSP: 002b:00007f1f1bba4db8 EFLAGS: 00000206 ORIG_RAX: 0000000000000028 RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000043f4da RDX: 0000000020000000 RSI: 0000000000000003 RDI: 0000000000000003 RBP: 00007f1f1bba4dd0 R08: 00007f1f1bba5700 R09: 00007f1f1bba5700 R10: 0000000000000fec R11: 0000000000000206 R12: 0000000000000000 R13: 0000000000000000 R14: 00007f1f1bba59c0 R15: 00007f1f1bba5700 ================================================================== --- lib/iov_iter.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/iov_iter.c b/lib/iov_iter.c index f835964c9485..097a8820e930 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -427,17 +427,19 @@ void iov_iter_init(struct iov_iter *i, int direction, } EXPORT_SYMBOL(iov_iter_init); -static void memcpy_from_page(char *to, struct page *page, size_t offset, size_t len) +static void memmove_from_page(char *to, struct page *page, size_t offset, + size_t len) { char *from = kmap_atomic(page); - memcpy(to, from + offset, len); + memmove(to, from + offset, len); kunmap_atomic(from); } -static void memcpy_to_page(struct page *page, size_t offset, const char *from, size_t len) +static void memmove_to_page(struct page *page, size_t offset, const char *from, + size_t len) { char *to = kmap_atomic(page); - memcpy(to + offset, from, len); + memmove(to + offset, from, len); kunmap_atomic(to); } @@ -525,7 +527,7 @@ static size_t copy_pipe_to_iter(const void *addr, size_t bytes, return 0; for ( ; n; idx = next_idx(idx, pipe), off = 0) { size_t chunk = min_t(size_t, n, PAGE_SIZE - off); - memcpy_to_page(pipe->bufs[idx].page, off, addr, chunk); + memmove_to_page(pipe->bufs[idx].page, off, addr, chunk); i->idx = idx; i->iov_offset = off + chunk; n -= chunk; @@ -543,8 +545,8 @@ size_t copy_to_iter(const void *addr, size_t bytes, struct iov_iter *i) iterate_and_advance(i, bytes, v, __copy_to_user(v.iov_base, (from += v.iov_len) - v.iov_len, v.iov_len), - memcpy_to_page(v.bv_page, v.bv_offset, - (from += v.bv_len) - v.bv_len, v.bv_len), + memmove_to_page(v.bv_page, v.bv_offset, + (from += v.bv_len) - v.bv_len, v.bv_len), memcpy(v.iov_base, (from += v.iov_len) - v.iov_len, v.iov_len) ) @@ -562,8 +564,8 @@ size_t copy_from_iter(void *addr, size_t bytes, struct iov_iter *i) iterate_and_advance(i, bytes, v, __copy_from_user((to += v.iov_len) - v.iov_len, v.iov_base, v.iov_len), - memcpy_from_page((to += v.bv_len) - v.bv_len, v.bv_page, - v.bv_offset, v.bv_len), + memmove_from_page((to += v.bv_len) - v.bv_len, v.bv_page, + v.bv_offset, v.bv_len), memcpy((to += v.iov_len) - v.iov_len, v.iov_base, v.iov_len) ) @@ -586,8 +588,8 @@ bool copy_from_iter_full(void *addr, size_t bytes, struct iov_iter *i) v.iov_base, v.iov_len)) return false; 0;}), - memcpy_from_page((to += v.bv_len) - v.bv_len, v.bv_page, - v.bv_offset, v.bv_len), + memmove_from_page((to += v.bv_len) - v.bv_len, v.bv_page, + v.bv_offset, v.bv_len), memcpy((to += v.iov_len) - v.iov_len, v.iov_base, v.iov_len) ) @@ -606,8 +608,8 @@ size_t copy_from_iter_nocache(void *addr, size_t bytes, struct iov_iter *i) iterate_and_advance(i, bytes, v, __copy_from_user_inatomic_nocache((to += v.iov_len) - v.iov_len, v.iov_base, v.iov_len), - memcpy_from_page((to += v.bv_len) - v.bv_len, v.bv_page, - v.bv_offset, v.bv_len), + memmove_from_page((to += v.bv_len) - v.bv_len, v.bv_page, + v.bv_offset, v.bv_len), memcpy((to += v.iov_len) - v.iov_len, v.iov_base, v.iov_len) ) @@ -629,8 +631,8 @@ bool copy_from_iter_full_nocache(void *addr, size_t bytes, struct iov_iter *i) v.iov_base, v.iov_len)) return false; 0;}), - memcpy_from_page((to += v.bv_len) - v.bv_len, v.bv_page, - v.bv_offset, v.bv_len), + memmove_from_page((to += v.bv_len) - v.bv_len, v.bv_page, + v.bv_offset, v.bv_len), memcpy((to += v.iov_len) - v.iov_len, v.iov_base, v.iov_len) ) @@ -721,8 +723,8 @@ size_t iov_iter_copy_from_user_atomic(struct page *page, iterate_all_kinds(i, bytes, v, __copy_from_user_inatomic((p += v.iov_len) - v.iov_len, v.iov_base, v.iov_len), - memcpy_from_page((p += v.bv_len) - v.bv_len, v.bv_page, - v.bv_offset, v.bv_len), + memmove_from_page((p += v.bv_len) - v.bv_len, v.bv_page, + v.bv_offset, v.bv_len), memcpy((p += v.iov_len) - v.iov_len, v.iov_base, v.iov_len) ) kunmap_atomic(kaddr); -- 2.13.0.303.g4ebf302169-goog
