[ros-dev] Re: [ros-svn] [hbirr] 14964: Do always set the UserIosb of an irp in IoSecondStageCompletion.

[Alex Ionescu]

Hartmut Birr wrote:

>Alex Ionescu wrote:
>
>  
>
>>Ok, well that indicates that our VFAT driver is broken. It should be
>>fixed instead of breaking correct code.
>>
>>    
>>
>
>That isn't correct. The bug is in IoCreateFile or/and
>SecondStageCompletion. It is possible that a FS driver returns
>STATUS_PENDING and does later complete the irp with an error.
>  
>
Ok, that's normal.

>IoCreateFile does wait on the FileObject event but in some cases
>IoSecontCompletion doesn't set the result in Irp->UserIosb. This is the
>reason for bug #609. 
>
I would say the bug is in IoCreateFile, but I haven't had much time to 
look at it.

>There exists more bugs. All Directory/File function
>may always provide an user event handle. In some conditions they must
>provide an user event handle. 
>
They must only provide the handle if the operation will need one, 
depending if the file object is opened with SYNCH_IO or not. See the 
recent fixes I made.

>IoSecondStageCompletion does sometimes
>dereference the user event and sometimes not. 
>
Yes, and that is correct behavior. It should not get derefefenced if 
it's a KEVENT. We know it's a KEVENT if the operation doesn't have 
IRP_SYNCH_API.

>An other problem is the
>copying of  the results to Irp->UserIosb. The old code has used
>MmSafeCopyToUser. It must use this function or guard the copy operation
>by an exception frame and use MmProbeForWrite. If the operation is
>asynchronous, the calling thread may be buggy and does dereference the
>status block. In this case ros does crash. 
>
This is my fault, you are right that the copy should be SEHed.

>I've the feeling, you should
>start a second rewrite of the  io completion code.
>
>  
>
I have some additional changes locally but really no time to commit them 
until around May 15th when I finish school.
Thanks for your help and valuable comments.

There are so many problems because so many things depend on the old 
code. But I can guarantee you that my changes are correct.

>- Hartmut
>  
>
Best regards,
Alex Ionescu