| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <10ac9c65-e12b-6b7b-cb54-7d1379244428@xs4all.nl>
Date: Mon, 16 Oct 2017 17:18:58 +0200
From: Hans Verkuil <hverkuil@...all.nl>
To: Shuah Khan <shuahkh@....samsung.com>, mchehab@...nel.org,
hansverk@...co.com, kgene@...nel.org, krzk@...nel.org,
s.nawrocki@...sung.com, shailendra.v@...sung.com, shuah@...nel.org,
Julia.Lawall@...6.fr, kyungmin.park@...sung.com, kamil@...as.org,
jtp.park@...sung.com, a.hajda@...sung.com
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 2/2] media: s5p-mfc: fix lockdep warning
On 10/16/2017 05:16 PM, Shuah Khan wrote:
> The driver mmap functions shouldn't take lock when calling vb2_mmap().
> Fix it to not take the lock.
>
> Reference: commit log for f035eb4e976ef5a059e30bc91cfd310ff030a7d3
> and e752577ed7bf55c81e10343fced8b378cda2b63b
>
> The following lockdep warning is fixed with this change.
>
> [ 2106.181412] ======================================================
> [ 2106.187563] WARNING: possible circular locking dependency detected
> [ 2106.193718] 4.14.0-rc2-00002-gfab205f-dirty #4 Not tainted
> [ 2106.199175] ------------------------------------------------------
> [ 2106.205328] qtdemux0:sink/2614 is trying to acquire lock:
> [ 2106.210701] (&dev->mfc_mutex){+.+.}, at: [<bf175544>] s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
> [ 2106.218672]
> [ 2106.218672] but task is already holding lock:
> [ 2106.224477] (&mm->mmap_sem){++++}, at: [<c01df2e4>] vm_mmap_pgoff+0x44/0xb8
> [ 2106.231497]
> [ 2106.231497] which lock already depends on the new lock.
> [ 2106.231497]
> [ 2106.239642]
> [ 2106.239642] the existing dependency chain (in reverse order) is:
> [ 2106.247095]
> [ 2106.247095] -> #1 (&mm->mmap_sem){++++}:
> [ 2106.252473] __might_fault+0x80/0xb0
> [ 2106.256567] video_usercopy+0x1cc/0x510 [videodev]
> [ 2106.261845] v4l2_ioctl+0xa4/0xdc [videodev]
> [ 2106.266596] do_vfs_ioctl+0xa0/0xa18
> [ 2106.270667] SyS_ioctl+0x34/0x5c
> [ 2106.274395] ret_fast_syscall+0x0/0x28
> [ 2106.278637]
> [ 2106.278637] -> #0 (&dev->mfc_mutex){+.+.}:
> [ 2106.284186] lock_acquire+0x6c/0x88
> [ 2106.288173] __mutex_lock+0x68/0xa34
> [ 2106.292244] mutex_lock_interruptible_nested+0x1c/0x24
> [ 2106.297893] s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
> [ 2106.302747] v4l2_mmap+0x54/0x88 [videodev]
> [ 2106.307409] mmap_region+0x3a8/0x638
> [ 2106.311480] do_mmap+0x330/0x3a4
> [ 2106.315207] vm_mmap_pgoff+0x90/0xb8
> [ 2106.319279] SyS_mmap_pgoff+0x90/0xc0
> [ 2106.323439] ret_fast_syscall+0x0/0x28
> [ 2106.327683]
> [ 2106.327683] other info that might help us debug this:
> [ 2106.327683]
> [ 2106.335656] Possible unsafe locking scenario:
> [ 2106.335656]
> [ 2106.341548] CPU0 CPU1
> [ 2106.346053] ---- ----
> [ 2106.350559] lock(&mm->mmap_sem);
> [ 2106.353939] lock(&dev->mfc_mutex);
> [ 2106.353939] lock(&dev->mfc_mutex);
> [ 2106.365897] lock(&dev->mfc_mutex);
> [ 2106.369450]
> [ 2106.369450] *** DEADLOCK ***
> [ 2106.369450]
> [ 2106.375344] 1 lock held by qtdemux0:sink/2614:
> [ 2106.379762] #0: (&mm->mmap_sem){++++}, at: [<c01df2e4>] vm_mmap_pgoff+0x44/0xb8
> [ 2106.387214]
> [ 2106.387214] stack backtrace:
> [ 2106.391550] CPU: 7 PID: 2614 Comm: qtdemux0:sink Not tainted 4.14.0-rc2-00002-gfab205f-dirty #4
> [ 2106.400213] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [ 2106.406285] [<c01102c8>] (unwind_backtrace) from [<c010cabc>] (show_stack+0x10/0x14)
> [ 2106.413995] [<c010cabc>] (show_stack) from [<c08543a4>] (dump_stack+0x98/0xc4)
> [ 2106.421187] [<c08543a4>] (dump_stack) from [<c016b2fc>] (print_circular_bug+0x254/0x410)
> [ 2106.429245] [<c016b2fc>] (print_circular_bug) from [<c016c580>] (check_prev_add+0x468/0x938)
> [ 2106.437651] [<c016c580>] (check_prev_add) from [<c016f4dc>] (__lock_acquire+0x1314/0x14fc)
> [ 2106.445883] [<c016f4dc>] (__lock_acquire) from [<c016fefc>] (lock_acquire+0x6c/0x88)
> [ 2106.453596] [<c016fefc>] (lock_acquire) from [<c0869fb4>] (__mutex_lock+0x68/0xa34)
> [ 2106.461221] [<c0869fb4>] (__mutex_lock) from [<c086aa08>] (mutex_lock_interruptible_nested+0x1c/0x24)
> [ 2106.470425] [<c086aa08>] (mutex_lock_interruptible_nested) from [<bf175544>] (s5p_mfc_mmap+0x28/0xd4 [s5p_mfc])
> [ 2106.480494] [<bf175544>] (s5p_mfc_mmap [s5p_mfc]) from [<bf037120>] (v4l2_mmap+0x54/0x88 [videodev])
> [ 2106.489575] [<bf037120>] (v4l2_mmap [videodev]) from [<c01f4798>] (mmap_region+0x3a8/0x638)
> [ 2106.497875] [<c01f4798>] (mmap_region) from [<c01f4d58>] (do_mmap+0x330/0x3a4)
> [ 2106.505068] [<c01f4d58>] (do_mmap) from [<c01df330>] (vm_mmap_pgoff+0x90/0xb8)
> [ 2106.512260] [<c01df330>] (vm_mmap_pgoff) from [<c01f28cc>] (SyS_mmap_pgoff+0x90/0xc0)
> [ 2106.520059] [<c01f28cc>] (SyS_mmap_pgoff) from [<c0108820>] (ret_fast_syscall+0x0/0x28)
>
> Signed-off-by: Shuah Khan <shuahkh@....samsung.com>
> Suggested-by: Hans Verkuil <hansverk@...co.com>
> Acked-by: Marek Szyprowski <m.szyprowski@...sung.com>
Acked-by: Hans Verkuil <hansverk@...co.com>
Regards,
Hans
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 4c253fb..40c18b4 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1047,8 +1047,6 @@ static int s5p_mfc_mmap(struct file *file, struct vm_area_struct *vma)
> unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
> int ret;
>
> - if (mutex_lock_interruptible(&dev->mfc_mutex))
> - return -ERESTARTSYS;
> if (offset < DST_QUEUE_OFF_BASE) {
> mfc_debug(2, "mmaping source\n");
> ret = vb2_mmap(&ctx->vq_src, vma);
> @@ -1057,7 +1055,6 @@ static int s5p_mfc_mmap(struct file *file, struct vm_area_struct *vma)
> vma->vm_pgoff -= (DST_QUEUE_OFF_BASE >> PAGE_SHIFT);
> ret = vb2_mmap(&ctx->vq_dst, vma);
> }
> - mutex_unlock(&dev->mfc_mutex);
> return ret;
> }
>
>
Powered by blists - more mailing lists