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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
