From patchwork Fri Dec 16 11:44:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sakari Ailus X-Patchwork-Id: 9477831 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 45D406047D for ; Fri, 16 Dec 2016 12:08:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3587528803 for ; Fri, 16 Dec 2016 12:08:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 252602882F; Fri, 16 Dec 2016 12:08:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6CDCC28823 for ; Fri, 16 Dec 2016 12:08:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757353AbcLPMIb (ORCPT ); Fri, 16 Dec 2016 07:08:31 -0500 Received: from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:50294 "EHLO hillosipuli.retiisi.org.uk" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752380AbcLPMI1 (ORCPT ); Fri, 16 Dec 2016 07:08:27 -0500 Received: from valkosipuli.retiisi.org.uk (valkosipuli.retiisi.org.uk [IPv6:2001:1bc8:1a6:d3d5::80:2]) by hillosipuli.retiisi.org.uk (Postfix) with ESMTP id 5D31A60096; Fri, 16 Dec 2016 13:45:14 +0200 (EET) Received: by valkosipuli.retiisi.org.uk (Postfix, from userid 1000) id 385E52040F; Fri, 16 Dec 2016 13:44:44 +0200 (EET) Date: Fri, 16 Dec 2016 13:44:44 +0200 From: Sakari Ailus To: Mauro Carvalho Chehab Cc: Greg KH , Javier Martinez Canillas , Laurent Pinchart , Shuah Khan , Linux Media Mailing List , Mauro Carvalho Chehab , Sakari Ailus Subject: Re: [PATCH RFC] omap3isp: prevent releasing MC too early Message-ID: <20161216114443.GI16630@valkosipuli.retiisi.org.uk> References: <20161214151406.20380-1-mchehab@s-opensource.com> <3043978.ViByGAdkJL@avalon> <20161215103734.716a0619@vento.lan> <20161215105716.30186ff5@vento.lan> <20161215134438.GA28343@kroah.com> <20161215120706.6cbed1de@vento.lan> <20161216082124.GG16630@valkosipuli.retiisi.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161216082124.GG16630@valkosipuli.retiisi.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi folks, On Fri, Dec 16, 2016 at 10:21:25AM +0200, Sakari Ailus wrote: ... > At least some of the issues the patches claim to fix are really not fixed > either. They are just made slightly less likely to accidentally stumble > upon. I couldn't immediately trigger this with the current mainline kernel as the remaining time window isn't very large. However making this trivial-looking change: And then unbinding the omap3-isp driver will immediately produce this. The same would take place on any driver implementing the Media controller interface. What happens there is that the memory backing the media device has been released while the IOCTL call was in progress. This can happen because device removal and IOCTL calls are not serialised nor the media device is reference counted --- the latter of which I believe is the right thing to do: [ 105.756408] Unable to handle kernel NULL pointer dereference at virtual address 00000140 [ 105.756561] pgd = eda94000 [ 105.756591] [00000140] *pgd=adaf8831, *pte=00000000, *ppte=00000000 [ 105.756683] Internal error: Oops: 17 [#1] PREEMPT ARM [ 105.756744] Modules linked in: smiapp smiapp_pll omap3_isp videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core v4l2_common videodev media [ 105.756927] CPU: 0 PID: 2276 Comm: media-ctl Not tainted 4.9.0-rc6-00494-g9244e38-dirty #830 [ 105.757019] Hardware name: Generic OMAP36xx (Flattened Device Tree) [ 105.757080] task: ed970380 task.stack: eda5e000 [ 105.757141] PC is at __lock_acquire+0x94/0x1868 [ 105.757202] LR is at lock_acquire+0x70/0x90 [ 105.757263] pc : [] lr : [] psr: 20000093 sp : eda5fcc0 ip : eda5e000 fp : 00000000 [ 105.757354] r10: 00000001 r9 : 00000000 r8 : 0002715c [ 105.757415] r7 : 00000140 r6 : 00000001 r5 : 60000013 r4 : ed970380 [ 105.757476] r3 : 00000001 r2 : 00000001 r1 : 00000000 r0 : 00000140 [ 105.757537] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none [ 105.757629] Control: 10c5387d Table: ada94019 DAC: 00000051 [ 105.757690] Process media-ctl (pid: 2276, stack limit = 0xeda5e208) [ 105.757751] Stack: (0xeda5fcc0 to 0xeda60000) [ 105.757812] fcc0: ed970868 00000001 a409df10 c355d08b c0c75714 c01602f4 60000013 00000001 [ 105.757904] fce0: ee1c9514 c0839b48 ee1c9518 effed360 ed970868 60000013 c0833e74 c0839b48 [ 105.757995] fd00: effed360 effed360 c0e9166c ee1c951c ee1c9518 00000000 60000013 00000001 [ 105.758087] fd20: 00000000 0002715c eda5e000 00000000 00000000 c01601ec 00000001 00000000 [ 105.758178] fd40: 00000000 bf000e78 00000000 00000000 0000010c bf000e78 ed970380 c04ed214 [ 105.758270] fd60: 00000001 00000000 bf000e78 c015aa50 ed9703b0 ed9703b0 c0830d50 00000000 [ 105.758361] fd80: ed970848 c0830d60 ed970380 ed970380 c0830d60 c01602f4 c0e76744 00000051 [ 105.758483] fda0: eda5fdd0 00000000 0002715c eda5fdd0 bf002e28 c1007c01 00000000 0002715c [ 105.758575] fdc0: eda5e000 00000000 00000000 bf000e78 00000001 50414d4f 53492033 43432050 [ 105.758666] fde0: 00003250 00000000 00000000 00000000 00000000 00020000 00000000 00000000 [ 105.758758] fe00: 00000000 00010002 00000000 00000000 00000000 00000000 00000051 00000007 [ 105.758850] fe20: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 105.758941] fe40: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 105.759033] fe60: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 105.759124] fe80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 105.759216] fea0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 105.759307] fec0: 00000000 00000000 00000000 00000000 ed967c80 bf000ddc ed967c80 0002715c [ 105.759399] fee0: c1007c01 00000003 00000000 bf001808 0002715c ed967c80 00000003 ed9fece0 [ 105.759490] ff00: 00000003 c01ea4c4 0002715c c01eb4a8 00000000 00000000 c01fba18 ffffffff [ 105.759582] ff20: 00000001 00000000 ee84f010 ed89e440 00000000 ee2654e8 00000010 c01dacdc [ 105.759674] ff40: 00000000 00000000 00000000 ed89e538 ed8a2b50 ed9707a0 ed970380 00000000 [ 105.759765] ff60: eda5ff7c eda5ff84 ed967c80 ed967c80 0002715c c1007c01 00000003 eda5e000 [ 105.759857] ff80: 00000000 c01eb560 0002715c 00027008 00000000 00000036 c0107d84 eda5e000 [ 105.759948] ffa0: 00000000 c0107be0 0002715c 00027008 00000003 c1007c01 0002715c 80000000 [ 105.760070] ffc0: 0002715c 00027008 00000000 00000036 00027158 00000000 b6f71000 00000000 [ 105.760162] ffe0: 00026b08 be9cf85c 000126a4 b6eaaa5c 20000010 00000003 fda7fddf fdbdffba [ 105.760253] [] (__lock_acquire) from [] (lock_acquire+0x70/0x90) [ 105.760375] [] (lock_acquire) from [] (mutex_lock_nested+0x54/0x3f4) [ 105.760528] [] (mutex_lock_nested) from [] (media_device_ioctl+0x9c/0x120 [media]) [ 105.760681] [] (media_device_ioctl [media]) from [] (media_ioctl+0x54/0x60 [media]) [ 105.760803] [] (media_ioctl [media]) from [] (vfs_ioctl+0x18/0x30) [ 105.760894] [] (vfs_ioctl) from [] (do_vfs_ioctl+0x90c/0x974) [ 105.760986] [] (do_vfs_ioctl) from [] (SyS_ioctl+0x50/0x6c) [ 105.761108] [] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x1c) [ 105.761199] Code: e59f3f58 e593300c e3530000 0a000003 (e5972000) [ 105.761260] ---[ end trace bdb47e9a97e34f03 ]--- diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c index f2772ba..6ec4125 100644 --- a/drivers/media/media-devnode.c +++ b/drivers/media/media-devnode.c @@ -32,6 +32,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include #include #include #include @@ -128,6 +129,8 @@ __media_ioctl(struct file *filp, unsigned int cmd, unsigned long arg, if (!media_devnode_is_registered(devnode)) return -EIO; + msleep(1000); + return ioctl_func(filp, cmd, arg); } And calling MEDIA_IOC_ENUM_ENTITIES (for instance) in a loop (v4l-utils): diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c index 1fd6525..5be3cea 100644 --- a/utils/media-ctl/libmediactl.c +++ b/utils/media-ctl/libmediactl.c @@ -522,7 +522,7 @@ static int media_enum_entities(struct media_device *media) entity->fd = -1; entity->info.id = id | MEDIA_ENT_ID_FLAG_NEXT; entity->media = media; - +while (1) ret = ioctl(media->fd, MEDIA_IOC_ENUM_ENTITIES, &entity->info); if (ret < 0) { ret = errno != EINVAL ? -errno : 0;