lsilogic mpt fusion: mptctl: Fixed race condition in mptctl.ko concerning mptctl_id variable among parallel functions variable among parallel functions
Subsystems (2)
PATCH v4 1/1 lsilogic mpt fusion: mptctl: Fixed race condition in mptctl.ko concerning mptctl_id variable among parallel functions variable among parallel functions
Certain functions in the driver, such as mptctl_do_fw_download() and mptctl_do_mpt_command(), rely on the instance of mptctl_id, which does the id-ing. There is race condition possible when these functions operate in concurrency. Via, mutexes and global/local variable setting, the functions are mutually signalled to cooperate.
---
Changelog v2
Lacked a version number but added properly terminated else condition at (former) line 2300.
Changelog v3
Fixes "return -EAGAIN" lines which were erroneously tabbed as if to be guarded by "if" conditions lying above them.
Changelog v4
Removes a pair of needless mutexes in init() function, allows for checking of NULL
condition outside of race-fixing mutexes in functions mptctl_do_mpt_command(), mptctl_do_fw_command, and mptctl_hp_hostinfo().
drivers/message/fusion/mptctl.c | 59 ++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 16 deletions(-)
drivers/message/fusion/mptctl.c +43 -16
@@ -816,12 +816,17 @@ mptctl_do_fw_download(int ioc, 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 ) +mutex_lock (&mpctl_mutex); + mf =mpt_get_msg_frame (mptctl_id, iocp); +mutex_unlock (&mpctl_mutex); +if (mf ==NULL ) {return -EAGAIN ; + } } - +mutex_lock (&mpctl_mutex);dctlprintk (iocp,printk (MYIOC_s_DEBUG_FMT"mptctl_do_fwdl called. mptctl_id = %xh.\n" , iocp->name, mptctl_id)); +mutex_unlock (&mpctl_mutex);dctlprintk (iocp,printk (MYIOC_s_DEBUG_FMT"DbG: kfwdl.bufp = %p\n" , iocp->name, ufwbuf)); dctlprintk(iocp, printk(MYIOC_s_DEBUG_FMT "DbG: kfwdl.fwlen = %d\n",
@@ -943,7 +948,9 @@ mptctl_do_fw_download(int ioc, char __user *ufwbuf, size_t fwlen) ReplyMsg =NULL ;SET_MGMT_MSG_CONTEXT (iocp->ioctl_cmds.msg_context, dlmsg->MsgContext); INITIALIZE_MGMT_STATUS(iocp->ioctl_cmds.status) +mutex_lock (&mpctl_mutex);mpt_put_msg_frame (mptctl_id, iocp, mf); +mutex_lock (&mpctl_mutex);/* Now wait for the command to complete */ retry_wait:
@@ -1889,8 +1896,12 @@ mptctl_do_mpt_command (struct mpt_ioctl_command karg, void __user *mfPtr)/* Get a free request frame and save the message context. */ -if ((mf =mpt_get_msg_frame (mptctl_id, ioc)) ==NULL ) -return -EAGAIN ; +mutex_lock (&mpctl_mutex); + mf =mpt_get_msg_frame (mptctl_id, ioc); +mutex_unlock (&mpctl_mutex); +if (mf ==NULL ) { +return -EAGAIN ; + } hdr = (MPIHeader_t *) mf; msgContext =le32_to_cpu (hdr->MsgContext);
@@ -2271,11 +2282,14 @@ mptctl_do_mpt_command (struct mpt_ioctl_command karg, void __user *mfPtr)DBG_DUMP_TM_REQUEST_FRAME (ioc, (u32 *)mf);if ((ioc->facts.IOCCapabilities &MPI_IOCFACTS_CAPABILITY_HIGH_PRI_Q ) && - (ioc->facts.MsgVersion >=MPI_VERSION_01_05 )) + (ioc->facts.MsgVersion >=MPI_VERSION_01_05 )) { +mutex_lock (&mpctl_mutex);mpt_put_msg_frame_hi_pri (mptctl_id, ioc, mf); -else { - rc =mpt_send_handshake_request (mptctl_id, ioc, -sizeof (SCSITaskMgmt_t), (u32 *)mf,CAN_SLEEP ); +mutex_unlock (&mpctl_mutex); + }else { +mutex_lock (&mpctl_mutex); + rc =mpt_send_handshake_request (mptctl_id, ioc,sizeof (SCSITaskMgmt_t), (u32 *)mf,CAN_SLEEP ); +mutex_unlock (&mpctl_mutex);if (rc !=0 ) { dfailprintk(ioc, printk(MYIOC_s_ERR_FMT "send_handshake FAILED! (ioc %p, mf %p)\n",
@@ -2287,8 +2301,11 @@ mptctl_do_mpt_command (struct mpt_ioctl_command karg, void __user *mfPtr) } } - }else + }else { +mutex_lock (&mpctl_mutex);mpt_put_msg_frame (mptctl_id, ioc, mf); +mutex_unlock (&mpctl_mutex); + }/* Now wait for the command to complete */ timeout = (karg.timeout >0 ) ? karg.timeout :MPT_IOCTL_DEFAULT_TIMEOUT ;
@@ -2563,7 +2580,10 @@ mptctl_hp_hostinfo(unsigned long arg, unsigned int data_size)/* * Gather ISTWI(Industry Standard Two Wire Interface) Data */ -if ((mf =mpt_get_msg_frame (mptctl_id, ioc)) ==NULL ) { +mutex_lock (&mpctl_mutex); + mf =mpt_get_msg_frame (mptctl_id, ioc); +mutex_unlock (&mpctl_mutex); +if (mf ==NULL ) {dfailprintk (ioc,printk (MYIOC_s_WARN_FMT"%s, no msg frames!!\n" , ioc->name, __func__)); goto out;
@@ -2593,7 +2613,9 @@ mptctl_hp_hostinfo(unsigned long arg, unsigned int data_size)SET_MGMT_MSG_CONTEXT (ioc->ioctl_cmds.msg_context, IstwiRWRequest->MsgContext); INITIALIZE_MGMT_STATUS(ioc->ioctl_cmds.status) +mutex_lock (&mpctl_mutex);mpt_put_msg_frame (mptctl_id, ioc, mf); +mutex_unlock (&mpctl_mutex); retry_wait: timeleft = wait_for_completion_timeout(&ioc->ioctl_cmds.done,
@@ -2991,6 +3013,7 @@ static int __init mptctl_init(void) {int err;int where =1 ; +u8 m_id, m_taskmgmt_id;show_mptmod_ver (my_NAME, my_VERSION);
@@ -3010,25 +3033,26 @@ static int __init mptctl_init(void)* Install our handler */ ++where; - mptctl_id =mpt_register (mptctl_reply,MPTCTL_DRIVER , + m_id =mpt_register (mptctl_reply,MPTCTL_DRIVER ,"mptctl_reply" ); -if (!mptctl_id || mptctl_id >=MPT_MAX_PROTOCOL_DRIVERS ) { +if (!m_id || m_id >=MPT_MAX_PROTOCOL_DRIVERS ) { printk(KERN_ERR MYNAM ": ERROR: Failed to register with Fusion MPT base driver\n");misc_deregister (&mptctl_miscdev); err = -EBUSY ; goto out_fail; } + mptctl_id = m_id;// registration with Fusion MPT base successful - mptctl_taskmgmt_id =mpt_register (mptctl_taskmgmt_reply,MPTCTL_DRIVER , + m_taskmgmt_id =mpt_register (mptctl_taskmgmt_reply,MPTCTL_DRIVER ,"mptctl_taskmgmt_reply" ); -if (!mptctl_taskmgmt_id || mptctl_taskmgmt_id >=MPT_MAX_PROTOCOL_DRIVERS ) { +if (!m_taskmgmt_id || m_taskmgmt_id >=MPT_MAX_PROTOCOL_DRIVERS ) { printk(KERN_ERR MYNAM ": ERROR: Failed to register with Fusion MPT base driver\n");mpt_deregister (mptctl_id);misc_deregister (&mptctl_miscdev); err = -EBUSY ; goto out_fail; } - + mptctl_taskmgmt_id = m_taskmgmt_id;// registration with Fusion MPT base successful mpt_reset_register (mptctl_id, mptctl_ioc_reset);mpt_event_register (mptctl_id, mptctl_event_process);
@@ -3044,13 +3068,14 @@ out_fail:/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/ static void mptctl_exit (void ) { +mutex_lock (&mpctl_mutex);misc_deregister (&mptctl_miscdev); printk(KERN_INFO MYNAM ": Deregistered /dev/%s @ (major,minor=%d,%d)\n", mptctl_miscdev.name, MISC_MAJOR, mptctl_miscdev.minor);/* De-register event handler from base module */ mpt_event_deregister (mptctl_id); - +/* De-register reset handler from base module */ mpt_reset_deregister (mptctl_id);
@@ -3058,6 +3083,8 @@ static void mptctl_exit(void)mpt_deregister (mptctl_taskmgmt_id);mpt_deregister (mptctl_id); +mutex_unlock (&mpctl_mutex); +mpt_device_driver_deregister (MPTCTL_DRIVER ); }
Signature · 1 line
2.17.1