linux-kernel - Re: [PATCHv2 02/10] ftrace: Change mcount call replacement logic

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]

Message-ID: <1324321380.5916.26.camel@gandalf.stny.rr.com>
Date:	Mon, 19 Dec 2011 14:03:00 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Jiri Olsa <jolsa@...hat.com>
Cc:	fweisbec@...il.com, mingo@...hat.com, paulus@...ba.org,
	acme@...stprotocols.net, a.p.zijlstra@...llo.nl,
	linux-kernel@...r.kernel.org, aarapov@...hat.com
Subject: Re: [PATCHv2 02/10] ftrace: Change mcount call replacement logic

On Mon, 2011-12-05 at 18:22 +0100, Jiri Olsa wrote:
> The current logic of updating calls is to do the mcount replacement
> only when ftrace_ops is being registered. When ftrace_ops is being
> unregistered then only in case it was the last registered ftrace_ops,
> all calls are disabled.
> 
> This is an issue when ftrace_ops without FTRACE_OPS_FL_GLOBAL flag
> is being unregistered, because all the functions stays enabled
> and thus inherrited by global_ops, like in following scenario:
> 
>   - set restricting global filter
>   - enable function trace
>   - register/unregister ftrace_ops with flags != FTRACE_OPS_FL_GLOBAL
>     and with no filter

I don't see this problem. I just changed stack_tracer to have its own
filter (I've been wanting to do that for a long time, so when I saw this
email, I decided it's a good time to implement it).

Here's what I did:

# echo schedule > set_ftrace_filter
# cat set_ftrace_filter
schedule
# cat enabled_functions
schedule (1)
# echo 1 > /proc/sys/kernel/stack_tracer_enabled
# cat enabled_functions
do_one_initcall (1)
match_dev_by_uuid (1)
name_to_dev_t (1)
idle_notifier_unregister (1)
idle_notifier_register (1)
start_thread_common.constprop.6 (1)
enter_idle (1)
exit_idle (1)
cpu_idle (1)
__show_regs (1)
release_thread (1)
[...]
_cond_resched (1)
preempt_schedule_irq (1)
schedule (2)
io_schedule (1)
yield_to (1)
yield (1)

// note that schedule is (2)

# echo 0 > /proc/sys/kernel/stack_tracer_enabled
# cat enabled_functions
schedule (1)


> 
> Now the global_ops will get by all the functions regardless the
> global_ops filter. So we need all functions that where enabled via
> this ftrace_ops and are not part of global filter to be disabled.

The global functions are not at issue here. What do you see?

Maybe I fixed something as I'm using the latest tip/perf/core. Note, I
can send you the stack_tracer patch if you want to take a look at this
example. I need to clean it up too.

> 
> Note, currently if there are only global ftrace_ops registered,
> there's no filter hash check and the filter is represented only
> by enabled records.
> 
> Changing the ftrace_shutdown logic to ensure the replacement
> is called for each ftrace_ops being unregistered.
> 
> Also changing FTRACE_ENABLE_CALLS into FTRACE_UPDATE_CALLS
> calls which seems more suitable now.

I still think this patch is wrong. What's the problem you are seeing?

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