scsi: mptfusion: Fix double fetch bug in ioctl
authorDan Carpenter <dan.carpenter@oracle.com>
Tue, 14 Jan 2020 12:34:14 +0000 (15:34 +0300)
committerMartin K. Petersen <martin.petersen@oracle.com>
Thu, 16 Jan 2020 04:05:52 +0000 (23:05 -0500)
commit28d76df18f0ad5bcf5fa48510b225f0ed262a99b
tree05416287cb5b208dae487eb8f47040e3b6fa8720
parent7b571c19d4c0b78d27dd3bf1f3c42e4032390af6
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>
drivers/message/fusion/mptctl.c