linux-kernel - Re: [PATCH v2] tracing/filters: Improve subsystem filter

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:	Tue, 21 Jul 2009 14:35:00 +0800
From:	Li Zefan <lizf@...fujitsu.com>
To:	Tom Zanussi <tzanussi@...il.com>
CC:	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...e.hu>,
	Steven Rostedt <rostedt@...dmis.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] tracing/filters: Improve subsystem filter

Tom Zanussi wrote:
> Hi,
> 
> On Mon, 2009-07-20 at 10:20 +0800, Li Zefan wrote:
>> Currently a subsystem filter should be applicable to all events
>> under the subsystem, and if it failed, all the event filters
>> will be cleared. Those behaviors make subsys filter much less
>> useful:
>>
>>   # echo 'vec == 1' > irq/softirq_entry/filter
>>   # echo 'irq == 5' > irq/filter
>>   bash: echo: write error: Invalid argument
>>   # cat irq/softirq_entry/filter
>>   none
>>
>> I'd expect it set the filter for irq_handler_entry/exit, and
>> not touch softirq_entry/exit.
>>
>> The basic idea is, try to see if the filter can be applied
>> to which events, and then just apply to the those events:
>>
>>   # echo 'vec == 1' > softirq_entry/filter
>>   # echo 'irq == 5' > filter
>>   # cat irq_handler_entry/filter
>>   irq == 5
>>   # cat softirq_entry/filter
>>   vec == 1
>>
> 
> It looks like it accomplishes the goal, but I wonder if you could
> simplify it by getting rid of the no_reset flag and all the state-saving
> while keeping the dry_run param on filter_add_pred to use from
> filter_add_subsysystem_pred() e.g. something like this:
> 
> filter_add_subsystem_pred(...)
> {
> 	...
> 	list_for_each_entry(call, &ftrace_events, list) {
> 		err = filter_add_pred(ps, call, pred, dry_run = 1);
> 		if (!err)
> 			filter_add_pred(ps, call, pred, dry_run = 0);

This is the same with just call "filter_add_pred(ps, call, pred)"

> 		...
> 	}
> }
> 
> IIRC the state-saving was necessary if the idea was to back out of (or
> avoid setting) all the filters if one failed, but the new model is to
> set whichever apply while leaving the rest alone.  I think the simpler
> approach above accomplishes that, but I may be missing something or have
> forgotten the original motivation for doing it this way.
> 

This won't work as expected. The state-saving is necessary, because
if any pred is not appliable for an event, the whole operation should
fail for that event.

What this patch does is apply the filter to those events that the
filter is appliable, but not apply parts of the filter to each event.

Imaging the result for those filters with the above code:

  # echo 'irq == 5 && vec == 1' > filter
  # echo 'irq == 5 || vec == 1' > filter
  # echo 'irq == 5 || foo == bar' > fitler
  # echo '(irq == 5 && vec == 1) || irq == 2' > filter

--
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/