From patchwork Fri Jul 8 22:29:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shuah Khan X-Patchwork-Id: 9221849 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 602186089D for ; Fri, 8 Jul 2016 22:31:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4F33D27C14 for ; Fri, 8 Jul 2016 22:31:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 43B25285CB; Fri, 8 Jul 2016 22:31:45 +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=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 5292527C14 for ; Fri, 8 Jul 2016 22:31:44 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bLeHR-0000O0-D1; Fri, 08 Jul 2016 22:29:53 +0000 Received: from resqmta-po-05v.sys.comcast.net ([2001:558:fe16:19:96:114:154:164]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bLeHN-0000Lq-Pp for linux-arm-kernel@lists.infradead.org; Fri, 08 Jul 2016 22:29:50 +0000 Received: from resomta-po-10v.sys.comcast.net ([96.114.154.234]) by resqmta-po-05v.sys.comcast.net with SMTP id LeGobm0jP3MWRLeH1bgjvd; Fri, 08 Jul 2016 22:29:27 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=comcast.net; s=q20140121; t=1468016967; bh=UW6HnCm3PSyNUPqk2zXlLenCjre1/mlWKeHRCeh/W44=; h=Received:Received:Received:From:To:Subject:Date:Message-Id; b=au8oym8xyaFYMy+Q7z6o2Y5irnitgQXejETpxMpb5g3xGlKTCEOJmTk4Oi5xo6MGe D0d7mdFN2ClYM4QUfLOYUujmDWX9sHgfe8GejKan0LN1C0eBAOVN43VTtu+0024X3W +zsbOJVFdB6SeRIs6XYpgMMPpyHccq7ONxTP49U7jkU688F/+zzVScEbaBI6UHpGsa 6i4Isvxy9pFmqIq5ttdCS291WOopi5hKtn8SNdCLl1mxO45SBCFHt7XxmW/MPjdiCX xhT7S/lC9jxOHsLzVULHUFHoARtT6kbvR7VRBrzAs/UVTxXwxq5aDT+2PZwYTcQDpv nDsZPeu6s4WTw== Received: from mail.gonehiking.org ([73.181.52.62]) by comcast with SMTP id LeH0bAeVwPx66LeH0bEabK; Fri, 08 Jul 2016 22:29:27 +0000 Received: from shuah-XPS-13-9350.sisa.samsung.com (shuah-xps.internal [192.168.1.87]) by mail.gonehiking.org (Postfix) with ESMTP id 2C81D9F1C9; Fri, 8 Jul 2016 16:29:26 -0600 (MDT) From: Shuah Khan To: kyungmin.park@samsung.com, k.debski@samsung.com, jtp.park@samsung.com, mchehab@kernel.org, javier@osg.samsung.com Subject: [PATCH] media: s5p-mfc fix invalid memory access from s5p_mfc_release() Date: Fri, 8 Jul 2016 16:29:25 -0600 Message-Id: <1468016965-10880-1-git-send-email-shuahkh@osg.samsung.com> X-Mailer: git-send-email 2.7.4 X-CMAE-Envelope: MS4wfDDr07w70zNKCSc8FlBhkL2Oj/dKVXH7GyN5AszcNQcZgbl3ZPbnRK0HaJyUlA9hBgwVDTM8Co7YkCRtr7rThJ6Eb6pFQxS0yrfLfGGl7BSHvk2083VD x810szpTuaUYJXCmA33OhkubVZqmR+xk2oolQcrFlIPux0NsVdo9xneKLn80k+HC64oOoDUhDN1iNbwqrsFfJYG28zA8a5gchJFBXj0idobH0qV3OEx4kEda lMSipjTc58V7MmpaFnOJKCty/xrsl7IfKzBxhK2Ik6nw6v5yQPOpmcthqB/n6Fa54aMwgkngTPyeMkqMm49gqbESRVAFdynhuvG5gPtluHzpEEtBSSChU5K2 UC7GcVDp27PtVg0jGMkSRFfYn8lWt+PDrvG4m3DIei29BacdSoOdPqdVemFhyH3Hj85kB74mqy0xtxcUXhu9SfyQa4KDypLtDzZJCuLIjC8iny4v4pLv+gw3 dBuRe2TpBbtvxBn6OUJJqyYjx8oPwIcXX9MhQsGMmu8I+nhaiphINbhLzfc= X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160708_152949_911250_F4FF43B2 X-CRM114-Status: GOOD ( 13.52 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-kernel@vger.kernel.org, luisbg@osg.samsung.com, Shuah Khan , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP If s5p_mfc_release() runs after s5p_mfc_remove(), the former will access invalid s5p_mfc_dev pointer saved in the s5p_mfc_ctx and runs into kernel paging request errors. Clear ctx dev pointer in s5p_mfc_remove() and change s5p_mfc_release() to avoid work that requires ctx->dev. odroid kernel: Unable to handle kernel paging request at virtual address f17c1104 odroid kernel: pgd = ebca4000 odroid kernel: [f17c1104] *pgd=6e23d811, *pte=00000000, *ppte=00000000 odroid kernel: Internal error: Oops: 807 [#1] PREEMPT SMP ARM odroid kernel: Modules linked in: cpufreq_userspace cpufreq_powersave cpufreq_conservative s5p_mfc s5p_jpeg v4l2_mem2mem videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core v4l2_common videodev media odroid kernel: Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) odroid kernel: task: c2241400 ti: e7018000 task.ti: e7018000 odroid kernel: PC is at s5p_mfc_reset+0x40/0x28c [s5p_mfc] odroid kernel: LR is at s5p_mfc_reset+0x34/0x28c [s5p_mfc] odroid kernel: pc : [] lr : [] psr: 60010013 odroid kernel: [] (s5p_mfc_reset [s5p_mfc]) from [] (s5p_mfc_deinit_hw+0x14/0x3c [s5p_mfc]) odroid kernel: [] (s5p_mfc_deinit_hw [s5p_mfc]) from [] (s5p_mfc_release+0xac/0x1c4 [s5p_mfc]) odroid kernel: [] (s5p_mfc_release [s5p_mfc]) from [] (v4l2_release+0x38/0x74 [videodev]) odroid kernel: [] (v4l2_release [videodev]) from [] (__fput+0x80/0x1c8) odroid kernel: [] (__fput) from [] (task_work_run+0x94/0xc8) odroid kernel: [] (task_work_run) from [] (do_work_pending+0x7c/0xa4) odroid kernel: [] (do_work_pending) from [] (slow_work_pending+0xc/0x20) odroid kernel: Code: eb3edacc e5953078 e3a06000 e2833c11 (e5836004) Signed-off-by: Shuah Khan Tested-by: Luis de Bethencourt --- drivers/media/platform/s5p-mfc/s5p_mfc.c | 72 ++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 22 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index f537b74..c96421f 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -925,39 +925,50 @@ static int s5p_mfc_release(struct file *file) struct s5p_mfc_ctx *ctx = fh_to_ctx(file->private_data); struct s5p_mfc_dev *dev = ctx->dev; + /* if dev is null, do cleanup that doesn't need dev */ mfc_debug_enter(); - mutex_lock(&dev->mfc_mutex); + if (dev) + mutex_lock(&dev->mfc_mutex); s5p_mfc_clock_on(); vb2_queue_release(&ctx->vq_src); vb2_queue_release(&ctx->vq_dst); - /* Mark context as idle */ - clear_work_bit_irqsave(ctx); - /* If instance was initialised and not yet freed, - * return instance and free resources */ - if (ctx->state != MFCINST_FREE && ctx->state != MFCINST_INIT) { - mfc_debug(2, "Has to free instance\n"); - s5p_mfc_close_mfc_inst(dev, ctx); - } - /* hardware locking scheme */ - if (dev->curr_ctx == ctx->num) - clear_bit(0, &dev->hw_lock); - dev->num_inst--; - if (dev->num_inst == 0) { - mfc_debug(2, "Last instance\n"); - s5p_mfc_deinit_hw(dev); - del_timer_sync(&dev->watchdog_timer); - if (s5p_mfc_power_off() < 0) - mfc_err("Power off failed\n"); + if (dev) { + /* Mark context as idle */ + clear_work_bit_irqsave(ctx); + /* + * If instance was initialised and not yet freed, + * return instance and free resources + */ + if (ctx->state != MFCINST_FREE && ctx->state != MFCINST_INIT) { + mfc_debug(2, "Has to free instance\n"); + s5p_mfc_close_mfc_inst(dev, ctx); + } + /* hardware locking scheme */ + if (dev->curr_ctx == ctx->num) + clear_bit(0, &dev->hw_lock); + dev->num_inst--; + if (dev->num_inst == 0) { + mfc_debug(2, "Last instance\n"); + s5p_mfc_deinit_hw(dev); + del_timer_sync(&dev->watchdog_timer); + if (s5p_mfc_power_off() < 0) + mfc_err("Power off failed\n"); + } } mfc_debug(2, "Shutting down clock\n"); s5p_mfc_clock_off(); - dev->ctx[ctx->num] = NULL; + if (dev) + dev->ctx[ctx->num] = NULL; s5p_mfc_dec_ctrls_delete(ctx); v4l2_fh_del(&ctx->fh); - v4l2_fh_exit(&ctx->fh); + /* vdev is gone if dev is null */ + if (dev) + v4l2_fh_exit(&ctx->fh); kfree(ctx); mfc_debug_leave(); - mutex_unlock(&dev->mfc_mutex); + if (dev) + mutex_unlock(&dev->mfc_mutex); + return 0; } @@ -1309,9 +1320,26 @@ err_res: static int s5p_mfc_remove(struct platform_device *pdev) { struct s5p_mfc_dev *dev = platform_get_drvdata(pdev); + struct s5p_mfc_ctx *ctx; + int i; v4l2_info(&dev->v4l2_dev, "Removing %s\n", pdev->name); + /* + * Clear ctx dev pointer to avoid races between s5p_mfc_remove() + * and s5p_mfc_release() and s5p_mfc_release() accessing ctx->dev + * after s5p_mfc_remove() is run during unbind. + */ + mutex_lock(&dev->mfc_mutex); + for (i = 0; i < MFC_NUM_CONTEXTS; i++) { + ctx = dev->ctx[i]; + if (!ctx) + continue; + /* clear ctx->dev */ + ctx->dev = NULL; + } + mutex_unlock(&dev->mfc_mutex); + del_timer_sync(&dev->watchdog_timer); flush_workqueue(dev->watchdog_workqueue); destroy_workqueue(dev->watchdog_workqueue);