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 <hverkuil-cisco@xs4all.nl>
---

On 17.08.23 12:41, Hans Verkuil wrote:
> 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 <hverkuil-cisco@xs4all.nl>
> ---
> 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;
>   }
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

On Thu, 17 Aug 2023 at 12:41, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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