[PATCH resend v5 4/5] mountmgr: Add support for volume information queries.

[Chip Davis]

February 4, 2021 1:53 PM, "Chip Davis" <cdavis at codeweavers.com> wrote:

> February 4, 2021 12:59 PM, "Erich E. Hoover" <erich.e.hoover at gmail.com> wrote:
> 
>> On Thu, Feb 4, 2021 at 11:42 AM Chip Davis <cdavis at codeweavers.com> wrote:
>> 
>>> February 4, 2021 10:40 AM, "Erich E. Hoover" <erich.e.hoover at gmail.com> wrote:
>>> 
>>> @@ -1826,6 +1828,117 @@ static void query_property( struct disk_device *device, IRP *irp )
>>> [...]
>>> +done:
>>> + LeaveCriticalSection( &device_section );
>>> + IoCompleteRequest( irp, IO_NO_INCREMENT );
>>> + return STATUS_SUCCESS;
>>> +}
>>> 
>>> Shouldn't that return what's in io->u.Status? I thought MS recommended doing that for driver
>>> dispatch handlers.
>> 
>> I just copied what we were doing in
>> dlls/mountmgr.sys/device.c:harddisk_ioctl, and it looks like it used
>> to be that way and AJ changed it in
>> 41eb2fd714c18422381f46e8b4f3608c06bef627. Do you happen to have a
>> link to wherever you saw that recommendation?
> 
> https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/different-ways-of-handling-irps-che
> t-sheet
> 
> See scenario 5.

It looks like that change was intended to prevent accessing IRP memory after it might have been freed. The thing to do, then, is to copy the status out of the IOSB into a local variable before calling IoCompleteRequest().

Chip