From: Wenwen Wang <wang6495@umn.edu> To: Wenwen Wang <wang6495@umn.edu> Cc: Kangjie Lu <kjlu@umn.edu>, Sathya Prakash <sathya.prakash@broadcom.com>, Chaitra P B <chaitra.basappa@broadcom.com>, Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>, "James E.J. Bottomley" <jejb@linux.vnet.ibm.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, MPT-FusionLinux.pdl@broadcom.com (open list:LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)), linux-scsi@vger.kernel.org (open list:LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)), linux-kernel@vger.kernel.org (open list) Subject: [PATCH] scsi: mpt3sas: fix a missing-check bug Date: Sat, 5 May 2018 01:31:35 -0500 [thread overview] Message-ID: <1525501896-8235-1-git-send-email-wang6495@umn.edu> (raw) In _ctl_ioctl_main(), 'ioctl_header' is first copied from the userspace pointer 'arg'. 'ioctl_header.ioc_number' is then verified by _ctl_verify_adapter(). If the verification is failed, an error code -ENODEV is returned. Otherwise, the verification result, i.e., the MPT3SAS adapter that matches with the 'ioctl_header.ioc_number', is saved to 'ioc'. Later on, if the 'cmd' is MPT3COMMAND, the whole ioctl command struct is copied again from the userspace pointer 'arg' and saved to 'karg'. Then the function _ctl_do_mpt_command() is invoked to execute the command with the adapter 'ioc' and 'karg' as inputs. Given that the pointer 'arg' resides in userspace, a malicious userspace process can race to change the 'ioc_number' between the two copies, which will cause inconsistency issues, potentially security issues since an inconsistent adapter could be used with the command struct 'karg' as inputs of _ctl_do_mpt_command(). Moreover, the user can potentially provide a valid 'ioc_number' to pass the verification, and then modify it to an invalid 'ioc_number'. That means, an invalid 'ioc_number' can potentially bypass the verification check. To fix this issue, we need to recheck the 'ioc_number' copied after the second copy to make sure it is not changed since the first copy. Signed-off-by: Wenwen Wang <wang6495@umn.edu> --- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c index d3cb387..0c140c7 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c @@ -2388,6 +2388,11 @@ _ctl_ioctl_main(struct file *file, unsigned int cmd, void __user *arg, break; } + if (karg.hdr.ioc_number != ioctl_header.ioc_number) { + ret = -EINVAL; + break; + } + if (_IOC_SIZE(cmd) == sizeof(struct mpt3_ioctl_command)) { uarg = arg; ret = _ctl_do_mpt_command(ioc, karg, &uarg->mf); -- 2.7.4
WARNING: multiple messages have this Message-ID (diff)
From: Wenwen Wang <wang6495@umn.edu> To: Wenwen Wang <wang6495@umn.edu> Cc: Kangjie Lu <kjlu@umn.edu>, Sathya Prakash <sathya.prakash@broadcom.com>, Chaitra P B <chaitra.basappa@broadcom.com>, Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>, "James E.J. Bottomley" <jejb@linux.vnet.ibm.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, "open list:LSILOGIC MPT FUSION DRIVERS FC/SAS/SPI" <MPT-FusionLinux.pdl@broadcom.com>, "open list:LSILOGIC MPT FUSION DRIVERS FC/SAS/SPI" <linux-scsi@vger.kernel.org>, open list <linux-kernel@vger.kernel.org> Subject: [PATCH] scsi: mpt3sas: fix a missing-check bug Date: Sat, 5 May 2018 01:31:35 -0500 [thread overview] Message-ID: <1525501896-8235-1-git-send-email-wang6495@umn.edu> (raw) In _ctl_ioctl_main(), 'ioctl_header' is first copied from the userspace pointer 'arg'. 'ioctl_header.ioc_number' is then verified by _ctl_verify_adapter(). If the verification is failed, an error code -ENODEV is returned. Otherwise, the verification result, i.e., the MPT3SAS adapter that matches with the 'ioctl_header.ioc_number', is saved to 'ioc'. Later on, if the 'cmd' is MPT3COMMAND, the whole ioctl command struct is copied again from the userspace pointer 'arg' and saved to 'karg'. Then the function _ctl_do_mpt_command() is invoked to execute the command with the adapter 'ioc' and 'karg' as inputs. Given that the pointer 'arg' resides in userspace, a malicious userspace process can race to change the 'ioc_number' between the two copies, which will cause inconsistency issues, potentially security issues since an inconsistent adapter could be used with the command struct 'karg' as inputs of _ctl_do_mpt_command(). Moreover, the user can potentially provide a valid 'ioc_number' to pass the verification, and then modify it to an invalid 'ioc_number'. That means, an invalid 'ioc_number' can potentially bypass the verification check. To fix this issue, we need to recheck the 'ioc_number' copied after the second copy to make sure it is not changed since the first copy. Signed-off-by: Wenwen Wang <wang6495@umn.edu> --- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c index d3cb387..0c140c7 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c @@ -2388,6 +2388,11 @@ _ctl_ioctl_main(struct file *file, unsigned int cmd, void __user *arg, break; } + if (karg.hdr.ioc_number != ioctl_header.ioc_number) { + ret = -EINVAL; + break; + } + if (_IOC_SIZE(cmd) == sizeof(struct mpt3_ioctl_command)) { uarg = arg; ret = _ctl_do_mpt_command(ioc, karg, &uarg->mf); -- 2.7.4
next reply other threads:[~2018-05-05 6:31 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-05-05 6:31 Wenwen Wang [this message] 2018-05-05 6:31 ` [PATCH] scsi: mpt3sas: fix a missing-check bug Wenwen Wang 2018-05-09 5:28 ` Wenwen Wang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1525501896-8235-1-git-send-email-wang6495@umn.edu \ --to=wang6495@umn.edu \ --cc=MPT-FusionLinux.pdl@broadcom.com \ --cc=chaitra.basappa@broadcom.com \ --cc=jejb@linux.vnet.ibm.com \ --cc=kjlu@umn.edu \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=sathya.prakash@broadcom.com \ --cc=suganath-prabu.subramani@broadcom.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.