From patchwork Tue Sep 8 21:36:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 249378 Delivered-To: patch@linaro.org Received: by 2002:a92:5b9c:0:0:0:0:0 with SMTP id c28csp4917639ilg; Tue, 8 Sep 2020 14:38:01 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwueOA5+xOhLhko3XOkgawfxgmIufjReqd2ZwAmXMurwjouaPjer1xz5TnracQ+M+3VDRZF X-Received: by 2002:a17:906:a985:: with SMTP id jr5mr463321ejb.549.1599601081810; Tue, 08 Sep 2020 14:38:01 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1599601081; cv=none; d=google.com; s=arc-20160816; b=cueoCeL1jyPtCY3siIJObtvQMtxDtLL7vDi5KcWZ07zwHds2LTbmBl7cceVowxgqxG ItpdLEO9MOsskoF3JABOYn+cbcoaOhEu9+F/svQh5RigwuwLTu0fWU8DT5G/i24+LuVu DS5JjNyuNcP3gNbs8EhX3+zasps2JaAtf06+EkEapNoi3NaOrdRphgwjmgzfC6CHKksM YL7+QavQc+YLyEapAT+2VnNJ0B+UYAs1V1+9xdLAfjiGNK9ILED84lTJGyyr8XH2cvBE Dej8vyXmMM/yYXsLSwd5Z78AaTozwwfWDHLNdNsmoOYZbrNuUpxRCfs7Vdq8A9EtrBYX B5Fw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=6rlUi8XkwHRuMGj8qjCe5rLCzeZ63xWS26gHzZtyAMk=; b=ZZLThFWEpR+eKa6dK9JJs9C6EFv8PtSvly7eNvzBq60BvSLAZCdjMVEA2ttw9zbjWZ 1nA1gAM5V79c2KI/cyqOy43A1mPlYgVlD+IKPpBpKGGwaQ5bSb8OAor6+EhChqSKsB0F C/giJEtNdwemw3H49wZ8IeGF8c8Pp4tPFLyrA4EdfehqRtVnPKQUhnZ2loiL9B8x3Yzj yzfp4AQ6qvb3ryLcsEOg+6nML4Cc5hGkORC6a67bhDA0fQsPi/NHrn2m4pbtV66STy/q +0x1dzSfElfqx1qkjcGV5lcVTTH3ZHnSPdYiV3SWA1GISUZXPoyN5VrUVW4HcPkqBVHV eF2g== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-scsi-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-scsi-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id dn25si195813edb.268.2020.09.08.14.38.01 for ; Tue, 08 Sep 2020 14:38:01 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-scsi-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-scsi-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-scsi-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728248AbgIHVh5 (ORCPT ); Tue, 8 Sep 2020 17:37:57 -0400 Received: from mout.kundenserver.de ([212.227.126.131]:35399 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726002AbgIHVhz (ORCPT ); Tue, 8 Sep 2020 17:37:55 -0400 Received: from threadripper.lan ([149.172.98.151]) by mrelayeu.kundenserver.de (mreue012 [212.227.15.129]) with ESMTPA (Nemesis) id 1MSZDt-1k8m772Fsf-00Ssj4; Tue, 08 Sep 2020 23:37:20 +0200 From: Arnd Bergmann To: Adaptec OEM Raid Solutions , "James E.J. Bottomley" , "Martin K. Petersen" , Balsundar P Cc: hch@infradead.org, Arnd Bergmann , Zou Wei , Hannes Reinecke , Sagar Biradar , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/3] scsi: aacraid: improve compat_ioctl handlers Date: Tue, 8 Sep 2020 23:36:21 +0200 Message-Id: <20200908213715.3553098-1-arnd@arndb.de> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 X-Provags-ID: V03:K1:KRcdwEjkKj9+XoPTDAYicg3zaMW+8aDN7KFTDFdvtQylHXBrceI 4d8eunLAXcM9jP6dDtH+8xmH6N+5+Ao6vVxs4IdympM60xBQnobDY+FN5htDyO4Z9o/PVTC Svxotml9vbAurjzKdHkv2jkFKiF0mLWpVHpCF5JzIn8pjfrK0QYnzJy/PMMe41SHwCcSM1J mgL0yMlSogNhxz1j5/wsw== X-Spam-Flag: NO X-UI-Out-Filterresults: notjunk:1; V03:K0:Son/2bucWXE=:OTWFTbHTBU5W/x1+wSmX61 n854zU9YhMAAMMGHbpF+rRAAVaTaxn1hsv1mDzufM8Vv/1e87bT2fyl01TMoyyijVGG/IUSSN C/TRFFWsB9LLcWv11jwHRu8JwC3GSGyQ+HoZ2SMJfQ0IXMzWyCWK+I+wWG28/61dOKaSqDl9C CIjjK2kHy0im1xEZ+zoEH+Tqchj/DEE50sXWU7sAmmM7ao9y6Nk/1ckwc196Uif0Ahk/4xoVm tV73tyRXAezE/lt48vEN1BS8s1LsqP+SlKGFmq7FurWjtexBPF36ktnMxymszfz90J1pYd7Km larvBBPrb8QXfWl6K48iPnXGb9aQsZ2mS56fg0U8ANhsxLdajgpXelYeklp8az5XFiEIiJvhS 04fKPSN81HLvABlkGAppA+VdXY+epXySk2bVkjdv9r4w00EFMWaQI6YFNylTWPfPJktIiti/o 6pmpiIrgLWHUnmvbHvTNlaM7blRU1pXJAhm9wWwkKSqFyTK2VxHWNv6Kj6cxllRk/J8x1jl/O UmF6UGzlRZcvkVS/aLCehAN/L8+JI/i0ISqPWQqclnXWifOC3PQCpN/mkSkoWg6iMdstrW/47 oO424NllNUX+86z4cr2TgHs/rvdQusIdewL3WrIKMmJ2PIHz2uxWOKBpFfZRKggD9sf2ARhwY vnHAdDU0lX9dXVsW+gVfdVSoJNdsnasZFumOdZ/2uZ9pAhqkQJKecYDpuBZxNEKlS56y+d4Ix EwfVKiLlli97rjsMmOq98EFcUyzWd+SGvkQFz2U6O0bS0LLenxbDo39pXCogLCWummQ6ssaJL 0f5BVExDQaaTOOadR5zu/dJicHBqU/CtvPIsnzos7195rZ5x3t4OXrFrKRNs5tvbSmUaP3Y Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The use of compat_alloc_user_space() can be easily replaced by handling compat arguments in the regular handler, and this will make it work for big-endian kernels as well, which at the moment get an invalid indirect pointer argument. Calling aac_ioctl() instead of aac_compat_do_ioctl() means the compat and native code paths behave the same way again, which they stopped when the adapter health check was added only in the native function. Fixes: 572ee53a9bad ("scsi: aacraid: check adapter health") Signed-off-by: Arnd Bergmann --- drivers/scsi/aacraid/commctrl.c | 20 +++++++++-- drivers/scsi/aacraid/linit.c | 61 ++------------------------------- 2 files changed, 20 insertions(+), 61 deletions(-) -- 2.27.0 Reviewed-by: Christoph Hellwig Reviewed-by: Christoph Hellwig diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c index 59e82a832042..6d6f72d68164 100644 --- a/drivers/scsi/aacraid/commctrl.c +++ b/drivers/scsi/aacraid/commctrl.c @@ -25,6 +25,7 @@ #include #include #include +#include #include /* ssleep prototype */ #include #include @@ -243,8 +244,23 @@ static int next_getadapter_fib(struct aac_dev * dev, void __user *arg) struct list_head * entry; unsigned long flags; - if(copy_from_user((void *)&f, arg, sizeof(struct fib_ioctl))) - return -EFAULT; + if (in_compat_syscall()) { + struct compat_fib_ioctl { + u32 fibctx; + s32 wait; + compat_uptr_t fib; + } cf; + + if (copy_from_user(&f, arg, sizeof(struct compat_fib_ioctl))) + return -EFAULT; + + f.fibctx = cf.fibctx; + f.wait = cf.wait; + f.fib = compat_ptr(cf.fib); + } else { + if (copy_from_user(&f, arg, sizeof(struct fib_ioctl))) + return -EFAULT; + } /* * Verify that the HANDLE passed in was a valid AdapterFibContext * diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c index 8588da0a0655..8c0f55845138 100644 --- a/drivers/scsi/aacraid/linit.c +++ b/drivers/scsi/aacraid/linit.c @@ -1182,63 +1182,6 @@ static long aac_cfg_ioctl(struct file *file, return aac_do_ioctl(aac, cmd, (void __user *)arg); } -#ifdef CONFIG_COMPAT -static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned long arg) -{ - long ret; - switch (cmd) { - case FSACTL_MINIPORT_REV_CHECK: - case FSACTL_SENDFIB: - case FSACTL_OPEN_GET_ADAPTER_FIB: - case FSACTL_CLOSE_GET_ADAPTER_FIB: - case FSACTL_SEND_RAW_SRB: - case FSACTL_GET_PCI_INFO: - case FSACTL_QUERY_DISK: - case FSACTL_DELETE_DISK: - case FSACTL_FORCE_DELETE_DISK: - case FSACTL_GET_CONTAINERS: - case FSACTL_SEND_LARGE_FIB: - ret = aac_do_ioctl(dev, cmd, (void __user *)arg); - break; - - case FSACTL_GET_NEXT_ADAPTER_FIB: { - struct fib_ioctl __user *f; - - f = compat_alloc_user_space(sizeof(*f)); - ret = 0; - if (clear_user(f, sizeof(*f))) - ret = -EFAULT; - if (copy_in_user(f, (void __user *)arg, sizeof(struct fib_ioctl) - sizeof(u32))) - ret = -EFAULT; - if (!ret) - ret = aac_do_ioctl(dev, cmd, f); - break; - } - - default: - ret = -ENOIOCTLCMD; - break; - } - return ret; -} - -static int aac_compat_ioctl(struct scsi_device *sdev, unsigned int cmd, - void __user *arg) -{ - struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata; - if (!capable(CAP_SYS_RAWIO)) - return -EPERM; - return aac_compat_do_ioctl(dev, cmd, (unsigned long)arg); -} - -static long aac_compat_cfg_ioctl(struct file *file, unsigned cmd, unsigned long arg) -{ - if (!capable(CAP_SYS_RAWIO)) - return -EPERM; - return aac_compat_do_ioctl(file->private_data, cmd, arg); -} -#endif - static ssize_t aac_show_model(struct device *device, struct device_attribute *attr, char *buf) { @@ -1523,7 +1466,7 @@ static const struct file_operations aac_cfg_fops = { .owner = THIS_MODULE, .unlocked_ioctl = aac_cfg_ioctl, #ifdef CONFIG_COMPAT - .compat_ioctl = aac_compat_cfg_ioctl, + .compat_ioctl = aac_cfg_ioctl, #endif .open = aac_cfg_open, .llseek = noop_llseek, @@ -1536,7 +1479,7 @@ static struct scsi_host_template aac_driver_template = { .info = aac_info, .ioctl = aac_ioctl, #ifdef CONFIG_COMPAT - .compat_ioctl = aac_compat_ioctl, + .compat_ioctl = aac_ioctl, #endif .queuecommand = aac_queuecommand, .bios_param = aac_biosparm, From patchwork Tue Sep 8 21:36:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 257685 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2781BC43461 for ; Tue, 8 Sep 2020 21:38:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DF3032080C for ; Tue, 8 Sep 2020 21:38:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729037AbgIHViJ (ORCPT ); Tue, 8 Sep 2020 17:38:09 -0400 Received: from mout.kundenserver.de ([212.227.126.187]:48247 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728647AbgIHViG (ORCPT ); Tue, 8 Sep 2020 17:38:06 -0400 Received: from threadripper.lan ([149.172.98.151]) by mrelayeu.kundenserver.de (mreue012 [212.227.15.129]) with ESMTPA (Nemesis) id 1MYcy3-1k1cR223AB-00Vdt2; Tue, 08 Sep 2020 23:37:51 +0200 From: Arnd Bergmann To: Kashyap Desai , Sumit Saxena , Shivasharan S , "James E.J. Bottomley" , "Martin K. Petersen" Cc: hch@infradead.org, Arnd Bergmann , stable@vger.kernel.org, Anand Lodnoor , Chandrakanth Patil , Hannes Reinecke , megaraidlinux.pdl@broadcom.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 2/3] scsi: megaraid_sas: check user-provided offsets Date: Tue, 8 Sep 2020 23:36:22 +0200 Message-Id: <20200908213715.3553098-2-arnd@arndb.de> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200908213715.3553098-1-arnd@arndb.de> References: <20200908213715.3553098-1-arnd@arndb.de> MIME-Version: 1.0 X-Provags-ID: V03:K1:ySbOIHgYtcPCmDgKFJWQFKZzEpToys/t268vTJCmMrSGnITz77V xl5OAKtcncF9BqiN5rvTw8tDPiLZYdRujzLHGB3QOrcmuo53qW/gJlHNEKphYn0nYEAXhPt wpToVMv/Hyxv9/DTHreJq0bmwBQYh7+jRsM64wZyVdDwgI9bJDS3FXlC0d2H6XnCCEwTBFq TIT3+6Lq7J3BhphtiWgmA== X-UI-Out-Filterresults: notjunk:1; V03:K0:Yd64zGE+X4E=:xdJ2yudVuGKJoDcfhgjFlS 6vAp1av9q/HZUaGSmDDq2yVfr/RppdX2z8N6ZJ5lY2HgBs/5B6bYzlDL9TfTglgoM8utkS5ri LySDmoFHheBCtm9M8k4/ooxgV88XhwSikIF2dp9EA+D7I24mnn8Tt0/WjBhW06KDH9hggWY6Y +YkkyIOwgyECscXQTgnXQ1zwdtj9JmcViO/If2siZI8LJ7STwkvubJNV8alGmW6N9JQAebxyi +PJQcXFP3iUst6slq7UEpbuuVUwtJSs6LcfsGYi1N3CGgb8QGpALMxT9vfPnG94OTkpE+JIsT Av4GT0xdy67csPFsRqXoTlPxX2qLsyTptj4ytlrClRvlvf7EYKH4v1SE/yNw8ziCGovE0/Awv EIpcsSLVZMWrK8ZnFlPyEUeAnSjb0CO5QmBijC18tXh1HY8q5JCZupbYoqwx5b/gZpLrm10iV zQ+Qg5sA0ROBOHPVY8TeTXp8e2yzxacvkvW84pc0lLRiNxOVQlSphhS8thj4vIiq748FVpRYy /QI99VdO0M18FPyuFo76jTH9wy2KjsFQ7KZgm+UL/EmK6x56uZU6nUG6s06seNPMihd9iFPxZ w3UPXEcGq8qjLYsffUOUdJHGH0NvYYOr/YkbfySXLOs5FUWTufkFHSYB/5/RtSdynotBKJdHI Dz3++XKwS4GU8ynCnIBshzjCQR40yhThSfWSj6v81PLN+YqFgH85OtfJ5TOYB38i7aVy0UIzz kBRhA6g6GAaxJdkgpuDiGuor938PcPxaEnF1afRNuTLLoN3E4652cGMNWwTQU1u4Ia4fVPIYu xJTKNZTzRbWC78+oHOTfSNqumIpv1BFE3vb6Gnb2HUj21/jE51eWSexkqjNdiEbbguAFvvf Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org It sounds unwise to let user space pass an unchecked 32-bit offset into a kernel structure in an ioctl. This is an unsigned variable, so checking the upper bound for the size of the structure it points into is sufficient to avoid data corruption, but as the pointer might also be unaligned, it has to be written carefully as well. While I stumbled over this problem by reading the code, I did not continue checking the function for further problems like it. Cc: stable@vger.kernel.org Signed-off-by: Arnd Bergmann --- drivers/scsi/megaraid/megaraid_sas_base.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 861f7140f52e..c3de69f3bee8 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -8095,7 +8095,7 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance, int error = 0, i; void *sense = NULL; dma_addr_t sense_handle; - unsigned long *sense_ptr; + void *sense_ptr; u32 opcode = 0; int ret = DCMD_SUCCESS; @@ -8218,6 +8218,12 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance, } if (ioc->sense_len) { + /* make sure the pointer is part of the frame */ + if (ioc->sense_off > (sizeof(union megasas_frame) - sizeof(__le64))) { + error = -EINVAL; + goto out; + } + sense = dma_alloc_coherent(&instance->pdev->dev, ioc->sense_len, &sense_handle, GFP_KERNEL); if (!sense) { @@ -8225,12 +8231,11 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance, goto out; } - sense_ptr = - (unsigned long *) ((unsigned long)cmd->frame + ioc->sense_off); + sense_ptr = (void *)cmd->frame + ioc->sense_off; if (instance->consistent_mask_64bit) - *sense_ptr = cpu_to_le64(sense_handle); + put_unaligned_le64(sense_handle, sense_ptr); else - *sense_ptr = cpu_to_le32(sense_handle); + put_unaligned_le32(sense_handle, sense_ptr); } /* From patchwork Tue Sep 8 21:36:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 249380 Delivered-To: patch@linaro.org Received: by 2002:a92:5b9c:0:0:0:0:0 with SMTP id c28csp4918028ilg; Tue, 8 Sep 2020 14:38:36 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw4uoIp6sFwHh3bxIy86x3uk7Kz4lwrWG0MPMjU5hyAcgDZg5UaBQLK2nrCHFx6rFt01FNk X-Received: by 2002:a17:906:2cc2:: with SMTP id r2mr474062ejr.482.1599601116502; Tue, 08 Sep 2020 14:38:36 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1599601116; cv=none; d=google.com; s=arc-20160816; b=sCHqLd8oEco5DY35htWVZ2cmuTCbWRaW5QYM8Sz6Cn75UWrVis/7Dw1tdXagXPklOO sr7MwIGfokEzaHR8gMQgA2P5ivzTQnKajV65joGukjhlWp/mFjkMGF3KjeeeSccmcxg2 y27VTPztgfaz5ubTlX5twqmTvgk7kaOOxF2fb3IhCMXDuJCuBjQSCALPG+qHXsldJ+UQ WxxLVoxTT6f83U7antNlkkAfhV4xs51N1jYh1UIztf772ZDun8P/leg/LrJfa1jtgXZz O/OGD38gl3E4nshob77dzF1zGnuZzfCV9Jf0g90ckTqB5OjpDoDITQkxjjJwA7fNphr5 w5BQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=ikFkP+m5OMPLdpij20ebQIqhEinRlH9DsPx/ChzuCdU=; b=gWVyavhuEt88B223niaKjfdjE+b3V7WCukq/BVOnUsnZUyqBy6DGwVpNY0DNkFMhlQ AAUrEwxWmWAFnDRbk96gYAPo7xKsbqPjglLf5geIWLNyjbU4vLE6ZrkKgdPveuMVlwyj DQxP5KKOYrE+Gz3y/FdKlbqXvpfG1R9DcIrIeCq3jcHm1zh1bqScJQzcCHG0+hxiYb3k /IbfPlSpv3vq2bKZybzzXjSYT2mBbsuZbxkrGZHEXCIp3ONwXdaFHoQfc4dhc6arzsv4 bKCUYmRayAJFbSi1YrhoIEfTt29k+NKv4Vr2+lqe1rDfo80n+ZbrT5PLc6vuDpa0c19J T3lg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-scsi-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-scsi-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id c15si211456edf.376.2020.09.08.14.38.36 for ; Tue, 08 Sep 2020 14:38:36 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-scsi-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-scsi-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-scsi-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729404AbgIHViZ (ORCPT ); Tue, 8 Sep 2020 17:38:25 -0400 Received: from mout.kundenserver.de ([212.227.126.133]:48831 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727935AbgIHViY (ORCPT ); Tue, 8 Sep 2020 17:38:24 -0400 Received: from threadripper.lan ([149.172.98.151]) by mrelayeu.kundenserver.de (mreue012 [212.227.15.129]) with ESMTPA (Nemesis) id 1MqqLB-1kstV21FIX-00mtWU; Tue, 08 Sep 2020 23:38:09 +0200 From: Arnd Bergmann To: Kashyap Desai , Sumit Saxena , Shivasharan S , "James E.J. Bottomley" , "Martin K. Petersen" Cc: hch@infradead.org, Arnd Bergmann , Anand Lodnoor , Chandrakanth Patil , Hannes Reinecke , megaraidlinux.pdl@broadcom.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 3/3] scsi: megaraid_sas: simplify compat_ioctl handling Date: Tue, 8 Sep 2020 23:36:23 +0200 Message-Id: <20200908213715.3553098-3-arnd@arndb.de> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200908213715.3553098-1-arnd@arndb.de> References: <20200908213715.3553098-1-arnd@arndb.de> MIME-Version: 1.0 X-Provags-ID: V03:K1:ALhk5O7zdQzPn/jSuPH+SFbSayui4FFXnnw3ouKbjiE+KJxxXHJ xTxJi6sZWQIF4ClRlZEyqVEjbZQrQzE/7GtswSVjmEX5iezDeeLnkgAsigIUUoU84vaWzJ/ XfqSldp+Aqx7jqXiwLuESijum4eObbxC2HXq5l8oMRagi7UsEIMRTpkDN96Lnbh5vxCe+Xm fAN3SeB+lK3H23/4YpSNA== X-Spam-Flag: NO X-UI-Out-Filterresults: notjunk:1; V03:K0:gB8irALDMJw=:O/YhdWxQkADB+Kl5gwn9kK ULGoh+6wFVyrYrbCLUDAcR5M+1j5jvMV+30keUMGxoMjhBnuap8ump34sCJ1/F8lqsn6fBLOM HzTjjyPrdWXzkgfjZg6zV3X4JEvhWuy1PEp4r0Ihj2NXWU+NxSmlcxN0P4XfvxaLJZmgW7+Ew Cw7hDArxfg4AWPSpMI5i7zF1fpguUH+LBhcUNRmy54GWqeVyoAUAO4dLqESraIVy7lRxF8/kz bhMi0n4ynGT7WLNfrcuHE1S6lomqNxW1QO343qR8v+X4fYS5KwUFSeOOnBqvwE6U1ZqpOwnZv MPTWNnWmg9ARuNwoqzlY7zd5EUK4UgDCkcOLYif1mu8InCZ/3Pz6oIGDVGVPuzVMwPff+Cq+w LP+J147saJjDS/2DSoxBvUIv0l2pNrfMT0J7GpdKtCUZsBIBN1EwlknytXCaHkeHJUlK6X2eG 6nv3ZHodzrI2gRluLXArKC/OjJ7infDW38Dd4MGv9t0xb1bL6bHyPtZGdA0ALXFp2RhowVE6h YwQn8U/B7YM1aOMe2Gl+femd5JuozUsp/UcCiqeewdA2MZGu5Ki2vvjuJ4CDjR1qFVzT8yExf 2S4bUbzWQ5ywhF/qbf1QinxmCkUaAVGl2zVJguno14PDYRPi8Emk6WOY87AAObgbaWp1NUjpu Gr9wQh5VG/JdhdSD7nqdekyqaCfF2xvF7D3Ar22XnJzrlA2JIacKv+XCWtk2XyAItZ4E5O6EF XZ1cT9721+czN0iIjBhP/ht0gclQeDu5X5JYQUT8tudhx3dKRRGKYSHnWdLDQEVi/57wmz3Js Yk/BbXJ84rlufntVXgaTt7ZJyNhEnpxDRT9Jseg09i1rHfSVfNBIsEuV/KVd3iG7ZNf24cy Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org There have been several attempts to fix serious problems in the compat handling in megasas_mgmt_compat_ioctl_fw(), and it also uses the compat_alloc_user_space() function. Folding the compat handling into the regular ioctl function with in_compat_syscall() simplifies it a lot and avoids some of the remaining problems: - missing handling of unaligned pointers - overflowing the ioc->frame.raw array from invalid input - compat_alloc_user_space() Signed-off-by: Arnd Bergmann --- drivers/scsi/megaraid/megaraid_sas_base.c | 123 ++++++++++------------ 1 file changed, 56 insertions(+), 67 deletions(-) -- 2.27.0 diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index c3de69f3bee8..601dab468823 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -8331,6 +8331,56 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance, return error; } +static struct megasas_iocpacket *megasas_compat_iocpacket_get_user(void __user *arg) +{ + int err = -EFAULT; +#ifdef CONFIG_COMPAT + struct megasas_iocpacket *ioc; + struct compat_megasas_iocpacket __user *cioc = arg; + int i; + + ioc = kzalloc(sizeof(*ioc), GFP_KERNEL); + if (copy_from_user(ioc, arg, + offsetof(struct megasas_iocpacket, frame) + 128)) + goto out; + + /* + * The sense_ptr is used in megasas_mgmt_fw_ioctl only when + * sense_len is not null, so prepare the 64bit value under + * the same condition. + */ + if (ioc->sense_len) { + compat_uptr_t *sense_ioc_ptr; + void __user *sense_cioc; + + /* make sure the pointer is inside of frame.raw */ + if (ioc->sense_off > + (sizeof(ioc->frame.raw) - sizeof(void __user*))) { + err = -EINVAL; + goto out; + } + + sense_ioc_ptr = (compat_uptr_t *)&ioc->frame.raw[ioc->sense_off]; + sense_cioc = compat_ptr(get_unaligned(sense_ioc_ptr)); + put_unaligned((unsigned long)sense_cioc, (void **)sense_ioc_ptr); + } + + for (i = 0; i < MAX_IOCTL_SGE; i++) { + compat_uptr_t iov_base; + if (get_user(iov_base, &cioc->sgl[i].iov_base) || + get_user(ioc->sgl[i].iov_len, &cioc->sgl[i].iov_len)) { + goto out; + } + ioc->sgl[i].iov_base = compat_ptr(iov_base); + } + + return ioc; +out: + kfree(ioc); +#endif + return ERR_PTR(err); +} + static int megasas_mgmt_ioctl_fw(struct file *file, unsigned long arg) { struct megasas_iocpacket __user *user_ioc = @@ -8339,7 +8389,11 @@ static int megasas_mgmt_ioctl_fw(struct file *file, unsigned long arg) struct megasas_instance *instance; int error; - ioc = memdup_user(user_ioc, sizeof(*ioc)); + if (in_compat_syscall()) + ioc = megasas_compat_iocpacket_get_user(user_ioc); + else + ioc = memdup_user(user_ioc, sizeof(struct megasas_iocpacket)); + if (IS_ERR(ioc)) return PTR_ERR(ioc); @@ -8444,78 +8498,13 @@ megasas_mgmt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) } #ifdef CONFIG_COMPAT -static int megasas_mgmt_compat_ioctl_fw(struct file *file, unsigned long arg) -{ - struct compat_megasas_iocpacket __user *cioc = - (struct compat_megasas_iocpacket __user *)arg; - struct megasas_iocpacket __user *ioc = - compat_alloc_user_space(sizeof(struct megasas_iocpacket)); - int i; - int error = 0; - compat_uptr_t ptr; - u32 local_sense_off; - u32 local_sense_len; - u32 user_sense_off; - - if (clear_user(ioc, sizeof(*ioc))) - return -EFAULT; - - if (copy_in_user(&ioc->host_no, &cioc->host_no, sizeof(u16)) || - copy_in_user(&ioc->sgl_off, &cioc->sgl_off, sizeof(u32)) || - copy_in_user(&ioc->sense_off, &cioc->sense_off, sizeof(u32)) || - copy_in_user(&ioc->sense_len, &cioc->sense_len, sizeof(u32)) || - copy_in_user(ioc->frame.raw, cioc->frame.raw, 128) || - copy_in_user(&ioc->sge_count, &cioc->sge_count, sizeof(u32))) - return -EFAULT; - - /* - * The sense_ptr is used in megasas_mgmt_fw_ioctl only when - * sense_len is not null, so prepare the 64bit value under - * the same condition. - */ - if (get_user(local_sense_off, &ioc->sense_off) || - get_user(local_sense_len, &ioc->sense_len) || - get_user(user_sense_off, &cioc->sense_off)) - return -EFAULT; - - if (local_sense_off != user_sense_off) - return -EINVAL; - - if (local_sense_len) { - void __user **sense_ioc_ptr = - (void __user **)((u8 *)((unsigned long)&ioc->frame.raw) + local_sense_off); - compat_uptr_t *sense_cioc_ptr = - (compat_uptr_t *)(((unsigned long)&cioc->frame.raw) + user_sense_off); - if (get_user(ptr, sense_cioc_ptr) || - put_user(compat_ptr(ptr), sense_ioc_ptr)) - return -EFAULT; - } - - for (i = 0; i < MAX_IOCTL_SGE; i++) { - if (get_user(ptr, &cioc->sgl[i].iov_base) || - put_user(compat_ptr(ptr), &ioc->sgl[i].iov_base) || - copy_in_user(&ioc->sgl[i].iov_len, - &cioc->sgl[i].iov_len, sizeof(compat_size_t))) - return -EFAULT; - } - - error = megasas_mgmt_ioctl_fw(file, (unsigned long)ioc); - - if (copy_in_user(&cioc->frame.hdr.cmd_status, - &ioc->frame.hdr.cmd_status, sizeof(u8))) { - printk(KERN_DEBUG "megasas: error copy_in_user cmd_status\n"); - return -EFAULT; - } - return error; -} - static long megasas_mgmt_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { switch (cmd) { case MEGASAS_IOC_FIRMWARE32: - return megasas_mgmt_compat_ioctl_fw(file, arg); + return megasas_mgmt_ioctl_fw(file, arg); case MEGASAS_IOC_GET_AEN: return megasas_mgmt_ioctl_aen(file, arg); }