linux-kernel - Re: [PATCH 3/5] ftrace: add ftrace

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] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 2 Mar 2009 19:06:44 +0100
From:	Frédéric Weisbecker <fweisbec@...il.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <peterz@...radead.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/5] ftrace: add ftrace_bprintk()

2009/3/2 Steven Rostedt <rostedt@...dmis.org>:
>
> On Mon, 2 Mar 2009, Fr?d?ric Weisbecker wrote:
>> >> +
>> >> +static
>> >> +void release_module_trace_bprintk_format(const char **start, const char **end)
>> >> +{
>> >> +     const char **iter;
>> >> +     lock_btrace();
>> >> +     for (iter = start; iter < end; iter++) {
>> >> +             struct trace_bprintk_fmt *tb_fmt;
>> >> +             if (!*iter)
>> >> +                     continue;
>> >> +
>> >> +             tb_fmt = container_of(*iter, struct trace_bprintk_fmt, fmt[0]);
>> >> +             tb_fmt->count--;
>> >> +             if (!tb_fmt->count && !btrace_metadata_count) {
>> >> +                     list_del(&tb_fmt->list);
>> >> +                     kfree(tb_fmt);
>> >
>> > Shouldn't *iter get assigned to NULL somewhere here?
>> >
>> > -- Steve
>>
>>
>> Hm, why?
>
> Well, after we free tb_fmt, the *iter will then point to garbage. Right?
>
> -- Steve


Now that you say it, I have some doubts about the possible sites that
can still dereference it
at this point.
I have to review and test it more seriously. I was convinced that the
count field kept track
of all references but now I'm not so sure, there can be still one
pending event that uses it into
the ring buffer, or it can be perhaps in use at the same time it is freed.
We should perhaps use rcu here, will see.
--
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/