[PATCH 5.4] tracing/ring-buffer: Only do full wait when cpu != RING_BUFFER_ALL_CPUS

[PATCH 5.4] tracing/ring-buffer: Only do full wait when cpu != RING_BUFFER_ALL_CPUS

[Pratyush Yadav]

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

Re: [PATCH 5.4] tracing/ring-buffer: Only do full wait when cpu != RING_BUFFER_ALL_CPUS

[Steven Rostedt]

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
> 
> 

Re: [PATCH 5.4] tracing/ring-buffer: Only do full wait when cpu != RING_BUFFER_ALL_CPUS

[Greg KH]

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