Same patch was already posted and is waiting to be merged: https://patchwork.linuxtv.org/patch/52944/ Regards, Hans On 11/17/2018 12:42 AM, Sudip Mukherjee wrote: > When we are using __find_plane_by_offset() to find the matching plane > number and the buffer, the queue is not locked. So, after we have > calculated the buffer number and assigned the pointer to vb, it can get > freed. And if that happens then we get a use-after-free when we try to > use this for the mmap and get the following calltrace: > > [ 30.623259] Call Trace: > [ 30.623531] dump_stack+0x244/0x39d > [ 30.623914] ? dump_stack_print_info.cold.1+0x20/0x20 > [ 30.624439] ? printk+0xa7/0xcf > [ 30.624777] ? kmsg_dump_rewind_nolock+0xe4/0xe4 > [ 30.625265] print_address_description.cold.7+0x9/0x1ff > [ 30.625809] kasan_report.cold.8+0x242/0x309 > [ 30.626263] ? vb2_mmap+0x712/0x790 > [ 30.626637] __asan_report_load8_noabort+0x14/0x20 > [ 30.627201] vb2_mmap+0x712/0x790 > [ 30.627642] ? vb2_poll+0x1d0/0x1d0 > [ 30.628060] vb2_fop_mmap+0x4b/0x70 > [ 30.628458] v4l2_mmap+0x153/0x200 > [ 30.628929] mmap_region+0xe85/0x1cd0 > > Lock the queue before we start finding the matching plane and buffer so > that there is no chance of the memory being freed while we are about > to use it. > > Reported-by: syzbot+be93025dd45dccd8923c@xxxxxxxxxxxxxxxxxxxxxxxxx > Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@xxxxxxxxx> > --- > drivers/media/common/videobuf2/videobuf2-core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index 975ff5669f72..a81320566e02 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -2125,9 +2125,12 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) > /* > * Find the plane corresponding to the offset passed by userspace. > */ > + mutex_lock(&q->mmap_lock); > ret = __find_plane_by_offset(q, off, &buffer, &plane); > - if (ret) > + if (ret) { > + mutex_unlock(&q->mmap_lock); > return ret; > + } > > vb = q->bufs[buffer]; > > @@ -2138,12 +2141,12 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) > */ > length = PAGE_ALIGN(vb->planes[plane].length); > if (length < (vma->vm_end - vma->vm_start)) { > + mutex_unlock(&q->mmap_lock); > dprintk(1, > "MMAP invalid, as it would overflow buffer length\n"); > return -EINVAL; > } > > - mutex_lock(&q->mmap_lock); > ret = call_memop(vb, mmap, vb->planes[plane].mem_priv, vma); > mutex_unlock(&q->mmap_lock); > if (ret) >
