Re: [PATCH] ring-buffer: fix array bounds checking

[Steven Rostedt]

On Tue, 17 Dec 2024 22:42:53 +0900
Jeongjun Park <aha310510@xxxxxxxxx> wrote:

> > Could you share that reproducer? Or at least the steps. As this situation should
> > never happen a, follow-up fix will be necessary.  
> 
> [1] When tested with a reproducer, pgoff was 8, subbuf_order was 0, and
> subbuf_pages was 1. However, nr_subbufs was 3, so oob-read or uaf occurred.
> 
> [1] : https://syzkaller.appspot.com/text?tag=ReproC&x=14514730580000

This was fixed by Edwards patch.

> Okay. In that case, I will just remove the variable declaration related patches
> and send you the v2 patch right away.
> 

I'm not sure this is needed nor is it a bug.

	while (p < nr_pages) {
		struct page *page = virt_to_page((void *)cpu_buffer->subbuf_ids[s]);
		int off = 0;

		if (WARN_ON_ONCE(s >= nr_subbufs)) {
			err = -EINVAL;
			goto out;
		}

The WARN_ON_ONCE() suggests that this should never happen. And I believe it shouldn't.

I'm fine if you want to make the change to:

	while (p < nr_pages) {
		struct page *page;
		int off = 0;

		if (WARN_ON_ONCE(s >= nr_subbufs)) {
			err = -EINVAL;
			goto out;
		}

		page = virt_to_page((void *)cpu_buffer->subbuf_ids[s]);

But it's not a bug fix. It's simply a cleanup that can wait till the next
merge window.

-- Steve