linux-kernel - Re: [PATCH] tracing: Decrement trace_array when bootconfig creates an instance

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:   Sat, 25 Jan 2020 11:23:00 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Divya Indi <divya.indi@...cle.com>
Subject: Re: [PATCH] tracing: Decrement trace_array when bootconfig creates
 an instance

On Fri, 24 Jan 2020 20:59:27 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:

> 
> From: "Steven Rostedt (VMware)" <rostedt@...dmis.org>
> 
> The trace_array_get_by_name() creates a ftrace instance and
> trace_array_put() is used to remove the reference. Even though the
> trace_array_get_by_name() creates the instance, it also adds a reference
> count to it, that prevents user space from removing it.
> 
> As the bootconfig just creates the instance on boot up, it should still be
> used where it can be deleted by user space after boot. A trace_array_put()
> is required to let that happen. Otherwise, the created instance can not
> be removed.
> 
> Also, change the documentation on trace_array_get_by_name() to make this not
> be so confusing.

Good catch! I misunderstood trace_array_put() would remove the instance.

Acked-by: Masami Hiramatsu <mhiramat@...nel.org>

Thank you!

> 
> Fixes: 4f712a4d04a4e ("tracing/boot: Add instance node support")
> Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
> ---
>  kernel/trace/trace.c      | 4 ++++
>  kernel/trace/trace_boot.c | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 6fed9b0a8d58..0a5569b1cace 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -8602,6 +8602,10 @@ static int instance_mkdir(const char *name)
>   * NOTE: This function increments the reference counter associated with the
>   * trace array returned. This makes sure it cannot be freed while in use.
>   * Use trace_array_put() once the trace array is no longer needed.
> + * If the trace_array is to be freed, trace_array_destroy() needs to
> + * be called after the trace_array_put(), or simply let user space delete
> + * it from the tracefs instances directory. But until the
> + * trace_array_put() is called, user space can not delete it.
>   *
>   */
>  struct trace_array *trace_array_get_by_name(const char *name)
> diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
> index cd541ac1cbc1..2f616cd926b0 100644
> --- a/kernel/trace/trace_boot.c
> +++ b/kernel/trace/trace_boot.c
> @@ -327,6 +327,7 @@ trace_boot_init_instances(struct xbc_node *node)
>  			continue;
>  		}
>  		trace_boot_init_one_instance(tr, inode);
> +		trace_array_put(tr);
>  	}
>  }
>  
> -- 
> 2.20.1
> 


-- 
Masami Hiramatsu <mhiramat@...nel.org>