Skip to content

Commit

Permalink
don't use __kernel_write() on kmap_local_page()
Browse files Browse the repository at this point in the history
[ Upstream commit 06bbaa6 ]

passing kmap_local_page() result to __kernel_write() is unsafe -
random ->write_iter() might (and 9p one does) get unhappy when
passed ITER_KVEC with pointer that came from kmap_local_page().

Fix by providing a variant of __kernel_write() that takes an iov_iter
from caller (__kernel_write() becomes a trivial wrapper) and adding
dump_emit_page() that parallels dump_emit(), except that instead of
__kernel_write() it uses __kernel_write_iter() with ITER_BVEC source.

Fixes: 3159ed5 "fs/coredump: use kmap_local_page()"
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
Al Viro authored and gregkh committed Oct 12, 2022
1 parent 5dac203 commit 71c41e0
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 13 deletions.
38 changes: 33 additions & 5 deletions fs/coredump.c
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,38 @@ static int __dump_skip(struct coredump_params *cprm, size_t nr)
}
}

static int dump_emit_page(struct coredump_params *cprm, struct page *page)
{
struct bio_vec bvec = {
.bv_page = page,
.bv_offset = 0,
.bv_len = PAGE_SIZE,
};
struct iov_iter iter;
struct file *file = cprm->file;
loff_t pos = file->f_pos;
ssize_t n;

if (cprm->to_skip) {
if (!__dump_skip(cprm, cprm->to_skip))
return 0;
cprm->to_skip = 0;
}
if (cprm->written + PAGE_SIZE > cprm->limit)
return 0;
if (dump_interrupted())
return 0;
iov_iter_bvec(&iter, WRITE, &bvec, 1, PAGE_SIZE);
n = __kernel_write_iter(cprm->file, &iter, &pos);
if (n != PAGE_SIZE)
return 0;
file->f_pos = pos;
cprm->written += PAGE_SIZE;
cprm->pos += PAGE_SIZE;

return 1;
}

int dump_emit(struct coredump_params *cprm, const void *addr, int nr)
{
if (cprm->to_skip) {
Expand Down Expand Up @@ -863,7 +895,6 @@ int dump_user_range(struct coredump_params *cprm, unsigned long start,

for (addr = start; addr < start + len; addr += PAGE_SIZE) {
struct page *page;
int stop;

/*
* To avoid having to allocate page tables for virtual address
Expand All @@ -874,10 +905,7 @@ int dump_user_range(struct coredump_params *cprm, unsigned long start,
*/
page = get_dump_page(addr);
if (page) {
void *kaddr = kmap_local_page(page);

stop = !dump_emit(cprm, kaddr, PAGE_SIZE);
kunmap_local(kaddr);
int stop = !dump_emit_page(cprm, page);
put_page(page);
if (stop)
return 0;
Expand Down
3 changes: 3 additions & 0 deletions fs/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ struct shrink_control;
struct fs_context;
struct user_namespace;
struct pipe_inode_info;
struct iov_iter;

/*
* block/bdev.c
Expand Down Expand Up @@ -221,3 +222,5 @@ ssize_t do_getxattr(struct user_namespace *mnt_userns,
int setxattr_copy(const char __user *name, struct xattr_ctx *ctx);
int do_setxattr(struct user_namespace *mnt_userns, struct dentry *dentry,
struct xattr_ctx *ctx);

ssize_t __kernel_write_iter(struct file *file, struct iov_iter *from, loff_t *pos);
22 changes: 14 additions & 8 deletions fs/read_write.c
Original file line number Diff line number Diff line change
Expand Up @@ -509,14 +509,9 @@ static ssize_t new_sync_write(struct file *filp, const char __user *buf, size_t
}

/* caller is responsible for file_start_write/file_end_write */
ssize_t __kernel_write(struct file *file, const void *buf, size_t count, loff_t *pos)
ssize_t __kernel_write_iter(struct file *file, struct iov_iter *from, loff_t *pos)
{
struct kvec iov = {
.iov_base = (void *)buf,
.iov_len = min_t(size_t, count, MAX_RW_COUNT),
};
struct kiocb kiocb;
struct iov_iter iter;
ssize_t ret;

if (WARN_ON_ONCE(!(file->f_mode & FMODE_WRITE)))
Expand All @@ -532,8 +527,7 @@ ssize_t __kernel_write(struct file *file, const void *buf, size_t count, loff_t

init_sync_kiocb(&kiocb, file);
kiocb.ki_pos = pos ? *pos : 0;
iov_iter_kvec(&iter, WRITE, &iov, 1, iov.iov_len);
ret = file->f_op->write_iter(&kiocb, &iter);
ret = file->f_op->write_iter(&kiocb, from);
if (ret > 0) {
if (pos)
*pos = kiocb.ki_pos;
Expand All @@ -543,6 +537,18 @@ ssize_t __kernel_write(struct file *file, const void *buf, size_t count, loff_t
inc_syscw(current);
return ret;
}

/* caller is responsible for file_start_write/file_end_write */
ssize_t __kernel_write(struct file *file, const void *buf, size_t count, loff_t *pos)
{
struct kvec iov = {
.iov_base = (void *)buf,
.iov_len = min_t(size_t, count, MAX_RW_COUNT),
};
struct iov_iter iter;
iov_iter_kvec(&iter, WRITE, &iov, 1, iov.iov_len);
return __kernel_write_iter(file, &iter, pos);
}
/*
* This "EXPORT_SYMBOL_GPL()" is more of a "EXPORT_SYMBOL_DONTUSE()",
* but autofs is one of the few internal kernel users that actually
Expand Down

0 comments on commit 71c41e0

Please sign in to comment.