| 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
| ||
|
Date: Sun, 26 Jan 2020 12:38:55 -0800
From: Joe Perches <joe@...ches.com>
To: Steven Rostedt <rostedt@...dmis.org>, linux-kernel@...r.kernel.org
Cc: Ingo Molnar <mingo@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [for-next][PATCH 7/7] tracing: Use pr_err() instead of WARN()
for memory failures
On Sun, 2020-01-26 at 14:19 -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@...dmis.org>
>
> As warnings can trigger panics, especially when "panic_on_warn" is set,
> memory failure warnings can cause panics and fail fuzz testers that are
> stressing memory.
>
> Create a MEM_FAIL() macro to use instead of WARN() in the tracing code
> (perhaps this should be a kernel wide macro?), and use that for memory
> failure issues. This should stop failing fuzz tests due to warnings.
It looks as if most of these could just be removed instead
as there is an existing dump_stack() on failure.
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
[]
> @@ -5459,7 +5459,7 @@ static void __init set_ftrace_early_graph(char *buf, int enable)
> struct ftrace_hash *hash;
>
> hash = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> - if (WARN_ON(!hash))
> + if (MEM_FAIL(!hash, "Failed to allocate hash\n"))
> return;
has dump_stack()
> while (buf) {
> @@ -6591,7 +6591,7 @@ static void add_to_clear_hash_list(struct list_head *clear_list,
>
> func = kmalloc(sizeof(*func), GFP_KERNEL);
> if (!func) {
> - WARN_ONCE(1, "alloc failure, ftrace filter could be stale\n");
> + MEM_FAIL(1, "alloc failure, ftrace filter could be stale\n");
has dump_stack()
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
[]
> @@ -3126,7 +3126,7 @@ static int alloc_percpu_trace_buffer(void)
> struct trace_buffer_struct *buffers;
>
> buffers = alloc_percpu(struct trace_buffer_struct);
> - if (WARN(!buffers, "Could not allocate percpu trace_printk buffer"))
> + if (MEM_FAIL(!buffers, "Could not allocate percpu trace_printk buffer"))
> return -ENOMEM;
has dump_stack()
>
> trace_percpu_buffer = buffers;
> @@ -7932,7 +7932,7 @@ static struct dentry *tracing_dentry_percpu(struct trace_array *tr, int cpu)
>
> tr->percpu_dir = tracefs_create_dir("per_cpu", d_tracer);
>
> - WARN_ONCE(!tr->percpu_dir,
> + MEM_FAIL(!tr->percpu_dir,
> "Could not create tracefs directory 'per_cpu/%d'\n", cpu);
keep?
> return tr->percpu_dir;
> @@ -8253,7 +8253,7 @@ create_trace_option_files(struct trace_array *tr, struct tracer *tracer)
> for (cnt = 0; opts[cnt].name; cnt++) {
> create_trace_option_file(tr, &topts[cnt], flags,
> &opts[cnt]);
> - WARN_ONCE(topts[cnt].entry == NULL,
> + MEM_FAIL(topts[cnt].entry == NULL,
> "Failed to create trace option: %s",
> opts[cnt].name);
> }
> @@ -8437,7 +8437,7 @@ static int allocate_trace_buffers(struct trace_array *tr, int size)
> #ifdef CONFIG_TRACER_MAX_TRACE
> ret = allocate_trace_buffer(tr, &tr->max_buffer,
> allocate_snapshot ? size : 1);
> - if (WARN_ON(ret)) {
> + if (MEM_FAIL(ret, "Failed to allocate trace buffer\n")) {
> ring_buffer_free(tr->array_buffer.buffer);
> tr->array_buffer.buffer = NULL;
> free_percpu(tr->array_buffer.data);
> @@ -8726,7 +8726,7 @@ static __init void create_trace_instances(struct dentry *d_tracer)
> trace_instance_dir = tracefs_create_instance_dir("instances", d_tracer,
> instance_mkdir,
> instance_rmdir);
> - if (WARN_ON(!trace_instance_dir))
> + if (MEM_FAIL(!trace_instance_dir, "Failed to create instances directory\n"))
> return;
> }
>
> @@ -8796,7 +8796,7 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
> #endif
>
> if (ftrace_create_function_files(tr, d_tracer))
> - WARN(1, "Could not allocate function filter files");
> + MEM_FAIL(1, "Could not allocate function filter files");
>
> #ifdef CONFIG_TRACER_SNAPSHOT
> trace_create_file("snapshot", 0644, d_tracer,
> @@ -9348,8 +9348,7 @@ __init static int tracer_alloc_buffers(void)
>
> /* TODO: make the number of buffers hot pluggable with CPUS */
> if (allocate_trace_buffers(&global_trace, ring_buf_size) < 0) {
> - printk(KERN_ERR "tracer: failed to allocate ring buffer!\n");
> - WARN_ON(1);
> + MEM_FAIL(1, "tracer: failed to allocate ring buffer!\n");
has dump_stack()
> goto out_free_savedcmd;
> }
>
> @@ -9422,7 +9421,8 @@ void __init early_trace_init(void)
> if (tracepoint_printk) {
> tracepoint_print_iter =
> kmalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
> - if (WARN_ON(!tracepoint_print_iter))
> + if (MEM_FAIL(!tracepoint_print_iter,
> + "Failed to allocate trace iterator\n"))
has dump_stack()
> tracepoint_printk = 0;
> else
> static_key_enable(&tracepoint_printk_key.key);
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 4812a36affac..6bb64d89c321 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -94,6 +94,18 @@ enum trace_type {
>
> #include "trace_entries.h"
>
> +/* Use this for memory failure errors */
> +#define MEM_FAIL(condition, fmt, ...) ({ \
> + static bool __section(.data.once) __warned; \
> + int __ret_warn_once = !!(condition); \
> + \
> + if (unlikely(__ret_warn_once && !__warned)) { \
> + __warned = true; \
> + pr_err("ERROR: " fmt, ##__VA_ARGS__); \
> + } \
> + unlikely(__ret_warn_once); \
> +})
> +
> /*
> * syscalls are special, and need special handling, this is why
> * they are not included in trace_entries.h
Powered by blists - more mailing lists