| Message ID | 20231003091315.3919528-1-make_ruc2021@163.com |
|---|---|
| State | Accepted |
| Commit | feec1b29d7a81be3c1a0a0865eed67d05d4089c9 |
| Headers | show |
| Series | [v2] media: videobuf2: Fix IS_ERR checking in vb2_vmalloc_put_userptr() | expand |
On Tue, Oct 3, 2023 at 6:13 PM Ma Ke <make_ruc2021@163.com> wrote: > > In order to avoid error pointers from frame_vector_pages(), we could > use IS_ERR() to check the return value to fix this. This checking > operation could make sure that vector contains pages. > > Signed-off-by: Ma Ke <make_ruc2021@163.com> > --- > drivers/media/common/videobuf2/videobuf2-vmalloc.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c > index 7c635e292106..5aa66305546d 100644 > --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c > +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c > @@ -133,13 +133,15 @@ static void vb2_vmalloc_put_userptr(void *buf_priv) > > if (!buf->vec->is_pfns) { > n_pages = frame_vector_count(buf->vec); > - pages = frame_vector_pages(buf->vec); > if (vaddr) > vm_unmap_ram((void *)vaddr, n_pages); > if (buf->dma_dir == DMA_FROM_DEVICE || > - buf->dma_dir == DMA_BIDIRECTIONAL) > - for (i = 0; i < n_pages; i++) > - set_page_dirty_lock(pages[i]); > + buf->dma_dir == DMA_BIDIRECTIONAL){ Missing space between ) and { . > + pages = frame_vector_pages(buf->vec); > + if (!WARN_ON_ONCE(IS_ERR(pages))) I still think this cannot happen, but the frame vector code is a mess, so being a bit more defensive here is probably fine, until someone can clean up that code (probably by removing the PFN-only code path completely). > + for (i = 0; i < n_pages; i++) > + set_page_dirty_lock(pages[i]); > + } > } else { > iounmap((__force void __iomem *)buf->vaddr); > } > -- > 2.37.2 > With that one formatting issue fixed: (Mauro, Hans, maybe it could be fixed up when applying?) Acked-by: Tomasz Figa <tfiga@chromium.org> Best regards, Tomasz
diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c index 7c635e292106..5aa66305546d 100644 --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c @@ -133,13 +133,15 @@ static void vb2_vmalloc_put_userptr(void *buf_priv) if (!buf->vec->is_pfns) { n_pages = frame_vector_count(buf->vec); - pages = frame_vector_pages(buf->vec); if (vaddr) vm_unmap_ram((void *)vaddr, n_pages); if (buf->dma_dir == DMA_FROM_DEVICE || - buf->dma_dir == DMA_BIDIRECTIONAL) - for (i = 0; i < n_pages; i++) - set_page_dirty_lock(pages[i]); + buf->dma_dir == DMA_BIDIRECTIONAL){ + pages = frame_vector_pages(buf->vec); + if (!WARN_ON_ONCE(IS_ERR(pages))) + for (i = 0; i < n_pages; i++) + set_page_dirty_lock(pages[i]); + } } else { iounmap((__force void __iomem *)buf->vaddr); }
In order to avoid error pointers from frame_vector_pages(), we could use IS_ERR() to check the return value to fix this. This checking operation could make sure that vector contains pages. Signed-off-by: Ma Ke <make_ruc2021@163.com> --- drivers/media/common/videobuf2/videobuf2-vmalloc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)