2019-04-20 02:25:45

by Wenwen Wang

Subject: [PATCH] tracing: Fix a memory leak bug

In trace_pid_write(), the buffer for trace parser is allocated through
kmalloc() in trace_parser_get_init(). Later on, after the buffer is used,
it is then freed through kfree() in trace_parser_put(). However, it is
possible that trace_pid_write() is terminated due to unexpected errors,
e.g., ENOMEM. In that case, the allocated buffer will not be freed, which
is a memory leak bug.

To fix this issue, free the allocated buffer when an error is encountered.

Signed-off-by: Wenwen Wang <[email protected]>
---
kernel/trace/trace.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6c24755..fd12c9c 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -496,8 +496,10 @@ int trace_pid_write(struct trace_pid_list *filtered_pids,
* not modified.
*/
pid_list = kmalloc(sizeof(*pid_list), GFP_KERNEL);
- if (!pid_list)
+ if (!pid_list) {
+ trace_parser_put(&parser);
return -ENOMEM;
+ }

pid_list->pid_max = READ_ONCE(pid_max);

@@ -507,6 +509,7 @@ int trace_pid_write(struct trace_pid_list *filtered_pids,

pid_list->pids = vzalloc((pid_list->pid_max + 7) >> 3);
if (!pid_list->pids) {
+ trace_parser_put(&parser);
kfree(pid_list);
return -ENOMEM;
}
--
2.7.4

2019-04-20 02:39:38

by Steven Rostedt

Subject: Re: [PATCH] tracing: Fix a memory leak bug

On Fri, 19 Apr 2019 21:22:59 -0500
Wenwen Wang <[email protected]> wrote:

> In trace_pid_write(), the buffer for trace parser is allocated through
> kmalloc() in trace_parser_get_init(). Later on, after the buffer is used,
> it is then freed through kfree() in trace_parser_put(). However, it is
> possible that trace_pid_write() is terminated due to unexpected errors,
> e.g., ENOMEM. In that case, the allocated buffer will not be freed, which
> is a memory leak bug.
>
> To fix this issue, free the allocated buffer when an error is encountered.

Thanks for the patch. Did you find this through manual inspection,
running KASAN or via one of the static analyzers?

-- Steve

>
> Signed-off-by: Wenwen Wang <[email protected]>
> ---
> kernel/trace/trace.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 6c24755..fd12c9c 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -496,8 +496,10 @@ int trace_pid_write(struct trace_pid_list
> *filtered_pids,
> * not modified.
> */
> pid_list = kmalloc(sizeof(*pid_list), GFP_KERNEL);
> - if (!pid_list)
> + if (!pid_list) {
> + trace_parser_put(&parser);
> return -ENOMEM;
> + }
>
> pid_list->pid_max = READ_ONCE(pid_max);
>
> @@ -507,6 +509,7 @@ int trace_pid_write(struct trace_pid_list
> *filtered_pids,
> pid_list->pids = vzalloc((pid_list->pid_max + 7) >> 3);
> if (!pid_list->pids) {
> + trace_parser_put(&parser);
> kfree(pid_list);
> return -ENOMEM;
> }

2019-04-20 03:11:11

by Wenwen Wang

Subject: Re: [PATCH] tracing: Fix a memory leak bug

On Fri, Apr 19, 2019 at 9:37 PM Steven Rostedt <[email protected]> wrote:
>
> On Fri, 19 Apr 2019 21:22:59 -0500
> Wenwen Wang <[email protected]> wrote:
>
> > In trace_pid_write(), the buffer for trace parser is allocated through
> > kmalloc() in trace_parser_get_init(). Later on, after the buffer is used,
> > it is then freed through kfree() in trace_parser_put(). However, it is
> > possible that trace_pid_write() is terminated due to unexpected errors,
> > e.g., ENOMEM. In that case, the allocated buffer will not be freed, which
> > is a memory leak bug.
> >
> > To fix this issue, free the allocated buffer when an error is encountered.
>
> Thanks for the patch. Did you find this through manual inspection,
> running KASAN or via one of the static analyzers?

Thanks for your question, Steve. It was based on a prototype of a
research project, which aims to statically detect memory leak bugs in
operating system kernels.

Wenwen