CVE-2020-12652 (C, published 2020-05-05T05:15Z)
Description: value: The __mptctl_ioctl function in drivers/message/fusion/mptctl.c in the Linux kernel before 5.4.14 allows local users to hold an incorrect lock during the ioctl operation and trigger a race condition, i.e., a "double fetch" vulnerability, aka CID-28d76df18f0a. NOTE: the vendor states "The security impact of this bug is not as bad as it could have been because these operations are all privileged and root already has enormous destructive power."
Severity: MEDIUM (CVSS v3 base score: 4.1)
Weakness: CWE-362 \u2014 Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition')
The product contains a code sequence that can run concurrently with other code, and the code sequence requires temporary, exclusive access to a shared resource, but a timing window exists in which the shared resource can be modified by another code sequence that is operating concurrently.
Repository: https://github.com/torvalds/linux
Files changed:
drivers/message/fusion/mptctl.c
Commit message: scsi: mptfusion: Fix double fetch bug in ioctl

Tom Hatskevich reported that we look up "iocp" then, in the called
functions we do a second copy_from_user() and look it up again.
The problem that could cause is:

drivers/message/fusion/mptctl.c
   674          /* All of these commands require an interrupt or
   675           * are unknown/illegal.
   676           */
   677          if ((ret = mptctl_syscall_down(iocp, nonblock)) != 0)
                                               ^^^^
We take this lock.

   678                  return ret;
   679
   680          if (cmd == MPTFWDOWNLOAD)
   681                  ret = mptctl_fw_download(arg);
                                                 ^^^
Then the user memory changes and we look up "iocp" again but a different
one so now we are holding the incorrect lock and have a race condition.

   682          else if (cmd == MPTCOMMAND)
   683                  ret = mptctl_mpt_command(arg);

The security impact of this bug is not as bad as it could have been
because these operations are all privileged and root already has
enormous destructive power.  But it's still worth fixing.

This patch passes the "iocp" pointer to the functions to avoid the
second lookup.  That deletes 100 lines of code from the driver so
it's a nice clean up as well.

Link: https://lore.kernel.org/r/20200114123414.GA7957@kadam
Reported-by: Tom Hatskevich <tom2001tom.23@gmail.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>


Vulnerable code:
static int mptctl_fw_download(unsigned long arg);
static int mptctl_getiocinfo(unsigned long arg, unsigned int cmd);
static int mptctl_gettargetinfo(unsigned long arg);
static int mptctl_readtest(unsigned long arg);
static int mptctl_mpt_command(unsigned long arg);
static int mptctl_eventquery(unsigned long arg);
static int mptctl_eventenable(unsigned long arg);
static int mptctl_eventreport(unsigned long arg);
static int mptctl_replace_fw(unsigned long arg);

static int mptctl_do_reset(unsigned long arg);
static int mptctl_hp_hostinfo(unsigned long arg, unsigned int cmd);
static int mptctl_hp_targetinfo(unsigned long arg);
static int mptctl_do_mpt_command(struct mpt_ioctl_command karg, void __user *mfPtr);
static int mptctl_do_fw_download(int ioc, char __user *ufwbuf, size_t fwlen);
		return mptctl_getiocinfo(arg, _IOC_SIZE(cmd));
		return mptctl_gettargetinfo(arg);
		return mptctl_readtest(arg);
		return mptctl_eventquery(arg);
		return mptctl_eventenable(arg);
		return mptctl_eventreport(arg);
		return mptctl_replace_fw(arg);
		ret = mptctl_fw_download(arg);
		ret = mptctl_mpt_command(arg);
		ret = mptctl_do_reset(arg);
		ret = mptctl_hp_hostinfo(arg, _IOC_SIZE(cmd));
		ret = mptctl_hp_targetinfo(arg);
static int mptctl_do_reset(unsigned long arg)
	MPT_ADAPTER		*iocp;
	if (mpt_verify_adapter(krinfo.hdr.iocnum, &iocp) < 0) {
		printk(KERN_DEBUG MYNAM "%s@%d::mptctl_do_reset - ioc%d not found!\n",
				__FILE__, __LINE__, krinfo.hdr.iocnum);
		return -ENODEV; /* (-6) No such device or address */
	}

mptctl_fw_download(unsigned long arg)
	return mptctl_do_fw_download(kfwdl.iocnum, kfwdl.bufp, kfwdl.fwlen);
mptctl_do_fw_download(int ioc, char __user *ufwbuf, size_t fwlen)
	MPT_ADAPTER		*iocp;
	if (mpt_verify_adapter(ioc, &iocp) < 0) {
		printk(KERN_DEBUG MYNAM "ioctl_fwdl - ioc%d not found!\n",
				 ioc);
		return -ENODEV; /* (-6) No such device or address */
	} else {

		/*  Valid device. Get a message frame and construct the FW download message.
	 	*/
		if ((mf = mpt_get_msg_frame(mptctl_id, iocp)) == NULL)
			return -EAGAIN;
	}
	dctlprintk(iocp, printk(MYIOC_s_DEBUG_FMT "DbG: kfwdl.ioc   = %04xh\n",
	    iocp->name, ioc));
mptctl_getiocinfo (unsigned long arg, unsigned int data_size)
	MPT_ADAPTER		*ioc;
	int			iocnum;
	if (((iocnum = mpt_verify_adapter(karg->hdr.iocnum, &ioc)) < 0) ||
	    (ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_getiocinfo() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		kfree(karg);
		return -ENODEV;
	}

mptctl_gettargetinfo (unsigned long arg)
	MPT_ADAPTER		*ioc;
	int			iocnum;
	if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
	    (ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_gettargetinfo() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		return -ENODEV;
	}

mptctl_readtest (unsigned long arg)
	MPT_ADAPTER *ioc;
	int iocnum;
	if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
	    (ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_readtest() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		return -ENODEV;
	}

mptctl_eventquery (unsigned long arg)
	MPT_ADAPTER *ioc;
	int iocnum;
	if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
	    (ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_eventquery() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		return -ENODEV;
	}

mptctl_eventenable (unsigned long arg)
	MPT_ADAPTER *ioc;
	int iocnum;
	if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
	    (ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_eventenable() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		return -ENODEV;
	}

mptctl_eventreport (unsigned long arg)
	MPT_ADAPTER		 *ioc;
	int			 iocnum;
	if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
	    (ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_eventreport() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		return -ENODEV;
	}
mptctl_replace_fw (unsigned long arg)
	MPT_ADAPTER		 *ioc;
	int			 iocnum;
	if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
	    (ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_replace_fw() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		return -ENODEV;
	}

mptctl_mpt_command (unsigned long arg)
	MPT_ADAPTER	*ioc;
	int		iocnum;
	if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
	    (ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_mpt_command() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		return -ENODEV;
	}

	rc = mptctl_do_mpt_command (karg, &uarg->MF);
mptctl_do_mpt_command (struct mpt_ioctl_command karg, void __user *mfPtr)
	MPT_ADAPTER	*ioc;
	int		iocnum, flagsLength;
	if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
	    (ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_do_mpt_command() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		return -ENODEV;
	}

mptctl_hp_hostinfo(unsigned long arg, unsigned int data_size)
	MPT_ADAPTER		*ioc;
	int			iocnum;
	if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
	    (ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_hp_hostinfo() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		return -ENODEV;
	}
mptctl_hp_targetinfo(unsigned long arg)
	MPT_ADAPTER		*ioc;
	int			iocnum;
	if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
		(ioc == NULL)) {
		printk(KERN_DEBUG MYNAM "%s::mptctl_hp_targetinfo() @%d - ioc%d not found!\n",
				__FILE__, __LINE__, iocnum);
		return -ENODEV;
	}
	ret = mptctl_do_fw_download(kfw.iocnum, kfw.bufp, kfw.fwlen);
	ret = mptctl_do_mpt_command (karg, &uarg->MF);

Fixed code:
static int mptctl_fw_download(MPT_ADAPTER *iocp, unsigned long arg);
static int mptctl_getiocinfo(MPT_ADAPTER *iocp, unsigned long arg, unsigned int cmd);
static int mptctl_gettargetinfo(MPT_ADAPTER *iocp, unsigned long arg);
static int mptctl_readtest(MPT_ADAPTER *iocp, unsigned long arg);
static int mptctl_mpt_command(MPT_ADAPTER *iocp, unsigned long arg);
static int mptctl_eventquery(MPT_ADAPTER *iocp, unsigned long arg);
static int mptctl_eventenable(MPT_ADAPTER *iocp, unsigned long arg);
static int mptctl_eventreport(MPT_ADAPTER *iocp, unsigned long arg);
static int mptctl_replace_fw(MPT_ADAPTER *iocp, unsigned long arg);

static int mptctl_do_reset(MPT_ADAPTER *iocp, unsigned long arg);
static int mptctl_hp_hostinfo(MPT_ADAPTER *iocp, unsigned long arg, unsigned int cmd);
static int mptctl_hp_targetinfo(MPT_ADAPTER *iocp, unsigned long arg);
static int mptctl_do_mpt_command(MPT_ADAPTER *iocp, struct mpt_ioctl_command karg, void __user *mfPtr);
static int mptctl_do_fw_download(MPT_ADAPTER *iocp, char __user *ufwbuf, size_t fwlen);
		return mptctl_getiocinfo(iocp, arg, _IOC_SIZE(cmd));
		return mptctl_gettargetinfo(iocp, arg);
		return mptctl_readtest(iocp, arg);
		return mptctl_eventquery(iocp, arg);
		return mptctl_eventenable(iocp, arg);
		return mptctl_eventreport(iocp, arg);
		return mptctl_replace_fw(iocp, arg);
		ret = mptctl_fw_download(iocp, arg);
		ret = mptctl_mpt_command(iocp, arg);
		ret = mptctl_do_reset(iocp, arg);
		ret = mptctl_hp_hostinfo(iocp, arg, _IOC_SIZE(cmd));
		ret = mptctl_hp_targetinfo(iocp, arg);
static int mptctl_do_reset(MPT_ADAPTER *iocp, unsigned long arg)
mptctl_fw_download(MPT_ADAPTER *iocp, unsigned long arg)
	return mptctl_do_fw_download(iocp, kfwdl.bufp, kfwdl.fwlen);
mptctl_do_fw_download(MPT_ADAPTER *iocp, char __user *ufwbuf, size_t fwlen)
	/*  Valid device. Get a message frame and construct the FW download message.
	*/
	if ((mf = mpt_get_msg_frame(mptctl_id, iocp)) == NULL)
		return -EAGAIN;
mptctl_getiocinfo (MPT_ADAPTER *ioc, unsigned long arg, unsigned int data_size)
mptctl_gettargetinfo (MPT_ADAPTER *ioc, unsigned long arg)
mptctl_readtest (MPT_ADAPTER *ioc, unsigned long arg)
mptctl_eventquery (MPT_ADAPTER *ioc, unsigned long arg)
mptctl_eventenable (MPT_ADAPTER *ioc, unsigned long arg)
mptctl_eventreport (MPT_ADAPTER *ioc, unsigned long arg)
mptctl_replace_fw (MPT_ADAPTER *ioc, unsigned long arg)
mptctl_mpt_command (MPT_ADAPTER *ioc, unsigned long arg)
	rc = mptctl_do_mpt_command (ioc, karg, &uarg->MF);
mptctl_do_mpt_command (MPT_ADAPTER *ioc, struct mpt_ioctl_command karg, void __user *mfPtr)
	int		flagsLength;
mptctl_hp_hostinfo(MPT_ADAPTER *ioc, unsigned long arg, unsigned int data_size)
mptctl_hp_targetinfo(MPT_ADAPTER *ioc, unsigned long arg)
	ret = mptctl_do_fw_download(iocp, kfw.bufp, kfw.fwlen);
	ret = mptctl_do_mpt_command (iocp, karg, &uarg->MF);