2023-08-18 13:19:48

by Hans Verkuil

[permalink] [raw]
Subject: [PATCH] media: vb2: frame_vector.c: replace WARN_ONCE with a comment

The WARN_ONCE was issued also in cases that had nothing to do with VM_IO
(e.g. if the start address was just a random value and uaccess fails with
-EFAULT).

There are no reports of WARN_ONCE being issued for actual VM_IO cases, so
just drop it and instead add a note to the comment before the function.

Signed-off-by: Hans Verkuil <[email protected]>
---
diff --git a/drivers/media/common/videobuf2/frame_vector.c b/drivers/media/common/videobuf2/frame_vector.c
index 0f430ddc1f67..fd87747be9b1 100644
--- a/drivers/media/common/videobuf2/frame_vector.c
+++ b/drivers/media/common/videobuf2/frame_vector.c
@@ -31,6 +31,10 @@
* different type underlying the specified range of virtual addresses.
* When the function isn't able to map a single page, it returns error.
*
+ * Note that get_vaddr_frames() cannot follow VM_IO mappings. It used
+ * to be able to do that, but that could (racily) return non-refcounted
+ * pfns.
+ *
* This function takes care of grabbing mmap_lock as necessary.
*/
int get_vaddr_frames(unsigned long start, unsigned int nr_frames, bool write,
@@ -59,8 +63,6 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames, bool write,
if (likely(ret > 0))
return ret;

- /* This used to (racily) return non-refcounted pfns. Let people know */
- WARN_ONCE(1, "get_vaddr_frames() cannot follow VM_IO mapping");
vec->nr_frames = 0;
return ret ? ret : -EFAULT;
}



2023-08-18 13:54:59

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH] media: vb2: frame_vector.c: replace WARN_ONCE with a comment

On 17/08/2023 16:56, Linus Torvalds wrote:
> On Thu, 17 Aug 2023 at 12:41, Hans Verkuil <[email protected]> wrote:
>>
>> There are no reports of WARN_ONCE being issued for actual VM_IO cases, so
>> just drop it and instead add a note to the comment before the function.
>
> Ack. That was meant to catch any (unlikely) strange users, but yeah,
> it can obviously be triggered by "intentional" strange users, ie
> syzbot and friends, so since there seems to be no sign of actual
> real-world use, just removing the WARN_ONCE() is the right thing to
> do.
>
> I'm assuming I'll get this eventually through the regular media pulls?
>
> Linus

Yes, that's the plan.

Regards,

Hans