[weiden] 20931: some fixes in NtReadFile and NtWrite file for (a)synchronous pipe handling and parameter checks
some fixes in NtReadFile and NtWrite file for (a)synchronous pipe handling and parameter checks Modified: trunk/reactos/ntoskrnl/io/file.c _____
Modified: trunk/reactos/ntoskrnl/io/file.c --- trunk/reactos/ntoskrnl/io/file.c 2006-01-17 03:00:21 UTC (rev 20930) +++ trunk/reactos/ntoskrnl/io/file.c 2006-01-17 03:25:55 UTC (rev 20931) @@ -531,6 +531,8 @@
PIO_STACK_LOCATION StackPtr; PKEVENT EventObject = NULL; BOOLEAN LocalEvent = FALSE; + ULONG AccessType; + OBJECT_HANDLE_INFORMATION HandleInformation; KPROCESSOR_MODE PreviousMode = ExGetPreviousMode();
DPRINT("IopDeviceFsIoControl(DeviceHandle 0x%p Event 0x%p UserApcRoutine 0x%p " @@ -541,17 +543,84 @@ IoControlCode,InputBuffer,InputBufferLength,OutputBuffer, OutputBufferLength);
- if (IoStatusBlock == NULL) return STATUS_ACCESS_VIOLATION; + AccessType = IO_METHOD_FROM_CTL_CODE(IoControlCode);
- /* Check granted access against the access rights from IoContolCode */ + if (PreviousMode != KernelMode) + { + _SEH_TRY + { + ProbeForWrite(IoStatusBlock, + sizeof(IO_STATUS_BLOCK), + sizeof(ULONG)); + + /* probe the input and output buffers if needed */ + if (AccessType == METHOD_BUFFERED) + { + if (OutputBuffer != NULL) + { + ProbeForWrite(OutputBuffer, + OutputBufferLength, + 1); + } + else + { + /* make sure the caller can't fake this as we depend on this */ + OutputBufferLength = 0; + } + } + + if (AccessType != METHOD_NEITHER) + { + if (InputBuffer != NULL) + { + ProbeForRead(InputBuffer, + InputBufferLength, + 1); + } + else + { + /* make sure the caller can't fake this as we depend on this */ + InputBufferLength = 0; + } + } + } + _SEH_HANDLE + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + + if (!NT_SUCCESS(Status)) + { + DPRINT("Probing the buffers failed!\n"); + return Status; + } + } + + /* Don't check for access rights right now, KernelMode can do anything */ Status = ObReferenceObjectByHandle(DeviceHandle, - (IoControlCode >> 14) & 0x3, + 0, IoFileObjectType, PreviousMode, (PVOID *) &FileObject, - NULL); + &HandleInformation); if (!NT_SUCCESS(Status)) return Status;
+ /* Check for sufficient access rights */ + if (PreviousMode != KernelMode) + { + ACCESS_MASK DesiredAccess = (ACCESS_MASK)((IoControlCode >> 14) & 3); + if (DesiredAccess != FILE_ANY_ACCESS && + !RtlAreAllAccessesGranted(HandleInformation.GrantedAccess, + (ACCESS_MASK)((IoControlCode >> 14) & 3))) + { + DPRINT1("Insufficient access rights! Granted: 0x%x Desired: 0x%x\n", + HandleInformation.GrantedAccess, (ACCESS_MASK)((IoControlCode >> 14) & 3)); + ObDereferenceObject (FileObject); + return STATUS_ACCESS_DENIED; + } + } + /* Check for an event */ if (Event) { @@ -605,6 +674,19 @@ EventObject, IoStatusBlock);
+ if (Irp == NULL) + { + DPRINT1("IoBuildDeviceIoControlRequest failed!\n"); + + if (EventObject != NULL) + { + ObDereferenceObject (EventObject); + } + + ObDereferenceObject (FileObject); + return STATUS_INSUFFICIENT_RESOURCES; + } + /* Set some extra settings */ Irp->Tail.Overlay.OriginalFileObject = FileObject; Irp->RequestorMode = PreviousMode; @@ -2917,9 +2999,10 @@ PDEVICE_OBJECT DeviceObject; PIO_STACK_LOCATION StackPtr; KPROCESSOR_MODE PreviousMode; - BOOLEAN LocalEvent = FALSE; PKEVENT EventObject = NULL; LARGE_INTEGER CapturedByteOffset; + ULONG CapturedKey = 0; + BOOLEAN Synchronous = FALSE;
DPRINT("NtReadFile(FileHandle 0x%p Buffer 0x%p Length %x ByteOffset 0x%p, " "IoStatusBlock 0x%p)\n", FileHandle, Buffer, Length, ByteOffset, @@ -2945,6 +3028,10 @@ { CapturedByteOffset = ProbeForReadLargeInteger(ByteOffset); } + if (Key != NULL) + { + CapturedKey = ProbeForReadUlong(Key); + } /* FIXME - probe other pointers and capture information */ } _SEH_HANDLE @@ -2961,6 +3048,10 @@ { CapturedByteOffset = *ByteOffset; } + if (Key != NULL) + { + CapturedKey = *Key; + } }
/* Get File Object */ @@ -2972,23 +3063,29 @@ NULL); if (!NT_SUCCESS(Status)) return Status;
- /* Check the Byte Offset */ - if (ByteOffset == NULL || - (CapturedByteOffset.u.LowPart == FILE_USE_FILE_POINTER_POSITION && - CapturedByteOffset.u.HighPart == -1)) + /* Check if we should use Sync IO or not */ + if (FileObject->Flags & FO_SYNCHRONOUS_IO) { - /* a valid ByteOffset is required if asynch. op. */ - if (!(FileObject->Flags & FO_SYNCHRONOUS_IO)) + if (ByteOffset == NULL || + (CapturedByteOffset.u.LowPart == FILE_USE_FILE_POINTER_POSITION && + CapturedByteOffset.u.HighPart == -1)) { - DPRINT1("NtReadFile: missing ByteOffset for asynch. op\n"); - ObDereferenceObject(FileObject); - return STATUS_INVALID_PARAMETER; + /* Use the Current Byte OFfset */ + CapturedByteOffset = FileObject->CurrentByteOffset; }
- /* Use the Current Byte OFfset */ - CapturedByteOffset = FileObject->CurrentByteOffset; + Synchronous = TRUE; } + else if (ByteOffset == NULL && !(FileObject->Flags & FO_NAMED_PIPE)) + { + /* a valid ByteOffset is required if asynch. op. */ + DPRINT1("NtReadFile: missing ByteOffset for asynch. op (0x%p)\n", + ByteOffset);
+ ObDereferenceObject(FileObject); + return STATUS_INVALID_PARAMETER; + } + /* Check for event */ if (Event) { @@ -3017,16 +3114,7 @@ DeviceObject = IoGetRelatedDeviceObject(FileObject); }
- /* Check if we should use Sync IO or not */ - if (FileObject->Flags & FO_SYNCHRONOUS_IO) - { - /* Use File Object event */ - KeClearEvent(&FileObject->Event); - } - else - { - LocalEvent = TRUE; - } + KeClearEvent(&FileObject->Event);
/* Create the IRP */ _SEH_TRY @@ -3038,6 +3126,7 @@ &CapturedByteOffset, EventObject, IoStatusBlock); + if (Irp == NULL) { Status = STATUS_INSUFFICIENT_RESOURCES; @@ -3073,13 +3162,13 @@ /* Setup Stack Data */ StackPtr = IoGetNextIrpStackLocation(Irp); StackPtr->FileObject = FileObject; - StackPtr->Parameters.Read.Key = Key ? *Key : 0; + StackPtr->Parameters.Read.Key = CapturedKey;
/* Call the Driver */ Status = IoCallDriver(DeviceObject, Irp); if (Status == STATUS_PENDING) { - if (!LocalEvent) + if (Synchronous) { KeWaitForSingleObject(&FileObject->Event, Executive, @@ -3679,7 +3768,7 @@ PDEVICE_OBJECT DeviceObject; PIO_STACK_LOCATION StackPtr; KPROCESSOR_MODE PreviousMode; - BOOLEAN LocalEvent = FALSE; + BOOLEAN Synchronous = FALSE; PKEVENT EventObject = NULL; LARGE_INTEGER CapturedByteOffset; ULONG CapturedKey = 0; @@ -3694,9 +3783,32 @@ PreviousMode = KeGetPreviousMode(); CapturedByteOffset.QuadPart = 0;
+ /* Get File Object */ + Status = ObReferenceObjectByHandle(FileHandle, + 0, + IoFileObjectType, + PreviousMode, + (PVOID*)&FileObject, + &ObjectHandleInfo); + if (!NT_SUCCESS(Status)) return Status; + + /* If this is a named pipe, make sure we don't ask for FILE_APPEND_DATA as it + overlaps with the FILE_CREATE_PIPE_INSTANCE access right! */ + if (!(FileObject->Flags & FO_NAMED_PIPE)) + DesiredAccess |= FILE_APPEND_DATA; + /* Validate User-Mode Buffers */ if(PreviousMode != KernelMode) { + /* check if the handle has either FILE_WRITE_DATA or FILE_APPEND_DATA was + granted. */ + if (!RtlAreAnyAccessesGranted(ObjectHandleInfo.GrantedAccess, + DesiredAccess)) + { + ObDereferenceObject(FileObject); + return STATUS_ACCESS_DENIED; + } + _SEH_TRY { ProbeForWrite(IoStatusBlock, @@ -3736,62 +3848,35 @@ } }
- /* Get File Object */ - Status = ObReferenceObjectByHandle(FileHandle, - 0, - IoFileObjectType, - PreviousMode, - (PVOID*)&FileObject, - &ObjectHandleInfo); - if (!NT_SUCCESS(Status)) return Status; - - /* check if the handle has either FILE_WRITE_DATA or FILE_APPEND_DATA was - granted. However, if this is a named pipe, make sure we don't ask for - FILE_APPEND_DATA as it interferes with the FILE_CREATE_PIPE_INSTANCE - access right! */ - if (!(FileObject->Flags & FO_NAMED_PIPE)) - DesiredAccess |= FILE_APPEND_DATA; - if (!RtlAreAnyAccessesGranted(ObjectHandleInfo.GrantedAccess, - DesiredAccess)) + /* check if this is an append operation */ + if ((ObjectHandleInfo.GrantedAccess & DesiredAccess) == FILE_APPEND_DATA) { - ObDereferenceObject(FileObject); - return STATUS_ACCESS_DENIED; + /* Give the drivers something to understand */ + CapturedByteOffset.u.LowPart = FILE_WRITE_TO_END_OF_FILE; + CapturedByteOffset.u.HighPart = -1; }
- /* Check if we got write Access */ - if (ObjectHandleInfo.GrantedAccess & FILE_WRITE_DATA) + /* Check if we should use Sync IO or not */ + if (FileObject->Flags & FO_SYNCHRONOUS_IO) { - /* Check the Byte Offset */ if (ByteOffset == NULL || (CapturedByteOffset.u.LowPart == FILE_USE_FILE_POINTER_POSITION && CapturedByteOffset.u.HighPart == -1)) { - /* a valid ByteOffset is required if asynch. op. */ - if (!(FileObject->Flags & FO_SYNCHRONOUS_IO)) - { - DPRINT1("NtReadFile: missing ByteOffset for asynch. op\n"); - ObDereferenceObject(FileObject); - return STATUS_INVALID_PARAMETER; - } - /* Use the Current Byte OFfset */ CapturedByteOffset = FileObject->CurrentByteOffset; } + + Synchronous = TRUE; } - else if ((ObjectHandleInfo.GrantedAccess & FILE_APPEND_DATA) && - !(FileObject->Flags & FO_NAMED_PIPE)) + else if (ByteOffset == NULL && !(FileObject->Flags & FO_NAMED_PIPE)) { /* a valid ByteOffset is required if asynch. op. */ - if (!(FileObject->Flags & FO_SYNCHRONOUS_IO)) - { - DPRINT1("NtWriteFile: missing ByteOffset for asynch. op\n"); - ObDereferenceObject(FileObject); - return STATUS_INVALID_PARAMETER; - } + DPRINT1("NtReadFile: missing ByteOffset for asynch. op (0x%p)\n", + ByteOffset);
- /* Give the drivers somethign to understand */ - CapturedByteOffset.u.LowPart = FILE_WRITE_TO_END_OF_FILE; - CapturedByteOffset.u.HighPart = 0xffffffff; + ObDereferenceObject(FileObject); + return STATUS_INVALID_PARAMETER; }
/* Check if we got an event */ @@ -3822,16 +3907,7 @@ DeviceObject = IoGetRelatedDeviceObject(FileObject); }
- /* Check if we should use Sync IO or not */ - if (FileObject->Flags & FO_SYNCHRONOUS_IO) - { - /* Use File Object event */ - KeClearEvent(&FileObject->Event); - } - else - { - LocalEvent = TRUE; - } + KeClearEvent(&FileObject->Event);
/* Build the IRP */ _SEH_TRY @@ -3888,7 +3964,7 @@ Status = IoCallDriver(DeviceObject, Irp); if (Status == STATUS_PENDING) { - if (!LocalEvent) + if (Synchronous) { KeWaitForSingleObject(&FileObject->Event, Executive,
-
weiden＠svn.reactos.org