Re: Fwd: [draft] Tracing multibuffer support concurrency issues

From: Alexander Lam
Date: Mon Jul 01 2013 - 23:01:57 EST


On Mon, Jul 1, 2013 at 6:35 PM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> On Mon, 2013-07-01 at 15:33 -0700, Alexander Lam wrote:
>
>> To fix this we could go through the ftrace_trace_arrays list and use
>> addresses to check if a particular pointer to a trace_array is still
>> valid, but this is vulnerable to the ABA problem if a trace_array is
>> freed and another is reallocated at the same address. This method is
>> used by subsystem_open() in trace_events.c
>
> And what's so bad about that? If it is freed and a new one is allocated
> at the same address, let it return crap. I'm much more interested in not
> letting it crash then caring about inconsistent data from someone that's
> doing crazy things to the system.

I figured you might say something like that. I agree with you on this.

>>
>> An ugly way to get around the ABA issue is to use a monotonically
>> increasing ID # for each trace_array instance. Those IDs could be used
>> instead of pointers when creating debugfs files.
>
> Not worth it.
>
>>
>> Is there a better way to fix this problem?
>>
>> Also unaddressed are all of the other files which use a trace_array,
>> trace_cpu, or ftrace_event_file in their operation - these would need
>> the same fix.
>
> Hmm, really? Just the initiator. That is, when an event is enabled or
> anything gets opened, we block deletion from happening. That way we
> don't need to care about the rest. Only open and enabling events.

Yes, I did mean the initiator, but I meant the "search for this
pointer in a list" would have to be applied for each of those struct
types because they were being passed through inode->i_private. I see
how this isn't a problem anymore after looking at your patch. It
didn't occur to me that checking the tr field of those structs before
using other pointers in the struct would also work.

Thanks,
Alex

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