| Message ID | 20221216134241.81381-1-ptyadav@amazon.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [5.4] tracing/ring-buffer: Only do full wait when cpu != RING_BUFFER_ALL_CPUS | expand |
On Fri, 16 Dec 2022 14:42:41 +0100 Pratyush Yadav <ptyadav@amazon.de> wrote: > full_hit() directly uses cpu as an array index. Since > RING_BUFFER_ALL_CPUS == -1, calling full_hit() with cpu == > RING_BUFFER_ALL_CPUS will cause an invalid memory access. > > The upstream commit 42fb0a1e84ff ("tracing/ring-buffer: Have polling > block on watermark") already does this. This was missed when backporting > to v5.4.y. > > This bug was discovered and resolved using Coverity Static Analysis > Security Testing (SAST) by Synopsys, Inc. Nice. > > Fixes: e65ac2bdda54 ("tracing/ring-buffer: Have polling block on watermark") > Signed-off-by: Pratyush Yadav <ptyadav@amazon.de> > --- > > I am not familiar with this code. This was just pointed out by our > static analysis tool and I wrote a quick patch fixing this. Only > compile-tested. > > kernel/trace/ring_buffer.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > index 176d858903bd..11e8189dd8ae 100644 > --- a/kernel/trace/ring_buffer.c > +++ b/kernel/trace/ring_buffer.c > @@ -727,6 +727,7 @@ __poll_t ring_buffer_poll_wait(struct ring_buffer *buffer, int cpu, > > if (cpu == RING_BUFFER_ALL_CPUS) { > work = &buffer->irq_work; > + full = 0; Good catch. This was indeed missed in the backport. The backported patch even added the comment: * @full: wait until the percentage of pages are available, if @cpu != RING_BUFFER_ALL_CPUS Greg, please take this patch. Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> Thanks, -- Steve > } else { > if (!cpumask_test_cpu(cpu, buffer->cpumask)) > return -EINVAL; > -- > 2.38.1 > > > > > Amazon Development Center Germany GmbH > Krausenstr. 38 > 10117 Berlin > Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B > Sitz: Berlin > Ust-ID: DE 289 237 879 > >
On Fri, Dec 16, 2022 at 09:15:33AM -0500, Steven Rostedt wrote: > On Fri, 16 Dec 2022 14:42:41 +0100 > Pratyush Yadav <ptyadav@amazon.de> wrote: > > > full_hit() directly uses cpu as an array index. Since > > RING_BUFFER_ALL_CPUS == -1, calling full_hit() with cpu == > > RING_BUFFER_ALL_CPUS will cause an invalid memory access. > > > > The upstream commit 42fb0a1e84ff ("tracing/ring-buffer: Have polling > > block on watermark") already does this. This was missed when backporting > > to v5.4.y. > > > > This bug was discovered and resolved using Coverity Static Analysis > > Security Testing (SAST) by Synopsys, Inc. > > Nice. > > > > > Fixes: e65ac2bdda54 ("tracing/ring-buffer: Have polling block on watermark") > > Signed-off-by: Pratyush Yadav <ptyadav@amazon.de> > > --- > > > > I am not familiar with this code. This was just pointed out by our > > static analysis tool and I wrote a quick patch fixing this. Only > > compile-tested. > > > > kernel/trace/ring_buffer.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > > index 176d858903bd..11e8189dd8ae 100644 > > --- a/kernel/trace/ring_buffer.c > > +++ b/kernel/trace/ring_buffer.c > > @@ -727,6 +727,7 @@ __poll_t ring_buffer_poll_wait(struct ring_buffer *buffer, int cpu, > > > > if (cpu == RING_BUFFER_ALL_CPUS) { > > work = &buffer->irq_work; > > + full = 0; > > Good catch. This was indeed missed in the backport. The backported patch > even added the comment: > > * @full: wait until the percentage of pages are available, if @cpu != RING_BUFFER_ALL_CPUS > > Greg, please take this patch. > > Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> Now queued up, thanks. greg k-h
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index 176d858903bd..11e8189dd8ae 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c @@ -727,6 +727,7 @@ __poll_t ring_buffer_poll_wait(struct ring_buffer *buffer, int cpu, if (cpu == RING_BUFFER_ALL_CPUS) { work = &buffer->irq_work; + full = 0; } else { if (!cpumask_test_cpu(cpu, buffer->cpumask)) return -EINVAL;
full_hit() directly uses cpu as an array index. Since RING_BUFFER_ALL_CPUS == -1, calling full_hit() with cpu == RING_BUFFER_ALL_CPUS will cause an invalid memory access. The upstream commit 42fb0a1e84ff ("tracing/ring-buffer: Have polling block on watermark") already does this. This was missed when backporting to v5.4.y. This bug was discovered and resolved using Coverity Static Analysis Security Testing (SAST) by Synopsys, Inc. Fixes: e65ac2bdda54 ("tracing/ring-buffer: Have polling block on watermark") Signed-off-by: Pratyush Yadav <ptyadav@amazon.de> --- I am not familiar with this code. This was just pointed out by our static analysis tool and I wrote a quick patch fixing this. Only compile-tested. kernel/trace/ring_buffer.c | 1 + 1 file changed, 1 insertion(+) -- 2.38.1 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879