[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20131113103837.GB543@linux.vnet.ibm.com>
Date: Wed, 13 Nov 2013 16:08:37 +0530
From: Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Ingo Molnar <mingo@...nel.org>, Ingo Molnar <mingo@...e.hu>,
Ananth N Mavinakayanahalli <ananth@...ibm.com>,
David Long <dave.long@...aro.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] uprobes: Fix the memory out of bound overwrite in
copy_insn()
* Oleg Nesterov <oleg@...hat.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@...hat.com>
Acked-by: Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
--
Thanks and Regards
Srikar Dronamraju
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists