ntoskrnl.exe: Make IoAllocateIrp not crash on negative values.
Bernhard Übelacker
bernhardu at vr-web.de
Tue Mar 29 15:17:57 CDT 2016
Hello Sebastian,
Am 28.03.2016 um 08:02 schrieb Sebastian Lackner:
> On 27.03.2016 20:27, Bernhard Übelacker wrote:
>> https://bugs.winehq.org/show_bug.cgi?id=39734
>>
>> Changes should avoid crash in acedrv11.sys.
>> IoAllocateIrp is called with a stack_size of -128.
>>
>> Tested against Windows XP.
>> (See the test based on wine-staging "driver testing framework".)
>
> For reference, here a testbot run which confirms that the change is correct (for XP/2003):
> https://newtestbot.winehq.org/JobDetails.pl?Key=21722
>
>>
>> Signed-off-by: Bernhard Übelacker <bernhardu at vr-web.de>
>> ---
>> dlls/ntoskrnl.exe/ntoskrnl.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
>> index 36488a7..f2ccc61 100644
>> --- a/dlls/ntoskrnl.exe/ntoskrnl.c
>> +++ b/dlls/ntoskrnl.exe/ntoskrnl.c
>> @@ -592,15 +592,20 @@ PIRP WINAPI IoAllocateIrp( CCHAR stack_size, BOOLEAN charge_quota )
>> {
>> SIZE_T size;
>> PIRP irp;
>> + CCHAR _stack_size = stack_size;
>>
>> TRACE( "%d, %d\n", stack_size, charge_quota );
>>
>> - size = sizeof(IRP) + stack_size * sizeof(IO_STACK_LOCATION);
>> + if (_stack_size <= 0 || (_stack_size > 1 && stack_size < 8))
>> + _stack_size = 8;
>
> I would suggest to use a different variable, its very easy to mix up "stack_size"
> and "_stack_size". You even mixed it up yourself in the if() condition above. ;)
> If you want, you can also simplify the condition to (stack_size < 8 && stack_size != 1).
>
>> +
>> + size = sizeof(IRP) + _stack_size * sizeof(IO_STACK_LOCATION);
>> irp = ExAllocatePool( NonPagedPool, size );
>> if (irp == NULL)
>> return NULL;
>> IoInitializeIrp( irp, size, stack_size );
>> - irp->AllocationFlags = IRP_ALLOCATED_FIXED_SIZE;
>> + if (stack_size >= 1 && stack_size <= 8)
>> + irp->AllocationFlags = IRP_ALLOCATED_FIXED_SIZE;
>> if (charge_quota)
>> irp->AllocationFlags |= IRP_LOOKASIDE_ALLOCATION;
>> return irp;
>>
thanks for your review.
I have sent an updated patch.
Is the test attached to the bug worth to be included into wine-staging?
Kind regards,
Bernhard
More information about the wine-devel
mailing list