[ros-kernel] Bugs in NtRead/WriteFile (Eric)

[Eric Kohl]

"Gunnar André Dalsnes" <hardon at online.no> wrote:

> Eric, lately you have made some changes to NtRead/WriteFile (and others?)
> where you use a local "safe" variable for the IoStatusBlock in the irp.
This
> surely breaks asynchronous io! When the io completes (after the call to
> NtRead/WriteFile has returned!!) it will try to copy Irp->IoStatus to what
> Irp->UserIosb points to, but since Irp->UserIosb points to a "safe" stack
> variable now out of scope, it will crash!

I had a similar problem with synchronous io operations in the disk format
code of usetup.exe (fslib). IoStatusBlock, which was allocated from the
user-mode stack, was out of scope when the disk driver completed a write
operation. Usetup.exe crashed in kernel mode.

For synchronous operations, a safe IoStatusBlock should be allocated from
non-paged pool and use the MmSafeCopy* routines to copy the io status upon
completion.

For asynchronous operations, a pointer to the original IoStatus Block should
be passed to the Irp.

Can you agree with that?


> And another change you made I don't understand:
>
> You changed
> Status = IoCallDriver(FileObject->DeviceObject, Irp);
> if (Status == STATUS_PENDING && FileObject->Flags & FO_SYNCHRONOUS_IO)
>
> Into
> Status = IoCallDriver(FileObject->DeviceObject, Irp);
> if (Event == NULL && Status == STATUS_PENDING && (FileObject->Flags &
> FO_SYNCHRONOUS_IO))
>
> This looks very wrong, since NtRead/WriteFile should *allways* wait for
> io-completion in case of synchronous operations, event when the user
> supplies an event!!!
> Asynchronous operations are different thou: when supplying an event, only
> the event will become signaled and not the file object. But this is
allready
> taken care of in IoSecondStageCompletion.

Okay! That sounds reasonable. I'll revert it. I thought that
providing an event object meant that the operation will be asynchronous.


Regards,
Eric