media: vb2: frame_vector.c: don't overwrite error code

get_vaddr_frames() first calls pin_user_pages_fast() and if
that fails tries follow_pfn(). But if that also fails, then
the error code from pin_user_pages_fast() is overwritten with
the error code from follow_pfn().

Specifically if pin_user_pages_fast() returns -ENOMEM, then
follow_pfn() will overwrite that with -EINVAL, which is very
confusing.

So store the error code from pin_user_pages_fast() and return
that if follow_pfn() returns -EINVAL. -EINVAL indicates that
the page is unsuitable for follow_pfn, so pin_user_pages_fast()
was the correct call to make, and that error code should be
returned instead.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

diff --git a/drivers/media/common/videobuf2/frame_vector.c b/drivers/media/common/videobuf2/frame_vector.c
index ce879f6..542dde9 100644 (file)
--- a/drivers/media/common/videobuf2/frame_vector.c
+++ b/drivers/media/common/videobuf2/frame_vector.c
@@ -37,6 +37,7 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
 {
        struct mm_struct *mm = current->mm;
        struct vm_area_struct *vma;
+       int ret_pin_user_pages_fast = 0;
        int ret = 0;
        int err;
 
@@ -56,6 +57,7 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
                vec->is_pfns = false;
                goto out_unlocked;
        }
+       ret_pin_user_pages_fast = ret;
 
        mmap_read_lock(mm);
        vec->got_ref = false;
@@ -71,7 +73,18 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
                while (ret < nr_frames && start + PAGE_SIZE <= vma->vm_end) {
                        err = follow_pfn(vma, start, &nums[ret]);
                        if (err) {
-                               if (ret == 0)
+                               if (ret)
+                                       goto out;
+                               // If follow_pfn() returns -EINVAL, then this
+                               // is not an IO mapping or a raw PFN mapping.
+                               // In that case, return the original error from
+                               // pin_user_pages_fast(). Otherwise this
+                               // function would return -EINVAL when
+                               // pin_user_pages_fast() returned -ENOMEM,
+                               // which makes debugging hard.
+                               if (err == -EINVAL && ret_pin_user_pages_fast)
+                                       ret = ret_pin_user_pages_fast;
+                               else
                                        ret = err;
                                goto out;
                        }