Re: [PATCH 3/3] Make jprobes a little safer for users

[Michael Ellerman <michael@ellerman.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1835 bytes --]

On Tue, 2007-06-26 at 11:49 +0530, Abhishek Sagar wrote:
> On 6/26/07, Michael Ellerman <michael@ellerman.id.au> wrote:
> 
> > We can then use that in register_jprobe() to check that the entry point
> > we're passed is actually in the kernel text, rather than just some random
> > value.
> 
> A similar cleanup is possible even for return probes then. I wonder if
> there are any kprobe related scenarios where the executable code may
> be located outside the core kernel text region (e.g, ITCM?). In that
> case would it also be wrong to assume that the jprobe handler may be
> situated outside the kernel core text / module  region? Would it then
> make sense to move this check from register_jprobe() to the arch
> dependent code?

It did occur to me that someone might be doing something crazy like
branching to code that's not in the kernel/module text - but I was
hoping that wouldn't be the case. I'm not sure what ITCM is?


> >  int __kprobes register_jprobe(struct jprobe *jp)
> >  {
> > +       unsigned long addr = arch_deref_entry_point(jp->entry);
> > +
> > +       if (!kernel_text_address(addr))
> > +               return -EINVAL;
> 
> Seems like you're checking for the jprobe handler to be within
> kernel/module range. Why not narrow this down to just module range
> (!module_text_address(addr), say)? Core kernel functions would not be
> ending with a 'jprobe_return()' anyway.

There's jprobe code in net/ipv4/tcp_probe.c and net/dccp/probe.c that
can be builtin or modular, so I think kernel_text_address() is right.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]