Messages in this thread | | | Date | Fri, 1 Mar 2013 16:51:32 +0530 | From | Ananth N Mavinakayanahalli <> | Subject | Re: [RFC PATCH v3 2/6] uretprobes/x86: hijack return address |
| |
On Fri, Mar 01, 2013 at 12:00:43PM +0100, Anton Arapov wrote: > On Fri, Mar 01, 2013 at 11:15:36AM +0530, Ananth N Mavinakayanahalli wrote: > > On Thu, Feb 28, 2013 at 12:00:11PM +0100, Anton Arapov wrote:
...
> > > +extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long > > > + rp_trampoline_vaddr, struct pt_regs *regs) > > > +{ > > > + int rasize, ncopied; > > > + unsigned long orig_ret_vaddr = 0; /* clear high bits for 32-bit apps */ > > > + > > > + rasize = is_ia32_task() ? 4 : 8; > > > + ncopied = copy_from_user(&orig_ret_vaddr, (void __user *)regs->sp, rasize); > > > + if (unlikely(ncopied)) > > > > What if ncopied < rasize? Agreed that the upper order bits can be 0, but should > > you not validate ncopied == rasize? > > Function returns 0 in case copy_from_user() was not able to copy > return address entirely, and "if (ncopied)" makes sure of it. We > can't continue if we have no correct return address. > > copy_from_user() returns number of bytes that were *not* copied, > thus "ncopied == rasize" means copy_from_user() was not able to copy > *all* bytes. I don't see the point of such check here. > > Or am I missing anything?
You are right... my bad.
Ananth
| |