Messages in this thread | | | Date | Tue, 31 Jul 2012 14:48:05 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH] uprobes: mmap_region() corrupts mm->mm_rb if uprobe_mmap() fails |
| |
On 07/31, Srikar Dronamraju wrote: > > > --- a/kernel/fork.c > > +++ b/kernel/fork.c > > @@ -454,8 +454,8 @@ static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm) > > if (retval) > > goto out; > > > > - if (file && uprobe_mmap(tmp)) > > - goto out; > > + if (file) > > + uprobe_mmap(tmp); > > I am not comfortable with this fix.
OK, so what you suggest for now?
Please note that it is very trivial to crash the kernel. Just do something like
echo "p /bin/true:OFFSET_OF_SYSCALL_INSN" > /sys/kernel/debug/tracing/uprobe_events /bin/true
(or any other unsupported insn)
Yes sure, I agree that in the long term this change should be reconsidered.
> I think the long term solution is as you mentioned, move the > instruction analysis to register.
Exactly. And we already discusssed this, we have a lot of other reasons to do this.
> Lets say there were 10 probes that were to be installed in that vma. > we were able to install five probes and the 6th one happened to fail > because of invalid instruction; we dont continue with the registering > probes for the remaining 4 probes.
Yes. And I already have the patch. I didn't send it because, unlike this fix, it depends on other changes (already in -tip).
Until we move analysis to register, until we teach the callers of uprobe_mmap() to bailout (and please note that vma_adjust() ignores the result too), uprobe_mmap() should not give up if install fails, it should continue.
> The intention behind failing mmap()/fork() if uprobe_mmap failed, > was to make sure that we always report the correct number of events.
Sure, I understand and agree.
But what we can do right now?
Oleg.
| |