linux-kernel - Re: Fwd: [draft] Tracing multibuffer support concurrency issues

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANiYik6ut7zqAnUFk+HtTmrkig79pEx0wkWM+1uBAMorPb0zSw@mail.gmail.com>
Date:	Mon, 1 Jul 2013 20:01:08 -0700
From:	Alexander Lam <azl@...gle.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org, David Sharp <dhsharp@...gle.com>,
	Vaibhav Nagarnaik <vnagarnaik@...gle.com>,
	Alexander Lam <lambchop468@...il.com>
Subject: Re: Fwd: [draft] Tracing multibuffer support concurrency issues

On Mon, Jul 1, 2013 at 6:35 PM, Steven Rostedt <rostedt@...dmis.org> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/