2022-05-09 06:00:23

by Jeff Xie

Subject: [PATCH] tracing: Fix possible crash in ftrace_free_ftrace_ops()

Currently if the ftrace_allocate_ftrace_ops() return -ENOMEM,
the ftrace_free_ftrace_ops() will kfree(NULL).

trace_array_create()
{
...
if (ftrace_allocate_ftrace_ops(tr) < 0)
goto out_free_tr;
...
out_free_tr:
ftrace_free_ftrace_ops(tr);
...
}

ftrace_allocate_ftrace_ops()
{
...
ops = kzalloc(sizeof(*ops), GFP_KERNEL);
if (!ops)
return -ENOMEM;
...
}

ftrace_free_ftrace_ops()
{
kfree(tr->ops);
tr->ops = NULL;
}

Signed-off-by: Jeff Xie <[email protected]>
---
kernel/trace/trace_functions.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index 9f1bfbe105e8..d186d6101695 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -73,6 +73,9 @@ int ftrace_allocate_ftrace_ops(struct trace_array *tr)

void ftrace_free_ftrace_ops(struct trace_array *tr)
{
+ if (!tr->ops)
+ return;
+
kfree(tr->ops);
tr->ops = NULL;
}
--
2.25.1

2022-05-09 10:07:16

by Jeff Xie

Subject: Re: [PATCH] tracing: Fix possible crash in ftrace_free_ftrace_ops()

I am so sorry to bother you, this may be an invalid patch, kfree can
return directly from null. ;-)

On Mon, May 9, 2022 at 12:18 AM Jeff Xie <[email protected]> wrote:
>
> Currently if the ftrace_allocate_ftrace_ops() return -ENOMEM,
> the ftrace_free_ftrace_ops() will kfree(NULL).
>
> trace_array_create()
> {
> ...
> if (ftrace_allocate_ftrace_ops(tr) < 0)
> goto out_free_tr;
> ...
> out_free_tr:
> ftrace_free_ftrace_ops(tr);
> ...
> }
>
> ftrace_allocate_ftrace_ops()
> {
> ...
> ops = kzalloc(sizeof(*ops), GFP_KERNEL);
> if (!ops)
> return -ENOMEM;
> ...
> }
>
> ftrace_free_ftrace_ops()
> {
> kfree(tr->ops);
> tr->ops = NULL;
> }
>
> Signed-off-by: Jeff Xie <[email protected]>
> ---
> kernel/trace/trace_functions.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> index 9f1bfbe105e8..d186d6101695 100644
> --- a/kernel/trace/trace_functions.c
> +++ b/kernel/trace/trace_functions.c
> @@ -73,6 +73,9 @@ int ftrace_allocate_ftrace_ops(struct trace_array *tr)
>
> void ftrace_free_ftrace_ops(struct trace_array *tr)
> {
> + if (!tr->ops)
> + return;
> +
> kfree(tr->ops);
> tr->ops = NULL;
> }
> --
> 2.25.1
>

Thanks,
JeffXie

2022-05-09 10:28:53

by Ammar Faizi

Subject: Re: [PATCH] tracing: Fix possible crash in ftrace_free_ftrace_ops()

On 5/9/22 8:33 AM, Jeff Xie wrote:
> I am so sorry to bother you, this may be an invalid patch, kfree can
> return directly from null. ;-)

Yeah, kfree(NULL) is a valid call. There's no potential crash here.

--
Ammar Faizi

2022-05-10 00:36:14

by Steven Rostedt

Subject: Re: [PATCH] tracing: Fix possible crash in ftrace_free_ftrace_ops()

On Mon, 9 May 2022 09:33:44 +0800
Jeff Xie <[email protected]> wrote:

> I am so sorry to bother you, this may be an invalid patch, kfree can
> return directly from null. ;-)

Yes, and so does free() in user space glibc.

-- Steve