Messages in this thread |  | | Date | Wed, 13 Nov 2013 16:08:37 +0530 | From | Srikar Dronamraju <> | Subject | Re: [PATCH 1/1] uprobes: Fix the memory out of bound overwrite in copy_insn() |
| |
* Oleg Nesterov <oleg@redhat.com> [2013-11-07 20:40:32]:
> 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 offset + size > i_size, the users of > arch_uprobe->insn can't know how many bytes were actually copied > anyway. But perhaps this needs more changes. > > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
-- Thanks and Regards Srikar Dronamraju
|  |