From patchwork Fri Jun 9 21:36:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 103542 Delivered-To: patch@linaro.org Received: by 10.140.91.77 with SMTP id y71csp399417qgd; Fri, 9 Jun 2017 14:36:49 -0700 (PDT) X-Received: by 10.84.216.29 with SMTP id m29mr42378757pli.223.1497044209073; Fri, 09 Jun 2017 14:36:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1497044209; cv=none; d=google.com; s=arc-20160816; b=ZHKoWeUhbr3z57M4+L31MSYpVf1Jd32PEQVjonhnq+vWUq2XaL5lAoqVoVvnpF3XkE brtKZvjk4zpfO8XAGmfIMARe5DsMBB9msR3K+hFiA7FPWQMcb1NlU6qlSiYDcP5GAzyV 7UbTEkxAcP0KF/7jhiW4WniE5Ox7vNN9Ydt9l/SYSdibuw6FTJnHu7qYPW5HuyvGzBok ff2qx/mIH+Kgs1f8ukXD+WVtmCsNidBTdbM1JjcvH2EuDnj2u/myY+mhvku1jk43xC9/ JgJEsrxUiJHGBrASpIa8GAQNTRvm1P/j72XkgNF004dQ9gbD0R+yiM3yTbn70EtjcQf4 Pq5Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :arc-authentication-results; bh=to7MGE2yt9V/z1A6H6bmMeyUu0OKdbQ8ldvy5VSkmpU=; b=b5Q8owMpSGGDBTU46CdaNtipQa66m5T2wS/HsuhV24XLzzsUZ8iTJ9Bzi5q5jIpyqO 05/9nCUgR3605Xgz0fMMTXkjWBYsQHWmS7N8Wgu1mP9BGsMeF1oNTnOkTTwLxPAUIWXy C3XMPRXijgYAcQEtiaYcjjjeGqm1nS4PdhQtawLr+nNKjKXMgDV7mlPjKaWqF68eqx0z NGqnn6NdEnaeRR8GAXuik6UVwu7Wq1Fd7uQryKHJ/LrATRlZ7f98qKrluAWc1QZ6FnhL sugC4qKMX4j53BhNil+B1mCnrCgy2LYgrra4v1akDVoqTbLeguPWZQZG3Eeu6cOzYmdq qXWg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-media-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-media-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id p19si7087207pgd.257.2017.06.09.14.36.48; Fri, 09 Jun 2017 14:36:49 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-media-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-media-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-media-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751612AbdFIVg1 (ORCPT + 4 others); Fri, 9 Jun 2017 17:36:27 -0400 Received: from mout.kundenserver.de ([212.227.17.24]:52710 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751554AbdFIVg1 (ORCPT ); Fri, 9 Jun 2017 17:36:27 -0400 Received: from wuerfel.lan ([78.42.17.5]) by mrelayeu.kundenserver.de (mreue101 [212.227.15.145]) with ESMTPA (Nemesis) id 0MZDVU-1da11i0L88-00L1WH; Fri, 09 Jun 2017 23:36:18 +0200 From: Arnd Bergmann To: "Lad, Prabhakar" , Mauro Carvalho Chehab Cc: Kevin Hilman , Sekhar Nori , Arnd Bergmann , linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] [media] davinci/dm644x: work around ccdc_update_raw_params trainwreck Date: Fri, 9 Jun 2017 23:36:04 +0200 Message-Id: <20170609213616.410415-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 X-Provags-ID: V03:K0:Bt0kZthl5ZWe6zwcaU4ddUkp6yJIdhLQwV9T61uZ4xhwc9dl5TK Nms6P1GPpB7RfIgelUphae3UdUX7Z+rIy7OPNefzr12yljqv1r0Y1fZsaj4JT32LyiquVBN 1S5Ysw7gqY2Gs8/9O7vkNNgn81sDXc9XRJ0fQ/iTW4Rnh96ultdIP1N+ZQdzf7XSPMVWBKX k04lT29OQ35AhSMQduVnQ== X-UI-Out-Filterresults: notjunk:1; V01:K0:/q+srTuB8xU=:vunTa7eilH//zfXdbDOxbW JPdWL5p+aBG8ONXfMxcNqXzPx98QxVq3ducLaCdRBziFFfniJkluYXdpLoOHb5mJAZ6oLLSev Y9LpRTrBX3UBmb8U5AT2l65uFTAAIGU/LX1lyWiCupQjoqobZVrqolwpqQkoZtKKJXz5555sC iYobyUgFRI4vT2BcVdTIZk4D3Ly8eeV6mH7f7sXXfgmy+4lvrfjNJU00vbSEMdv0sA8dwNuBT TV8un3iO1ORYU4qT+K+x2pJKC3jocYBufQd57TtzXZ8vj+Gbei9utxkUerdOneWAJjSvgkXbg 5JwCyqLPU81WYR+W+YAjkdAtaLzo6TfNBRZoX3QwEUbgTyWF1b0lWzmI7rMD5rYH4g+5+cC3P qmgbKumCDf0xZtuFAmBAcxSzhA0r1dEofaxbPmoIEZcLfgX8NNeZdCuj3KbgSMOiy+7bjscUY FodvbNMc3JQJBo8FQW9zPV7L8QPMzgr06MQLpVndsrF6woWTZlnVi8f1i5Uw9DiYSY3h2eCgb sbBfadoiYPVCZ7wISxAv06SNuNeM1ZQ8Nh7Vi/G9iQMCsiVQKJvdX/Qyj9hekyMAGxa6EBpUG s9jCrz6OC3WOddJwSvJZ4n5WdgomjqsPZ1UodCggQspXMjSy4kkh08aT5wypxL0wV/b/ytcpk Blg0X65cNCH2ywTuv4PxgDXK/CbCo9QKZiKoUh3eLJ8muhkURaO3goKujxvDyGADT2GU= Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Now that the davinci drivers can be enabled in compile tests on other architectures, I ran into this warning on a 64-bit build: drivers/media/platform/davinci/dm644x_ccdc.c: In function 'ccdc_update_raw_params': drivers/media/platform/davinci/dm644x_ccdc.c:279:7: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] While that looks fairly harmless (it would be fine on 32-bit), it was just the tip of the iceberg: - The function constantly mixes up pointers and phys_addr_t numbers - This is part of a 'VPFE_CMD_S_CCDC_RAW_PARAMS' ioctl command that is described as an 'experimental ioctl that will change in future kernels', but if we have users that probably won't happen. - The code to allocate the table never gets called after we copy_from_user the user input over the kernel settings, and then compare them for inequality. - We then go on to use an address provided by user space as both the __user pointer for input and pass it through phys_to_virt to come up with a kernel pointer to copy the data to. This looks like a trivially exploitable root hole. This patch disables all the obviously broken code, by zeroing out the sensitive data provided by user space. I also fix the type confusion here. If we think the ioctl has no stable users, we could consider just removing it instead. Fixes: 5f15fbb68fd7 ("V4L/DVB (12251): v4l: dm644x ccdc module for vpfe capture driver") Signed-off-by: Arnd Bergmann --- drivers/media/platform/davinci/dm644x_ccdc.c | 40 +++++++++++++++++----------- 1 file changed, 25 insertions(+), 15 deletions(-) -- 2.9.0 diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c b/drivers/media/platform/davinci/dm644x_ccdc.c index 740fbc7a8c14..1b42f50cad38 100644 --- a/drivers/media/platform/davinci/dm644x_ccdc.c +++ b/drivers/media/platform/davinci/dm644x_ccdc.c @@ -236,10 +236,22 @@ static int ccdc_update_raw_params(struct ccdc_config_params_raw *raw_params) { struct ccdc_config_params_raw *config_params = &ccdc_cfg.bayer.config_params; - unsigned int *fpc_virtaddr = NULL; - unsigned int *fpc_physaddr = NULL; + unsigned int *fpc_virtaddr; + phys_addr_t fpc_physaddr; memcpy(config_params, raw_params, sizeof(*raw_params)); + + /* + * FIXME: the code to copy the fault_pxl settings was present + * in the original version but clearly could never + * work and will interpret user-provided data in + * dangerous ways. Let's disable it completely to be + * on the safe side. + */ + config_params->fault_pxl.enable = 0; + config_params->fault_pxl.fp_num = 0; + config_params->fault_pxl.fpc_table_addr = 0; + /* * allocate memory for fault pixel table and copy the user * values to the table @@ -247,16 +259,15 @@ static int ccdc_update_raw_params(struct ccdc_config_params_raw *raw_params) if (!config_params->fault_pxl.enable) return 0; - fpc_physaddr = (unsigned int *)config_params->fault_pxl.fpc_table_addr; - fpc_virtaddr = (unsigned int *)phys_to_virt( - (unsigned long)fpc_physaddr); + fpc_physaddr = config_params->fault_pxl.fpc_table_addr; + fpc_virtaddr = (unsigned int *)phys_to_virt(fpc_physaddr); /* * Allocate memory for FPC table if current * FPC table buffer is not big enough to * accommodate FPC Number requested */ if (raw_params->fault_pxl.fp_num != config_params->fault_pxl.fp_num) { - if (fpc_physaddr != NULL) { + if (fpc_physaddr) { free_pages((unsigned long)fpc_virtaddr, get_order (config_params->fault_pxl.fp_num * @@ -270,13 +281,12 @@ static int ccdc_update_raw_params(struct ccdc_config_params_raw *raw_params) fault_pxl.fp_num * FP_NUM_BYTES)); - if (fpc_virtaddr == NULL) { + if (fpc_virtaddr) { dev_dbg(ccdc_cfg.dev, "\nUnable to allocate memory for FPC"); return -EFAULT; } - fpc_physaddr = - (unsigned int *)virt_to_phys((void *)fpc_virtaddr); + fpc_physaddr = virt_to_phys(fpc_virtaddr); } /* Copy number of fault pixels and FPC table */ @@ -287,7 +297,7 @@ static int ccdc_update_raw_params(struct ccdc_config_params_raw *raw_params) dev_dbg(ccdc_cfg.dev, "\n copy_from_user failed"); return -EFAULT; } - config_params->fault_pxl.fpc_table_addr = (unsigned long)fpc_physaddr; + config_params->fault_pxl.fpc_table_addr = fpc_physaddr; return 0; } @@ -295,13 +305,13 @@ static int ccdc_close(struct device *dev) { struct ccdc_config_params_raw *config_params = &ccdc_cfg.bayer.config_params; - unsigned int *fpc_physaddr = NULL, *fpc_virtaddr = NULL; + phys_addr_t fpc_physaddr; + unsigned int *fpc_virtaddr; - fpc_physaddr = (unsigned int *)config_params->fault_pxl.fpc_table_addr; + fpc_physaddr = config_params->fault_pxl.fpc_table_addr; - if (fpc_physaddr != NULL) { - fpc_virtaddr = (unsigned int *) - phys_to_virt((unsigned long)fpc_physaddr); + if (fpc_physaddr) { + fpc_virtaddr = phys_to_virt(fpc_physaddr); free_pages((unsigned long)fpc_virtaddr, get_order(config_params->fault_pxl.fp_num * FP_NUM_BYTES));