Windows: fix deadlock in symlink Attrib retrieval

Commit df22620f66f5ce92776177d4d800fc7f4ae4ae99 introduced a
deadlock when retrieving file attributes for symlink objects.
To correct the deadlock, do not hold the Fcb locks during the
AFSRetrieveFileAttributes() call.

Change-Id: I8d3d2c5f54d4e5c97f690fd1fd6db62128317cf2
Reviewed-on: http://gerrit.openafs.org/5792
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>
Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp b/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp
index 9659505..390ad80 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp
@@ -746,6 +746,7 @@ AFSQueryBasicInfo( IN PIRP Irp,
     NTSTATUS ntStatus = STATUS_SUCCESS;
     AFSDeviceExt *pDeviceExt = (AFSDeviceExt *)AFSRDRDeviceObject->DeviceExtension;
     ULONG ulFileAttribs = 0;
+    AFSFcb *pFcb = NULL;
     AFSCcb *pCcb = NULL;
     IO_STACK_LOCATION *pIrpSp = IoGetCurrentIrpStackLocation( Irp);
     AFSFileInfoCB stFileInfo;
@@ -760,11 +761,12 @@ AFSQueryBasicInfo( IN PIRP Irp,
 
         ulFileAttribs = DirectoryCB->ObjectInformation->FileAttributes;
 
+        pFcb = (AFSFcb *)pIrpSp->FileObject->FsContext;
+        pCcb = (AFSCcb *)pIrpSp->FileObject->FsContext2;
+
         if( DirectoryCB->ObjectInformation->FileType == AFS_FILE_TYPE_SYMLINK)
         {
 
-            pCcb = (AFSCcb *)pIrpSp->FileObject->FsContext2;
-
             pParentDirectoryCB = AFSGetParentEntry( pCcb->NameArray);
 
             AFSRetrieveParentPath( &pCcb->FullFileName,
@@ -773,6 +775,12 @@ AFSQueryBasicInfo( IN PIRP Irp,
             RtlZeroMemory( &stFileInfo,
                            sizeof( AFSFileInfoCB));
 
+            //
+            // Can't hold the Fcb while evaluating the path, leads to lock inversion
+            //
+
+            AFSReleaseResource( &pFcb->NPFcb->Resource);
+
             if( NT_SUCCESS( AFSRetrieveFileAttributes( pParentDirectoryCB,
                                                        DirectoryCB,
                                                        &uniParentPath,
@@ -783,6 +791,9 @@ AFSQueryBasicInfo( IN PIRP Irp,
 
                 ulFileAttribs |= FILE_ATTRIBUTE_REPARSE_POINT;
             }
+
+            AFSAcquireShared( &pFcb->NPFcb->Resource,
+                              TRUE);
         }
 
         Buffer->CreationTime = DirectoryCB->ObjectInformation->CreationTime;
@@ -824,6 +835,7 @@ AFSQueryStandardInfo( IN PIRP Irp,
 {
 
     NTSTATUS ntStatus = STATUS_SUCCESS;
+    AFSFcb *pFcb = NULL;
     AFSCcb *pCcb = NULL;
     PIO_STACK_LOCATION pIrpSp = IoGetCurrentIrpStackLocation( Irp);
     AFSFileInfoCB stFileInfo;
@@ -834,6 +846,7 @@ AFSQueryStandardInfo( IN PIRP Irp,
     if( *Length >= sizeof( FILE_STANDARD_INFORMATION))
     {
 
+        pFcb = (AFSFcb *)pIrpSp->FileObject->FsContext;
         pCcb = (AFSCcb *)pIrpSp->FileObject->FsContext2;
 
         RtlZeroMemory( Buffer,
@@ -851,8 +864,6 @@ AFSQueryStandardInfo( IN PIRP Irp,
         if( DirectoryCB->ObjectInformation->FileType == AFS_FILE_TYPE_SYMLINK)
         {
 
-            pCcb = (AFSCcb *)pIrpSp->FileObject->FsContext2;
-
             pParentDirectoryCB = AFSGetParentEntry( pCcb->NameArray);
 
             AFSRetrieveParentPath( &pCcb->FullFileName,
@@ -861,6 +872,12 @@ AFSQueryStandardInfo( IN PIRP Irp,
             RtlZeroMemory( &stFileInfo,
                            sizeof( AFSFileInfoCB));
 
+            //
+            // Can't hold the Fcb while evaluating the path, leads to lock inversion
+            //
+
+            AFSReleaseResource( &pFcb->NPFcb->Resource);
+
             if( NT_SUCCESS( AFSRetrieveFileAttributes( pParentDirectoryCB,
                                                        DirectoryCB,
                                                        &uniParentPath,
@@ -869,6 +886,9 @@ AFSQueryStandardInfo( IN PIRP Irp,
             {
                 ulFileAttribs = stFileInfo.FileAttributes;
             }
+
+            AFSAcquireShared( &pFcb->NPFcb->Resource,
+                              TRUE);
         }
 
         Buffer->Directory = BooleanFlagOn( ulFileAttribs, FILE_ATTRIBUTE_DIRECTORY);
@@ -1296,6 +1316,7 @@ AFSQueryNetworkInfo( IN PIRP Irp,
 
     NTSTATUS ntStatus = STATUS_SUCCESS;
     AFSDeviceExt *pDeviceExt = (AFSDeviceExt *)AFSRDRDeviceObject->DeviceExtension;
+    AFSFcb *pFcb = NULL;
     AFSCcb *pCcb = NULL;
     PIO_STACK_LOCATION pIrpSp = IoGetCurrentIrpStackLocation( Irp);
     AFSFileInfoCB stFileInfo;
@@ -1311,11 +1332,12 @@ AFSQueryNetworkInfo( IN PIRP Irp,
 
         ulFileAttribs = DirectoryCB->ObjectInformation->FileAttributes;
 
+        pFcb = (AFSFcb *)pIrpSp->FileObject->FsContext;
+        pCcb = (AFSCcb *)pIrpSp->FileObject->FsContext2;
+
         if( DirectoryCB->ObjectInformation->FileType == AFS_FILE_TYPE_SYMLINK)
         {
 
-            pCcb = (AFSCcb *)pIrpSp->FileObject->FsContext2;
-
             pParentDirectoryCB = AFSGetParentEntry( pCcb->NameArray);
 
             AFSRetrieveParentPath( &pCcb->FullFileName,
@@ -1324,6 +1346,12 @@ AFSQueryNetworkInfo( IN PIRP Irp,
             RtlZeroMemory( &stFileInfo,
                            sizeof( AFSFileInfoCB));
 
+            //
+            // Can't hold the Fcb while evaluating the path, leads to lock inversion
+            //
+
+            AFSReleaseResource( &pFcb->NPFcb->Resource);
+
             if( NT_SUCCESS( AFSRetrieveFileAttributes( pParentDirectoryCB,
                                                        DirectoryCB,
                                                        &uniParentPath,
@@ -1334,6 +1362,9 @@ AFSQueryNetworkInfo( IN PIRP Irp,
 
                 ulFileAttribs |= FILE_ATTRIBUTE_REPARSE_POINT;
             }
+
+            AFSAcquireShared( &pFcb->NPFcb->Resource,
+                              TRUE);
         }
 
         Buffer->CreationTime.QuadPart = DirectoryCB->ObjectInformation->CreationTime.QuadPart;
@@ -1453,6 +1484,7 @@ AFSQueryAttribTagInfo( IN PIRP Irp,
     NTSTATUS ntStatus = STATUS_BUFFER_TOO_SMALL;
     ULONG ulCopyLength = 0;
     AFSDeviceExt *pDeviceExt = (AFSDeviceExt *)AFSRDRDeviceObject->DeviceExtension;
+    AFSFcb *pFcb = NULL;
     AFSCcb *pCcb = NULL;
     PIO_STACK_LOCATION pIrpSp = IoGetCurrentIrpStackLocation( Irp);
     AFSFileInfoCB stFileInfo;
@@ -1468,11 +1500,12 @@ AFSQueryAttribTagInfo( IN PIRP Irp,
 
         ulFileAttribs = DirectoryCB->ObjectInformation->FileAttributes;
 
+        pFcb = (AFSFcb *)pIrpSp->FileObject->FsContext;
+        pCcb = (AFSCcb *)pIrpSp->FileObject->FsContext2;
+
         if( DirectoryCB->ObjectInformation->FileType == AFS_FILE_TYPE_SYMLINK)
         {
 
-            pCcb = (AFSCcb *)pIrpSp->FileObject->FsContext2;
-
             pParentDirectoryCB = AFSGetParentEntry( pCcb->NameArray);
 
             AFSRetrieveParentPath( &pCcb->FullFileName,
@@ -1481,6 +1514,12 @@ AFSQueryAttribTagInfo( IN PIRP Irp,
             RtlZeroMemory( &stFileInfo,
                            sizeof( AFSFileInfoCB));
 
+            //
+            // Can't hold the Fcb while evaluating the path, leads to lock inversion
+            //
+
+            AFSReleaseResource( &pFcb->NPFcb->Resource);
+
             if( NT_SUCCESS( AFSRetrieveFileAttributes( pParentDirectoryCB,
                                                        DirectoryCB,
                                                        &uniParentPath,
@@ -1491,6 +1530,9 @@ AFSQueryAttribTagInfo( IN PIRP Irp,
 
                 ulFileAttribs |= FILE_ATTRIBUTE_REPARSE_POINT;
             }
+
+            AFSAcquireShared( &pFcb->NPFcb->Resource,
+                              TRUE);
         }
 
         Buffer->FileAttributes = ulFileAttribs;