Re: [PATCH] block: add missing block_bio_complete() tracepoint

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


Hi,

2012-01-31 7:39 PM, Tejun Heo wrote:
Hello,

On Mon, Jan 30, 2012 at 10:30 PM, Namhyung Kim<namhyung@xxxxxxxxx>  wrote:
Right, but the point is it could make a NULL pointer dereference during
evaluation of the argument of the TP AFAICS. I'm not sure about the TP
implementation though, I think I was wrong - T_E_C() cannot protect us from
it because it happens just before jumping to the TP, right?

So I think we need a conditional jump (with the "likely" annotation) for
this even when the TP is disabled.

Hmmm... still not following. Where the said NULL dereference happen?
TEC conditional is equivalent to "if (COND) TP;".  If you don't use
TEC, it'll be "if (COND) if (TP enabled) TP;".  With TEC, it will be
"if (TP enabled) if (COND) TP;".  There's no other difference.

Thanks.


I've made a quick investigation on TP implementation, and finally figured out what I was wrong - I thought the COND would be checkd in a probe, but it's not. Thanks for pointing it out.

However, for some reason, it seems gcc generated code that evaluates the arguments - bdev_get_queue() in this case - before checking the COND. Simple test module below caused a NULL pointer dereference when I used TRACE_EVENT_CONDITION(), but not for conditional jump:

#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/bio.h>

static int __init init_mod(void)
{
        struct bio *bio = bio_alloc(GFP_KERNEL, 0);
        bio_endio(bio, 0);
        bio_put(bio);
        return 0;
}

static void __exit exit_mod(void)
{
}

module_init(init_mod);
module_exit(exit_mod);


Thanks,
Namhyung

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel


[DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]     [Fedora Docs]

Add to Google Powered by Linux