<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<HTML><HEAD>
<META content="text/html; charset=us-ascii" http-equiv=Content-Type>
<META name=GENERATOR content="MSHTML 8.00.6001.18876"></HEAD>
<BODY>
<DIV><SPAN class=805055607-08062010><FONT color=#0000ff size=2 face=Arial>Now
with the patch attached as a file</FONT></SPAN></DIV><BR>
<DIV dir=ltr lang=en-us class=OutlookMessageHeader align=left>
<HR tabIndex=-1>
<FONT size=2 face=Tahoma><B>From:</B> Alex Naslednikov <BR><B>Sent:</B> Tuesday,
June 08, 2010 10:56 AM<BR><B>To:</B> ofw@lists.openfabrics.org; Leonid Keller;
Tzachi Dar<BR><B>Cc:</B> Uri Habusha<BR><B>Subject:</B> [ofw][Patch][Core] Fix
at IRP cancellation mechanism<BR></FONT><BR></DIV>
<DIV></DIV>
<DIV><FONT size=2 face=Arial>This patch fixes the race (and BSOD) within IRP
cancellation mechanism.<BR>The original problem:<BR>1.Function
al_dev_cancel_ioctl() called from the 2 places:<BR>directly from IBAL and by OS
(as a callback)<BR>2. This is because I/O request can be cancelled both by
user-level mechanism and <BR>by OS.<BR>3. Function al_dev_cancel_ioctl() calls
to IoGetCurrentIrpStackLocation() in order <BR>to receive context and take a
spinlock (p_context->cb_lock)<BR>4. But IoGetCurrentIrpStackLocation() as
well further access to stack pointer<BR>should be protected !!! <BR>5. BSOD
happened when context or IRP itself were already cancelled by concurrent
<BR>calls to al_dev_cancel_ioctl()</FONT></DIV>
<DIV> </DIV>
<DIV><FONT size=2 face=Arial>Signed-off by: Alexander Naslednikov (xalex<SPAN
class=805414207-08062010> at mellanox.co.il)</SPAN><BR>Index:
B:/users/xalex/2_1_0/core/al/kernel/al_dev.c<BR>===================================================================<BR>---
B:/users/xalex/2_1_0/core/al/kernel/al_dev.c (revision 5929)<BR>+++
B:/users/xalex/2_1_0/core/al/kernel/al_dev.c (revision 5930)<BR>@@ -65,9
+65,16
@@<BR> __proxy_cancel_cblists(<BR> IN al_dev_open_context_t *p_context
);<BR> <BR>+CL_INLINE
void<BR>+al_dev_cancel_ioctl_unlocked(<BR>+ IN al_dev_open_context_t *p_context,<BR>+ IN
OUT cl_ioctl_handle_t *p_ioctl
);<BR> static
void<BR> __construct_open_context(<BR> IN al_dev_open_context_t *p_context
)<BR>@@ -343,12 +350,19 @@<BR> p_context->closing =
TRUE;<BR> <BR> /* Flush any pending IOCTLs in case user-mode
threads died on us. */<BR>+ /* Protect IOCTL cancellation by spinlock to
avoid race <BR>+ */<BR>+ <BR>+ cl_spinlock_acquire(
&p_context->cb_lock );<BR>+ <BR> if(
p_context->h_cm_ioctl )<BR>- al_dev_cancel_ioctl(
p_context->h_cm_ioctl );<BR>- if( p_context->h_comp_ioctl
)<BR>- al_dev_cancel_ioctl( p_context->h_comp_ioctl
);<BR>+ al_dev_cancel_ioctl_unlocked( p_context,
&p_context->h_cm_ioctl );<BR>+ if( p_context->h_comp_ioctl )
<BR>+ al_dev_cancel_ioctl_unlocked( p_context,
&p_context->h_comp_ioctl );<BR> if( p_context->h_misc_ioctl
)<BR>- al_dev_cancel_ioctl( p_context->h_misc_ioctl
);<BR>+ al_dev_cancel_ioctl_unlocked( p_context,
&p_context->h_misc_ioctl );<BR>+ <BR>+ cl_spinlock_release (
&p_context->cb_lock );<BR> <BR> while(
p_context->ref_cnt )<BR> {<BR>@@ -514,20 +528,31
@@<BR> <BR> /* Get the stack location.
*/<BR> p_io_stack = IoGetCurrentIrpStackLocation( h_ioctl
);<BR>+ if (!p_io_stack) {<BR>+ AL_PRINT( TRACE_LEVEL_WARNING,
AL_DBG_DEV, ("This IOCTL was previosly destroyed \n")
);<BR>+ return;<BR>+ }<BR> <BR> p_context =
(al_dev_open_context_t
*)p_io_stack->FileObject->FsContext;<BR>- ASSERT( p_context
);<BR>-<BR>+ if (!p_context) {<BR>+ AL_PRINT( TRACE_LEVEL_ERROR,
AL_DBG_ERROR, ("This IOCTL was previosly destroyed \n")
);<BR>+ return;<BR>+ }<BR> /* Clear the IOCTL.
*/<BR> cl_spinlock_acquire( &p_context->cb_lock
);<BR> switch( cl_ioctl_ctl_code( h_ioctl )
)<BR> {<BR> case
UAL_GET_COMP_CB_INFO:<BR> ph_ioctl =
&p_context->h_comp_ioctl;<BR>+ ASSERT(ph_ioctl);<BR> break;<BR> case
UAL_GET_MISC_CB_INFO:<BR> ph_ioctl =
&p_context->h_misc_ioctl;<BR> break;<BR>+ case
NULL:<BR>+ AL_PRINT( TRACE_LEVEL_WARNING, AL_DBG_DEV, ("This IOCTL
was previosly destroyed \n") );<BR>+ cl_spinlock_release(
&p_context->cb_lock
);<BR>+ return;<BR> default:<BR> AL_PRINT(
TRACE_LEVEL_ERROR, AL_DBG_ERROR, ("Invalid CB type\n")
);<BR> ph_ioctl = NULL;<BR>@@ -550,7 +575,33
@@<BR> AL_EXIT( AL_DBG_DEV );<BR> }<BR> <BR>+/*<BR>+ *
Cancel any pending IOCTL calls for the specified type.<BR>+ * This routine is
also called when closing the device.<BR>+ * The lock should already be taken by
the caller<BR>+ */<BR>+CL_INLINE
void<BR>+al_dev_cancel_ioctl_unlocked(<BR>+ IN al_dev_open_context_t *p_context,<BR>+ IN
OUT cl_ioctl_handle_t *p_ioctl
)<BR>+{<BR>+ <BR>+ AL_ENTER( AL_DBG_DEV );<BR> <BR>+#pragma
warning(push, 3)<BR>+ IoSetCancelRoutine( *p_ioctl, NULL );<BR>+#pragma
warning(pop)<BR>+<BR>+ /* Complete the IOCTL.
*/<BR>+ cl_ioctl_complete( *p_ioctl, CL_CANCELED, 0
);<BR>+ proxy_context_deref( p_context );<BR>+ *p_ioctl =
NULL;<BR>+<BR>+ AL_EXIT( AL_DBG_DEV
);<BR>+}<BR>+<BR>+<BR>+<BR> void<BR> al_dev_cancel_io(<BR> IN DEVICE_OBJECT *p_dev_obj,<BR>Index:
B:/users/xalex/2_1_0/inc/kernel/complib/cl_ioctl_osd.h<BR>===================================================================<BR>---
B:/users/xalex/2_1_0/inc/kernel/complib/cl_ioctl_osd.h (revision
5929)<BR>+++
B:/users/xalex/2_1_0/inc/kernel/complib/cl_ioctl_osd.h (revision
5930)<BR>@@ -49,7 +49,9 @@<BR> #define IOCTL_CODE( type, cmd
) \<BR> CTL_CODE( type, (cmd & 0x0FFF), METHOD_BUFFERED,
FILE_ANY_ACCESS)<BR> <BR>+#define BAD_IRP_STACK
0<BR> <BR>+<BR> typedef
PIRP cl_ioctl_handle_t;<BR> <BR> <BR>@@ -101,6 +103,10
@@<BR> IO_STACK_LOCATION *p_io_stack;<BR> <BR> p_io_stack
= IoGetCurrentIrpStackLocation( h_ioctl );<BR>+ if (!p_io_stack)
{<BR>+ /* "This IOCTL was previosly destroyed !
*/<BR>+ return BAD_IRP_STACK ;<BR>+ }<BR> return
p_io_stack->Parameters.DeviceIoControl.IoControlCode;<BR> }<BR> <BR></FONT></DIV></BODY></HTML>