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: 9221843 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 8895F6089D for ; Fri, 8 Jul 2016 22:29:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7840627C14 for ; Fri, 8 Jul 2016 22:29:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6CCE0285CB; Fri, 8 Jul 2016 22:29:51 +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.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=unavailable 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 ED6E627C14 for ; Fri, 8 Jul 2016 22:29:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756470AbcGHW3b (ORCPT ); Fri, 8 Jul 2016 18:29:31 -0400 Received: from resqmta-po-04v.sys.comcast.net ([96.114.154.163]:54380 "EHLO resqmta-po-04v.sys.comcast.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756418AbcGHW33 (ORCPT ); Fri, 8 Jul 2016 18:29:29 -0400 Received: from resomta-po-10v.sys.comcast.net ([96.114.154.234]) by resqmta-po-04v.sys.comcast.net with SMTP id LeGPbLee8wN2qLeH1b3FXi; 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=uGvx2Rw3Ncasi6O4kXZPkFpsjOa3dkFCpbjiZU5sIGHX/yexhFeLi8dB5EMmXswIq s3Fc20hClHCEaVZ0VVgzbBxqbEv+gjksMcxzUNuC4qv/Kr5snAITU/+V2RmyFh2w+V 2SgORsSE8quNL8XlKeRSJHClvCw3Fj9Kq+oAx7ZXI4c5HxL0ako8YgJKnnFvASk6OC nGGnCrghYEwTluhTCGYC9L9NN075cnovC4vC5xdXII77hjQPXrYOplnsTGeZsDRrjl SzGrHGSazltcXoj7+CJSzMBWD5mRDTsI07XfzNAdJIcmTJAk7hRlueWTTaS4YtX6Tp C3v/tC6tkyl/Q== 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 Cc: Shuah Khan , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, luisbg@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= 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 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);