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(+)
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; -- 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, 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
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Pratyush Yadav -
Steven Rostedt