Commit 7fd4b41f053681cccf188cc1731ae43fe38fa969

Authored by Paul Cassella
Committed by Mauro Carvalho Chehab
1 parent 0d44b1235c

[media] ivtv: yuv: handle get_user_pages() -errno returns

get_user_pages() may return -errno, such as -EFAULT.  So don't blindly use
its return value as an offset into dma->map[] for the next get_user_pages()
call.  Since we'll give up and return an error if either fails, don't even
make the second call if the first failed to give us exactly what we were
looking for.

The old code would also call put_page() on as many elements of dma->map[]
as we'd asked for, regardless of how many were valid.

[Andy Walls modified this patch to return -EFAULT instead of -EINVAL
as Paul's observation "I'm not sure -EINVAL is the best return code vs
-EFAULT or -ENOMEM, [...]" was correct.  The return value bubbles up
as a return code for write(), for which the V4L2 API spec indicates
EINVAL is incorrect and EFAULT is correct.]

Signed-off-by: Paul Cassella <fortytwo-ivtv@maneteren.bigw.org>
Signed-off-by: Andy Walls <awalls@md.metrocast.net>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

Showing 1 changed file with 40 additions and 12 deletions Side-by-side Diff

drivers/media/video/ivtv/ivtv-yuv.c
... ... @@ -77,22 +77,50 @@
77 77 /* Get user pages for DMA Xfer */
78 78 down_read(&current->mm->mmap_sem);
79 79 y_pages = get_user_pages(current, current->mm, y_dma.uaddr, y_dma.page_count, 0, 1, &dma->map[0], NULL);
80   - uv_pages = get_user_pages(current, current->mm, uv_dma.uaddr, uv_dma.page_count, 0, 1, &dma->map[y_pages], NULL);
  80 + uv_pages = 0; /* silence gcc. value is set and consumed only if: */
  81 + if (y_pages == y_dma.page_count) {
  82 + uv_pages = get_user_pages(current, current->mm,
  83 + uv_dma.uaddr, uv_dma.page_count, 0, 1,
  84 + &dma->map[y_pages], NULL);
  85 + }
81 86 up_read(&current->mm->mmap_sem);
82 87  
83   - dma->page_count = y_dma.page_count + uv_dma.page_count;
  88 + if (y_pages != y_dma.page_count || uv_pages != uv_dma.page_count) {
  89 + int rc = -EFAULT;
84 90  
85   - if (y_pages + uv_pages != dma->page_count) {
86   - IVTV_DEBUG_WARN
87   - ("failed to map user pages, returned %d instead of %d\n",
88   - y_pages + uv_pages, dma->page_count);
  91 + if (y_pages == y_dma.page_count) {
  92 + IVTV_DEBUG_WARN
  93 + ("failed to map uv user pages, returned %d "
  94 + "expecting %d\n", uv_pages, uv_dma.page_count);
89 95  
90   - for (i = 0; i < dma->page_count; i++) {
91   - put_page(dma->map[i]);
  96 + if (uv_pages >= 0) {
  97 + for (i = 0; i < uv_pages; i++)
  98 + put_page(dma->map[y_pages + i]);
  99 + rc = -EFAULT;
  100 + } else {
  101 + rc = uv_pages;
  102 + }
  103 + } else {
  104 + IVTV_DEBUG_WARN
  105 + ("failed to map y user pages, returned %d "
  106 + "expecting %d\n", y_pages, y_dma.page_count);
92 107 }
93   - dma->page_count = 0;
94   - return -EINVAL;
  108 + if (y_pages >= 0) {
  109 + for (i = 0; i < y_pages; i++)
  110 + put_page(dma->map[i]);
  111 + /*
  112 + * Inherit the -EFAULT from rc's
  113 + * initialization, but allow it to be
  114 + * overriden by uv_pages above if it was an
  115 + * actual errno.
  116 + */
  117 + } else {
  118 + rc = y_pages;
  119 + }
  120 + return rc;
95 121 }
  122 +
  123 + dma->page_count = y_pages + uv_pages;
96 124  
97 125 /* Fill & map SG List */
98 126 if (ivtv_udma_fill_sg_list (dma, &uv_dma, ivtv_udma_fill_sg_list (dma, &y_dma, 0)) < 0) {