Windows: Use AuthGroups for extent request error reporting
authorJeffrey Altman <jaltman@your-file-system.com>
Thu, 22 Dec 2011 02:17:33 +0000 (21:17 -0500)
committerJeffrey Altman <jaltman@secure-endpoints.com>
Thu, 22 Dec 2011 15:10:32 +0000 (07:10 -0800)
The afs redirector current tracks the most recent extent error
in the File Control Block.  Prior to this patchset the error
was returned to the requesting thread when the process Id matched
the most recent Process to issue a request.  This approach resulted
in a couple of problems.

 1. There are multiple threads that can issue an extent request
    on the same file at the same time representing different processes.
    Resetting the process Id with each new request could clear the
    error prior to its receipt.

 2. The failure may be due to inappropriate permissions.  Permissions
    are not associated with proceses but with Authentication Groups.

This patchset makes several changes:

 1. It enables the afsd_service to track the active authgroup as
    part of the cm_user_t structure and associates that object with
    the BIOD object to ensure that the active authgroup can be
    reported to the afs redirector.

 2. It modifies the AFSExtentFailureCB structure to include the
    AuthGroup GUID.

 3. It tracks the AuthGroup GUID associated with the extent
    failure in the non-paged file control block.

 4. It converts all tests on Process Id to use AuthGroup instead.

 5. It alters the behavior of error delivery such that reported
    error is only cleared after it has been reported once to a
    thread using the matching AuthGroup.

These changes make the situation better but not perfect as error
states can still be lost.  However, it avoids the case most often
seen in production where two processes (a end user process and an
anti-malware process) are fighting over a file and the anti-malware
process has no permission to access the file under its own credentials.

Change-Id: Ia5c3877b8d46de695c86884c4166dc812885a72c
Reviewed-on: http://gerrit.openafs.org/6396
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Peter Scott <pscott@kerneldrivers.com>
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>
Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>

12 files changed:
src/WINNT/afsd/cm_dcache.c
src/WINNT/afsd/cm_dcache.h
src/WINNT/afsd/cm_user.h
src/WINNT/afsrdr/common/AFSRedirCommonStructs.h
src/WINNT/afsrdr/common/AFSUserPrototypes.h
src/WINNT/afsrdr/common/AFSUserStructs.h
src/WINNT/afsrdr/kernel/lib/AFSExtentsSupport.cpp
src/WINNT/afsrdr/kernel/lib/AFSRead.cpp
src/WINNT/afsrdr/kernel/lib/AFSWrite.cpp
src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h
src/WINNT/afsrdr/user/RDRFunction.c
src/WINNT/afsrdr/user/RDRInit.cpp

index 8b14ba4..36e55bd 100644 (file)
@@ -1008,6 +1008,7 @@ long cm_SetupStoreBIOD(cm_scache_t *scp, osi_hyper_t *inOffsetp, long inSize,
 
     /* clear things out */
     biop->scp = scp;                   /* do not hold; held by caller */
+    biop->userp = userp;                /* do not hold; held by caller */
     biop->offset = *inOffsetp;
     biop->length = 0;
     biop->bufListp = NULL;
@@ -1247,6 +1248,7 @@ long cm_SetupFetchBIOD(cm_scache_t *scp, osi_hyper_t *offsetp,
     tblocksize = ConvertLongToLargeInteger(cm_data.buf_blockSize);
 
     biop->scp = scp;                   /* do not hold; held by caller */
+    biop->userp = userp;                /* do not hold; held by caller */
     biop->reqp = reqp;
     biop->offset = *offsetp;
     /* null out the list of buffers */
@@ -1546,7 +1548,7 @@ void cm_ReleaseBIOD(cm_bulkIO_t *biop, int isStore, long code, int scp_locked)
         if (RDR_Initialized && reportErrorToRedir) {
             DWORD status;
             smb_MapNTError(cm_MapRPCError(code, biop->reqp), &status, TRUE);
-            RDR_SetFileStatus( &scp->fid, status);
+            RDR_SetFileStatus( &scp->fid, &biop->userp->authgroup, status);
         }
     } else {
        if (!scp_locked)
index be5b723..dbe7539 100644 (file)
@@ -13,6 +13,7 @@
 /* bulk I/O descriptor */
 typedef struct cm_bulkIO {
     struct cm_scache *scp;             /* typically unheld vnode ptr */
+    struct cm_user *userp;              /* the user of the request */
     struct cm_req *reqp;                /* the request ptr */
     osi_hyper_t offset;                        /* offset of buffers */
     long length;                       /* # of bytes to be transferred */
index a980474..ed98032 100644 (file)
@@ -49,6 +49,7 @@ typedef struct cm_user {
     osi_mutex_t mx;                    /* mutex */
     int vcRefs;                                /* count of references from virtual circuits */
     long flags;
+    GUID authgroup;                     /* AFS redirector */
 } cm_user_t;
 
 #define CM_USERFLAG_DELETE     1       /* delete on last reference */
index a8a9a22..f813a58 100644 (file)
@@ -196,6 +196,8 @@ typedef struct _AFS_NONPAGED_FCB
 
             NTSTATUS        ExtentsRequestStatus;
 
+            GUID            ExtentsRequestAuthGroup;
+
             struct _AFS_FSD_EXTENT  *DirtyListHead;
 
             struct _AFS_FSD_EXTENT  *DirtyListTail;
index 9e29eaa..3d44e9b 100644 (file)
@@ -50,7 +50,7 @@ extern DWORD RDR_NetworkAddrChange(void);
 
 extern DWORD RDR_InvalidateVolume( IN ULONG cellID, IN ULONG volID, IN ULONG reason);
 
-extern DWORD RDR_SetFileStatus( IN cm_fid_t *pFileId, IN DWORD dwStatus);
+extern DWORD RDR_SetFileStatus( IN cm_fid_t *pFileId, IN GUID *pAuthGroup, IN DWORD dwStatus);
 
 extern DWORD
 RDR_InvalidateObject( IN ULONG cellID, IN ULONG volID, IN ULONG vnode,
index 34496fd..24ebd08 100644 (file)
@@ -480,6 +480,8 @@ typedef struct _AFS_EXTENT_FAILURE_CB
 
     ULONG           FailureStatus;
 
+    GUID            AuthGroup;      // Length: sizeof(GUID) */
+
 } AFSExtentFailureCB;
 
 //
index 86d596c..9d702b5 100644 (file)
@@ -125,7 +125,10 @@ AFSTearDownFcbExtents( IN AFSFcb *Fcb,
     __Enter
     {
 
-        if( pAuthGroup == NULL)
+        if( pAuthGroup == NULL ||
+            RtlCompareMemory( pAuthGroup,
+                              &Fcb->NPFcb->Specific.File.ExtentsRequestAuthGroup,
+                              sizeof( GUID)) == sizeof( GUID))
         {
 
             RtlZeroMemory( &stAuthGroup,
@@ -709,15 +712,24 @@ AFSRequestExtentsAsync( IN AFSFcb *Fcb,
         {
 
             //
-            // If this isn't the same process which caused the failure then try to request them again
+            // If this isn't the same authgroup which caused the failure
+            // then try to request them again
             //
 
-            if( Fcb->Specific.File.ExtentRequestProcessId == ullProcessId)
+            if( RtlCompareMemory( &pNPFcb->Specific.File.ExtentsRequestAuthGroup,
+                                  &Ccb->AuthGroup,
+                                  sizeof( GUID)) == sizeof( GUID))
             {
-                try_return( ntStatus = pNPFcb->Specific.File.ExtentsRequestStatus);
-            }
 
-            pNPFcb->Specific.File.ExtentsRequestStatus = STATUS_SUCCESS;
+                ntStatus = pNPFcb->Specific.File.ExtentsRequestStatus;
+
+                pNPFcb->Specific.File.ExtentsRequestStatus = STATUS_SUCCESS;
+
+                RtlZeroMemory( &pNPFcb->Specific.File.ExtentsRequestAuthGroup,
+                               sizeof( GUID));
+
+                try_return( ntStatus);
+            }
         }
 
         //
@@ -1982,6 +1994,10 @@ AFSProcessExtentFailure( PIRP Irp)
 
         pObjectInfo->Fcb->NPFcb->Specific.File.ExtentsRequestStatus = pFailureCB->FailureStatus;
 
+        RtlCopyMemory( &pObjectInfo->Fcb->NPFcb->Specific.File.ExtentsRequestAuthGroup,
+                       &pFailureCB->AuthGroup,
+                       sizeof( GUID));
+
         KeSetEvent( &pObjectInfo->Fcb->NPFcb->Specific.File.ExtentsRequestComplete,
                     0,
                     FALSE);
@@ -2409,7 +2425,8 @@ try_exit:
 }
 
 NTSTATUS
-AFSWaitForExtentMapping( AFSFcb *Fcb )
+AFSWaitForExtentMapping( AFSFcb *Fcb,
+                         AFSCcb *Ccb)
 {
     NTSTATUS ntStatus = STATUS_SUCCESS;
     LARGE_INTEGER liTimeOut;
@@ -2424,18 +2441,27 @@ AFSWaitForExtentMapping( AFSFcb *Fcb )
         {
 
             //
-            // If this isn't the same process which caused the failure then try to request them again
+            // If this isn't the same authgroup which caused the failure
+            // then try to request them again
             //
 
-            if( Fcb->Specific.File.ExtentRequestProcessId == ullProcessId)
+            if( RtlCompareMemory( &Fcb->NPFcb->Specific.File.ExtentsRequestAuthGroup,
+                                  &Ccb->AuthGroup,
+                                  sizeof( GUID)) == sizeof( GUID))
             {
-                try_return( ntStatus = Fcb->NPFcb->Specific.File.ExtentsRequestStatus);
-            }
 
-            Fcb->NPFcb->Specific.File.ExtentsRequestStatus = STATUS_SUCCESS;
+                ntStatus = Fcb->NPFcb->Specific.File.ExtentsRequestStatus;
+
+                Fcb->NPFcb->Specific.File.ExtentsRequestStatus = STATUS_SUCCESS;
+
+                RtlZeroMemory( &Fcb->NPFcb->Specific.File.ExtentsRequestAuthGroup,
+                               sizeof( GUID));
+
+                try_return( ntStatus);
+            }
         }
 
-        liTimeOut.QuadPart = -(50000000);
+        liTimeOut.QuadPart = -(1 * AFS_ONE_SECOND);
 
         ntStatus = KeWaitForSingleObject( &Fcb->NPFcb->Specific.File.ExtentsRequestComplete,
                                           Executive,
@@ -2447,17 +2473,26 @@ AFSWaitForExtentMapping( AFSFcb *Fcb )
         {
 
             //
-            // If this isn't the same process which caused the failure
-            // and this isn't the System process, then try to request them again
+            // If this isn't the same authgroup which caused the failure
+            // or the System Process,
+            // then try to request the extents again
             //
 
-            if( Fcb->Specific.File.ExtentRequestProcessId == ullProcessId ||
-                ullProcessId == 0x4)
+            if( RtlCompareMemory( &Fcb->NPFcb->Specific.File.ExtentsRequestAuthGroup,
+                                  &Ccb->AuthGroup,
+                                  sizeof( GUID)) == sizeof( GUID) ||
+                ullProcessId == (ULONGLONG)AFSSysProcess)
             {
-                try_return( ntStatus = Fcb->NPFcb->Specific.File.ExtentsRequestStatus);
-            }
 
-            Fcb->NPFcb->Specific.File.ExtentsRequestStatus = STATUS_SUCCESS;
+                ntStatus = Fcb->NPFcb->Specific.File.ExtentsRequestStatus;
+
+                Fcb->NPFcb->Specific.File.ExtentsRequestStatus = STATUS_SUCCESS;
+
+                RtlZeroMemory( &Fcb->NPFcb->Specific.File.ExtentsRequestAuthGroup,
+                               sizeof( GUID));
+
+                try_return( ntStatus);
+            }
         }
 
         if( ntStatus == STATUS_TIMEOUT)
@@ -2507,7 +2542,10 @@ AFSFlushExtents( IN AFSFcb *Fcb,
     __Enter
     {
 
-        if( pAuthGroup == NULL)
+        if( pAuthGroup == NULL ||
+            RtlCompareMemory( pAuthGroup,
+                              &Fcb->NPFcb->Specific.File.ExtentsRequestAuthGroup,
+                              sizeof( GUID)) == sizeof( GUID))
         {
 
             RtlZeroMemory( &stAuthGroup,
@@ -2823,7 +2861,10 @@ AFSReleaseExtentsWithFlush( IN AFSFcb *Fcb,
     __Enter
     {
 
-        if( pAuthGroup == NULL)
+        if( pAuthGroup == NULL ||
+            RtlCompareMemory( pAuthGroup,
+                              &Fcb->NPFcb->Specific.File.ExtentsRequestAuthGroup,
+                              sizeof( GUID)) == sizeof( GUID))
         {
 
             RtlZeroMemory( &stAuthGroup,
index 220fdfb..6810d4d 100644 (file)
@@ -388,7 +388,7 @@ AFSNonCachedRead( IN PDEVICE_OBJECT DeviceObject,
                           StartingByte.QuadPart,
                           ulReadByteCount);
 
-            ntStatus =  AFSWaitForExtentMapping ( pFcb );
+            ntStatus =  AFSWaitForExtentMapping ( pFcb, pCcb);
 
             if (!NT_SUCCESS(ntStatus))
             {
@@ -1142,8 +1142,6 @@ AFSCommonRead( IN PDEVICE_OBJECT DeviceObject,
             }
 
             pFcb->Specific.File.ExtentThreadId = (ULONGLONG)PsGetCurrentThreadId();
-
-            pFcb->NPFcb->Specific.File.ExtentsRequestStatus = STATUS_SUCCESS;
         }
 #endif
 
index 559549c..84b0063 100644 (file)
@@ -117,6 +117,7 @@ AFSCommonWrite( IN PDEVICE_OBJECT DeviceObject,
     HANDLE             hCallingUser = OnBehalfOf;
     ULONG              ulExtensionLength = 0;
     BOOLEAN            bRetry = FALSE;
+    ULONGLONG          ullProcessId = (ULONGLONG)PsGetCurrentProcessId();
 
     pIrpSp = IoGetCurrentIrpStackLocation( Irp);
 
@@ -397,13 +398,13 @@ AFSCommonWrite( IN PDEVICE_OBJECT DeviceObject,
 
         if( !bPagingIo &&
             ( pFcb->Specific.File.ExtentRequestProcessId == 0 ||
-              ( PsGetCurrentProcessId() != AFSSysProcess &&
-                pFcb->Specific.File.ExtentRequestProcessId != (ULONGLONG)PsGetCurrentProcessId())))
+              ( ullProcessId != (ULONGLONG)AFSSysProcess &&
+                pFcb->Specific.File.ExtentRequestProcessId != ullProcessId)))
         {
 
-            pFcb->Specific.File.ExtentRequestProcessId = (ULONGLONG)PsGetCurrentProcessId();
+            pFcb->Specific.File.ExtentRequestProcessId = ullProcessId;
 
-            if( pFcb->Specific.File.ExtentRequestProcessId == (ULONGLONG)AFSSysProcess)
+            if( ullProcessId == (ULONGLONG)AFSSysProcess)
             {
                 AFSDbgLogMsg( AFS_SUBSYSTEM_EXTENT_PROCESSING,
                               AFS_TRACE_LEVEL_WARNING,
@@ -411,8 +412,6 @@ AFSCommonWrite( IN PDEVICE_OBJECT DeviceObject,
                               __FUNCTION__,
                               pFcb);
             }
-
-            pFcb->NPFcb->Specific.File.ExtentsRequestStatus = STATUS_SUCCESS;
         }
 
         //
@@ -941,7 +940,7 @@ AFSNonCachedWrite( IN PDEVICE_OBJECT DeviceObject,
             bLocked= FALSE;
 
             //
-            // We will re-request the extents after 10 seconds of waiting for them
+            // We will re-request the extents after waiting for them
             //
 
             KeQueryTickCount( &liCurrentTime);
@@ -994,7 +993,7 @@ AFSNonCachedWrite( IN PDEVICE_OBJECT DeviceObject,
             // Wait for it
             //
 
-            ntStatus =  AFSWaitForExtentMapping ( pFcb );
+            ntStatus =  AFSWaitForExtentMapping ( pFcb, pCcb);
 
             if (!NT_SUCCESS(ntStatus))
             {
index e2e4717..380c983 100644 (file)
@@ -366,7 +366,8 @@ AFSRequestExtentsAsync( IN AFSFcb *Fcb,
                         IN ULONG Size);
 
 NTSTATUS
-AFSWaitForExtentMapping ( IN AFSFcb *Fcb );
+AFSWaitForExtentMapping ( IN AFSFcb *Fcb,
+                          IN AFSCcb *Ccb);
 
 NTSTATUS
 AFSProcessSetFileExtents( IN AFSSetFileExtentsCB *SetExtents );
index 4a57409..709a611 100644 (file)
@@ -231,8 +231,10 @@ RDR_UserFromAuthGroup( IN GUID *pGuid)
 
     unp = smb_FindUserByName(UuidString, cname, SMB_FLAG_CREATE);
     lock_ObtainMutex(&unp->mx);
-    if (!unp->userp)
+    if (!unp->userp) {
         unp->userp = cm_NewUser();
+        memcpy(&unp->userp->authgroup, pGuid, sizeof(GUID));
+    }
     unp->flags |= SMB_USERNAMEFLAG_SID;
     lock_ReleaseMutex(&unp->mx);
     userp = unp->userp;
@@ -2899,7 +2901,7 @@ RDR_BkgFetch(cm_scache_t *scp, afs_uint32 p1, afs_uint32 p2, afs_uint32 p3, afs_
     FileId.Hash = scp->fid.hash;
 
     if ((GetTickCount() - reqp->startTime) / 1000 > HardDeadtimeout * 5) {
-        RDR_SetFileStatus( &scp->fid, STATUS_IO_TIMEOUT);
+        RDR_SetFileStatus( &scp->fid, &userp->authgroup, STATUS_IO_TIMEOUT);
         return 0;
     }
 
@@ -2929,7 +2931,7 @@ RDR_BkgFetch(cm_scache_t *scp, afs_uint32 p1, afs_uint32 p2, afs_uint32 p3, afs_
         osi_Log2(afsd_logp, "RDR_BkgFetch cm_SyncOp failure scp=0x%p code=0x%x",
                  scp, code);
         smb_MapNTError(cm_MapRPCError(code, reqp), &status, TRUE);
-        RDR_SetFileStatus( &scp->fid, status);
+        RDR_SetFileStatus( &scp->fid, &userp->authgroup, status);
         return code;
     }
     lock_ReleaseWrite(&scp->rw);
@@ -3088,7 +3090,7 @@ RDR_BkgFetch(cm_scache_t *scp, afs_uint32 p1, afs_uint32 p2, afs_uint32 p3, afs_
 
     if (reportErrorToRedir) {
         smb_MapNTError(cm_MapRPCError(code, reqp), &status, TRUE);
-        RDR_SetFileStatus( &scp->fid, status);
+        RDR_SetFileStatus( &scp->fid, &userp->authgroup, status);
     }
 
     osi_Log4(afsd_logp, "Ending BKG Fetch scp 0x%p code 0x%x fetched 0x%x:%x",
@@ -3172,7 +3174,7 @@ RDR_RequestFileExtentsAsync( IN cm_user_t *userp,
         osi_Log2(afsd_logp, "RDR_RequestFileExtentsAsync cm_SyncOp failure scp=0x%p code=0x%x",
                  scp, code);
         smb_MapNTError(cm_MapRPCError(code, &req), &status, TRUE);
-        RDR_SetFileStatus( &scp->fid, status);
+        RDR_SetFileStatus( &scp->fid, &userp->authgroup, status);
         return FALSE;
     }
 
@@ -3210,7 +3212,7 @@ RDR_RequestFileExtentsAsync( IN cm_user_t *userp,
                     break;
                 default:
                     smb_MapNTError(cm_MapRPCError(code, &req), &status, TRUE);
-                    RDR_SetFileStatus(&FileId, status);
+                    RDR_SetFileStatus(&FileId, &userp->authgroup, status);
                     bHaveBuffer = FALSE;
                     code = 0;
                 }
index 371f465..1441ef4 100644 (file)
@@ -68,6 +68,11 @@ extern osi_log_t *afsd_logp;
 }
 #include <RDRPrototypes.h>
 
+static DWORD
+RDR_SetFileStatus2( AFSFileID * pFileId,
+                    GUID *pAuthGroup,
+                    DWORD dwStatus);
+
 #ifndef FlagOn
 #define FlagOn(_F,_SF)        ((_F) & (_SF))
 #endif
@@ -1379,7 +1384,7 @@ RDR_ProcessRequest( AFSCommRequest *RequestBuffer)
               osi_Log1(afsd_logp, "%S", osi_LogSaveStringW(afsd_logp, wchBuffer));
           }
 
-          RDR_SetFileStatus( (cm_fid_t *)&RequestBuffer->FileId, STATUS_NO_MEMORY);
+          RDR_SetFileStatus2( &RequestBuffer->FileId, &RequestBuffer->AuthGroup, STATUS_NO_MEMORY);
        }
     }
     else if (RequestBuffer->RequestType == AFS_REQUEST_TYPE_BYTE_RANGE_LOCK) {
@@ -1514,6 +1519,7 @@ RDR_SetFileExtents( AFSSetFileExtentsCB *pSetFileExtentsResultCB,
 
 extern "C" DWORD
 RDR_SetFileStatus( cm_fid_t *fidp,
+                   GUID *pAuthGroup,
                    DWORD dwStatus)
 {
     WCHAR               wchBuffer[1024];
@@ -1522,6 +1528,7 @@ RDR_SetFileStatus( cm_fid_t *fidp,
     DWORD               gle;
 
     RDR_fid2FID(fidp, &SetFileStatusCB.FileId);
+    memcpy(&SetFileStatusCB.AuthGroup, pAuthGroup, sizeof(GUID));
     SetFileStatusCB.FailureStatus = dwStatus;
 
     if (afsd_logp->enabled) {
@@ -1548,6 +1555,43 @@ RDR_SetFileStatus( cm_fid_t *fidp,
     return 0;
 }
 
+static DWORD
+RDR_SetFileStatus2( AFSFileID *pFileId,
+                   GUID *pAuthGroup,
+                   DWORD dwStatus)
+{
+    WCHAR               wchBuffer[1024];
+    AFSExtentFailureCB  SetFileStatusCB;
+    DWORD               bytesReturned;
+    DWORD               gle;
+
+    memcpy(&SetFileStatusCB.FileId, pFileId, sizeof(AFSFileID));
+    memcpy(&SetFileStatusCB.AuthGroup, pAuthGroup, sizeof(GUID));
+    SetFileStatusCB.FailureStatus = dwStatus;
+
+    if (afsd_logp->enabled) {
+        swprintf( wchBuffer, L"RDR_SetFileStatus2 IOCTL_AFS_EXTENT_FAILURE_CB Fid %08lX.%08lX.%08lX.%08lX Status 0x%lX",
+                  SetFileStatusCB.FileId.Cell, SetFileStatusCB.FileId.Volume,
+                  SetFileStatusCB.FileId.Vnode, SetFileStatusCB.FileId.Unique,
+                  dwStatus);
+
+        osi_Log1(afsd_logp, "%S", osi_LogSaveStringW(afsd_logp, wchBuffer));
+    }
+
+    if( !RDR_DeviceIoControl( glDevHandle,
+                              IOCTL_AFS_SET_FILE_EXTENT_FAILURE,
+                              (void *)&SetFileStatusCB,
+                              sizeof(AFSExtentFailureCB),
+                              (void *)NULL,
+                              0,
+                              &bytesReturned ))
+    {
+        gle = GetLastError();
+        return gle;
+    }
+
+    return 0;
+}
 
 extern "C" DWORD
 RDR_RequestExtentRelease(cm_fid_t *fidp, LARGE_INTEGER numOfHeldExtents, DWORD numOfExtents, AFSFileExtentCB *extentList)