| Message ID | 20221101191009.1e7378c8@rorschach.local.home (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ring-buffer: Check for NULL cpu_buffer in ring_buffer_wake_waiters() | expand |
On Tue, 1 Nov 2022 19:10:09 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > On some machines the number of listed CPUs may be bigger than the actual > CPUs that exist. The tracing subsystem allocates a per_cpu directory with > access to the per CPU ring buffer via a cpuX file. But to save space, the > ring buffer will only allocate buffers for online CPUs, even though the > CPU array will be as big as the nr_cpu_ids. > > With the addition of waking waiters on the ring buffer when closing the > file, the ring_buffer_wake_waiters() now needs to make sure that the > buffer is allocated (with the irq_work allocated with it) before trying to > wake waiters, as it will cause a NULL pointer dereference. > > While debugging this, I added a NULL check for the buffer itself (which is > OK to do), and also NULL pointer checks against buffer->buffers (which is > not fine, and will WARN) as well as making sure the CPU number passed in > is within the nr_cpu_ids (which is also not fine if it isn't). > > Link: https://lore.kernel.org/all/87h6zklb6n.wl-tiwai@suse.de/ > Link: https://lore.kernel.org/all/CAM6Wdxc0KRJMXVAA0Y=u6Jh2V=uWB-_Fn6M4xRuNppfXzL1mUg@mail.gmail.com/ > This looks good to me. Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thanks, > Cc: stable@vger.kernel.org > Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=1204705 > Reported-by: Takashi Iwai <tiwai@suse.de> > Reported-by: Roland Ruckerbauer <roland.rucky@gmail.com> > Fixes: f3ddb74ad079 ("tracing: Wake up ring buffer waiters on closing of the file") > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > kernel/trace/ring_buffer.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > index 199759c73519..9712083832f4 100644 > --- a/kernel/trace/ring_buffer.c > +++ b/kernel/trace/ring_buffer.c > @@ -937,6 +937,9 @@ void ring_buffer_wake_waiters(struct trace_buffer *buffer, int cpu) > struct ring_buffer_per_cpu *cpu_buffer; > struct rb_irq_work *rbwork; > > + if (!buffer) > + return; > + > if (cpu == RING_BUFFER_ALL_CPUS) { > > /* Wake up individual ones too. One level recursion */ > @@ -945,7 +948,15 @@ void ring_buffer_wake_waiters(struct trace_buffer *buffer, int cpu) > > rbwork = &buffer->irq_work; > } else { > + if (WARN_ON_ONCE(!buffer->buffers)) > + return; > + if (WARN_ON_ONCE(cpu >= nr_cpu_ids)) > + return; > + > cpu_buffer = buffer->buffers[cpu]; > + /* The CPU buffer may not have been initialized yet */ > + if (!cpu_buffer) > + return; > rbwork = &cpu_buffer->irq_work; > } > > -- > 2.35.1 >
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index 199759c73519..9712083832f4 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c @@ -937,6 +937,9 @@ void ring_buffer_wake_waiters(struct trace_buffer *buffer, int cpu) struct ring_buffer_per_cpu *cpu_buffer; struct rb_irq_work *rbwork; + if (!buffer) + return; + if (cpu == RING_BUFFER_ALL_CPUS) { /* Wake up individual ones too. One level recursion */ @@ -945,7 +948,15 @@ void ring_buffer_wake_waiters(struct trace_buffer *buffer, int cpu) rbwork = &buffer->irq_work; } else { + if (WARN_ON_ONCE(!buffer->buffers)) + return; + if (WARN_ON_ONCE(cpu >= nr_cpu_ids)) + return; + cpu_buffer = buffer->buffers[cpu]; + /* The CPU buffer may not have been initialized yet */ + if (!cpu_buffer) + return; rbwork = &cpu_buffer->irq_work; }