<!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>