Messages in this thread | | | Date | Tue, 07 Dec 2004 21:44:43 +0300 | From | Stas Sergeev <> | Subject | Re: [patch] kprobes: dont steal interrupts from vm86 |
| |
Hi Prasanna.
You wrote: > The patch below should fix this problem. Please Thanks.
> let me know if you any issues. Well, it kind of fixes the problem. Actually it happened that it fixes even the problem for the programs that are using the segmentation. Now am I happy? I am afraid, not. To me your patch looks very wrong (sorry), even though I am really not the one to do such a claims. Let's see how it "fixes" the problem with segmentation, that I outlined in my previous posting:
> - ret = 1; > + ret = 0; It is easy to notice that ret==0 at that point *always*, so effectively this code is now a no-op. Apparently also gcc mentioned that, and removed that code together with the surrounding "if" clause. And it was exactly the "if" condition where the bogus pointer gets dereferenced. Now, since it gets optimized away, the Oops is not any more. But if I stick a simple printk() in that block, the Oops is back. So you "fixed" that based on the gcc optimization. Now the comments (that you just altered) suggest that the break-point can be removed by another CPU. I don't think delivering the fault to the user-space in this case is wise (but that's what I'd care least, as I am not using the kprobes myself yet). Maybe instead it would be better to return 1 when p!=NULL? Also, you still use the completely invalid addrress and pass it to several functions like get_kprobe() (addr is invalid in either v86 case or when the segmentation is used). Since the deref is now optimized away by gcc, I can't write an Oops test-case for this, but why you do not perform the sanity checks to see whether or not the address is valid? (the checks like I suggested in previous posting)
Oh well. That said, your patch works for me. I'd appreciate another patch very much, that will address the problems I see, but can't demonstrate by the test-case any more. But of course at least for me it is already better than nothing, and of course it is not me to decide whether the patch is to apply or not. We'll see. Thanks anyway.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |