[PATCH v2] ring-buffer: Fix false warning in ring_buffer_map_get_reader()

Krystian Kaniewski posted 1 patch 3 hours ago
kernel/trace/ring_buffer.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
[PATCH v2] ring-buffer: Fix false warning in ring_buffer_map_get_reader()
Posted by Krystian Kaniewski 3 hours ago
The mmap reader can warn when it catches up with a writer that is still
committing events. rb_get_reader_page() returns NULL in this case, but
ring_buffer_map_get_reader() treats that result as an error.

The initial rb_per_cpu_empty() check filters out an empty buffer, but it
does not guarantee that the next reader page has committed data. Writers
do not take reader_lock and can advance commit_page before publishing the
committed length on the new page. rb_get_reader_page() can swap to that
page, catch up with the writer and return NULL.

Handle NULL through the existing no-data exit without warning. Remove the
caller's reader_page == commit_page check as well, since the reader-page
helper already handles that case. The existing metadata update and return
path remain in place.

Fixes: 117c39200d9d ("ring-buffer: Introducing ring-buffer mapping functions")
Assisted-by: Gemini:gemini-3.8-flash syzbot
Reported-by: syzbot+de3d7f9bcc9212f3fae1@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=de3d7f9bcc9212f3fae1
Link: https://syzkaller.appspot.com/ai_job?id=488adc18-a10e-42d2-a205-25327c38837f
Signed-off-by: Krystian Kaniewski <krystianmkaniewski@gmail.com>
---
Changes since v1:
- Rewrite the commit message to explain the race more concisely.
- Clarify why the earlier empty-buffer check does not exclude this case.
- Remove the redundant reader_page == commit_page check, as discussed
  with Vincent. Let rb_get_reader_page() handle the caught-up reader.

v1: https://lore.kernel.org/all/15bc282f-669e-4a94-911d-bb435513461f@mail.kernel.org/

 kernel/trace/ring_buffer.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 9c03a555a6ba..9222fc881bc2 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -7990,12 +7990,8 @@ consume:
 		goto out;
 	}
 
-	/* Did the reader catch up with the writer? */
-	if (cpu_buffer->reader_page == cpu_buffer->commit_page)
-		goto out;
-
 	reader = rb_get_reader_page(cpu_buffer);
-	if (WARN_ON(!reader))
+	if (!reader)
 		goto out;
 
 	/* Check if any events were dropped */

base-commit: df2908090cda368b01ff43709f51890076c56157
-- 
2.53.0
Re: [PATCH v2] ring-buffer: Fix false warning in ring_buffer_map_get_reader()
Posted by Vincent Donnefort 3 hours ago
On Thu, Sep 24, 2026 at 10:49:42AM +0200, Krystian Kaniewski wrote:
> The mmap reader can warn when it catches up with a writer that is still
> committing events. rb_get_reader_page() returns NULL in this case, but
> ring_buffer_map_get_reader() treats that result as an error.
> 
> The initial rb_per_cpu_empty() check filters out an empty buffer, but it
> does not guarantee that the next reader page has committed data. Writers
> do not take reader_lock and can advance commit_page before publishing the
> committed length on the new page. rb_get_reader_page() can swap to that
> page, catch up with the writer and return NULL.
> 
> Handle NULL through the existing no-data exit without warning. Remove the
> caller's reader_page == commit_page check as well, since the reader-page
> helper already handles that case. The existing metadata update and return
> path remain in place.
> 
> Fixes: 117c39200d9d ("ring-buffer: Introducing ring-buffer mapping functions")
> Assisted-by: Gemini:gemini-3.8-flash syzbot
> Reported-by: syzbot+de3d7f9bcc9212f3fae1@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=de3d7f9bcc9212f3fae1
> Link: https://syzkaller.appspot.com/ai_job?id=488adc18-a10e-42d2-a205-25327c38837f
> Signed-off-by: Krystian Kaniewski <krystianmkaniewski@gmail.com>
> ---
> Changes since v1:
> - Rewrite the commit message to explain the race more concisely.
> - Clarify why the earlier empty-buffer check does not exclude this case.
> - Remove the redundant reader_page == commit_page check, as discussed
>   with Vincent. Let rb_get_reader_page() handle the caught-up reader.
> 
> v1: https://lore.kernel.org/all/15bc282f-669e-4a94-911d-bb435513461f@mail.kernel.org/
> 
>  kernel/trace/ring_buffer.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 9c03a555a6ba..9222fc881bc2 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -7990,12 +7990,8 @@ consume:
>  		goto out;
>  	}
>  
> -	/* Did the reader catch up with the writer? */
> -	if (cpu_buffer->reader_page == cpu_buffer->commit_page)
> -		goto out;
> -
>  	reader = rb_get_reader_page(cpu_buffer);
> -	if (WARN_ON(!reader))
> +	if (!reader)
>  		goto out;
>  
>  	/* Check if any events were dropped */
> 
> base-commit: df2908090cda368b01ff43709f51890076c56157
> -- 
> 2.53.0

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>