leandrof/linux
1
0
Fork 0
forked from Minki/linux
Code Issues Pull Requests Packages Projects Releases Wiki Activity

uprobes: Fix the memory out of bound overwrite in copy_insn()

Browse Source 
1. copy_insn() doesn't look very nice, all calculations are
   confusing and it is not immediately clear why do we read
   the 2nd page first.

2. The usage of inode->i_size is wrong on 32-bit machines.

3. "Instruction at end of binary" logic is simply wrong, it
   doesn't handle the case when uprobe->offset > inode->i_size.

   In this case "bytes" overflows, and __copy_insn() writes to
   the memory outside of uprobe->arch.insn.

   Yes, uprobe_register() checks i_size_read(), but this file
   can be truncated after that. All i_size checks are racy, we
   do this only to catch the obvious mistakes.

Change copy_insn() to call __copy_insn() in a loop, simplify
and fix the bytes/nbytes calculations.

Note: we do not care if we read extra bytes after inode->i_size
if we got the valid page. This is fine because the task gets the
same page after page-fault, and arch_uprobe_analyze_insn() can't
know how many bytes were actually read anyway.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
This commit is contained in:
Oleg Nesterov 2013-11-07 19:41:57 +01:00
parent 70d7f98722
commit 2ded0980a6
1 changed files with 21 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
kernel/events/uprobes.c
View File

@ -504,9 +504,8 @@ static bool consumer_del(struct uprobe *uprobe, struct uprobe_consumer *uc)
return ret; return ret;
} }
static int static int __copy_insn(struct address_space *mapping, struct file *filp,
__copy_insn(struct address_space *mapping, struct file *filp, char *insn, void *insn, int nbytes, loff_t offset)
unsigned long nbytes, loff_t offset)
{ {
struct page *page; struct page *page;
@ -528,28 +527,28 @@ __copy_insn(struct address_space *mapping, struct file *filp, char *insn,
static int copy_insn(struct uprobe *uprobe, struct file *filp) static int copy_insn(struct uprobe *uprobe, struct file *filp)
{ {
struct address_space *mapping; struct address_space *mapping = uprobe->inode->i_mapping;
unsigned long nbytes; loff_t offs = uprobe->offset;
int bytes; void *insn = uprobe->arch.insn;
int size = MAX_UINSN_BYTES;
int len, err = -EIO;
nbytes = PAGE_SIZE - (uprobe->offset & ~PAGE_MASK); /* Copy only available bytes, -EIO if nothing was read */
mapping = uprobe->inode->i_mapping; do {
if (offs >= i_size_read(uprobe->inode))
break;
/* Instruction at end of binary; copy only available bytes */ len = min_t(int, size, PAGE_SIZE - (offs & ~PAGE_MASK));
if (uprobe->offset + MAX_UINSN_BYTES > uprobe->inode->i_size) err = __copy_insn(mapping, filp, insn, len, offs);
bytes = uprobe->inode->i_size - uprobe->offset;
else
bytes = MAX_UINSN_BYTES;
/* Instruction at the page-boundary; copy bytes in second page */
if (nbytes < bytes) {
int err = __copy_insn(mapping, filp, uprobe->arch.insn + nbytes,
bytes - nbytes, uprobe->offset + nbytes);
if (err) if (err)
return err; break;
bytes = nbytes;
} insn += len;
return __copy_insn(mapping, filp, uprobe->arch.insn, bytes, uprobe->offset); offs += len;
size -= len;
} while (size);
return err;
} }
static int prepare_uprobe(struct uprobe *uprobe, struct file *file, static int prepare_uprobe(struct uprobe *uprobe, struct file *file,