On Tue, 17 Dec 2024 22:42:53 +0900
Jeongjun Park [off-list ref] wrote:

quoted

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