linux-kernel - Re: [PATCH v5 bpf-next 2/4] ftrace: Allow IPMODIFY and DIRECT ops on the same function

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:   Fri, 9 Sep 2022 14:05:27 +0000
From:   Song Liu <songliubraving@...com>
To:     "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>
CC:     bpf <bpf@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "live-patching@...r.kernel.org" <live-patching@...r.kernel.org>,
        Song Liu <song@...nel.org>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "jolsa@...nel.org" <jolsa@...nel.org>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH v5 bpf-next 2/4] ftrace: Allow IPMODIFY and DIRECT ops on
 the same function



> On Sep 9, 2022, at 4:58 AM, Naveen N. Rao <naveen.n.rao@...ux.ibm.com> wrote:
> 
> Song Liu wrote:

[...]

>> +
>> /**
>>  * register_ftrace_function - register a function for profiling
>>  * @ops:	ops structure that holds the function for profiling.
>> @@ -8018,14 +8206,15 @@ int register_ftrace_function(struct ftrace_ops *ops)
>> {
>> 	int ret;
>> -	ftrace_ops_init(ops);
>> -
>> -	mutex_lock(&ftrace_lock);
>> -
>> -	ret = ftrace_startup(ops, 0);
>> +	lock_direct_mutex();
> 
> Trying to enable ftrace direct on powerpc, this is resulting in a hung task when testing samples/ftrace/ftrace-direct-modify.c
> 
> Essentially, the sample calls modify_ftrace_direct(), which grabs direct_mutex before calling ftrace_modify_direct_caller()->register_ftrace_function().
> 

Thanks for the report. Would the following change fix the issue?

Song

diff --git i/kernel/trace/ftrace.c w/kernel/trace/ftrace.c
index bc921a3f7ea8..2f1e6cfa834e 100644
--- i/kernel/trace/ftrace.c
+++ w/kernel/trace/ftrace.c
@@ -5496,7 +5496,7 @@ int __weak ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
        if (ret)
                goto out_lock;

-       ret = register_ftrace_function(&stub_ops);
+       ret = register_ftrace_function_nolock(&stub_ops);
        if (ret) {
                ftrace_set_filter_ip(&stub_ops, ip, 1, 0);
                goto out_lock;

> 
> - Naveen
> 
> 
>> +	ret = prepare_direct_functions_for_ipmodify(ops);
>> +	if (ret < 0)
>> +		goto out_unlock;
>> -	mutex_unlock(&ftrace_lock);
>> +	ret = register_ftrace_function_nolock(ops);
>> +out_unlock:
>> +	unlock_direct_mutex();
>> 	return ret;
>> }
>> EXPORT_SYMBOL_GPL(register_ftrace_function);
>> @@ -8044,6 +8233,7 @@ int unregister_ftrace_function(struct ftrace_ops *ops)
>> 	ret = ftrace_shutdown(ops, 0);
>> 	mutex_unlock(&ftrace_lock);
>> +	cleanup_direct_functions_after_ipmodify(ops);
>> 	return ret;
>> }
>> EXPORT_SYMBOL_GPL(unregister_ftrace_function);
>> -- 
>> 2.30.2