Re: IRP_MN_QUERY_STOP_DEVICE race in WDM book? by Eliyas
Eliyas
Fri Oct 13 11:16:36 CDT 2006
It's difficult for me to say whether you are asking a serious question or
telling us the latest revelation you received while reading the WDM book.
You post a question and then respond to your own postings every few hours.
Either you are using the newsgroups as a soundboard or there is another
BubbaGump that we don't know of answering your questions :-) In the 10 years
I have been browsing newsgroups, I haven't seen a poster like you. To the
most part I don't read your postings. Even if I do, I don't make an effort
to respond because I know you will answer that yourself given little more
time.
Sorry,
Eliyas
<BubbaGump> wrote in message
news:b57vi2h51os28kk1cal9108u8e3ej0gb06@4ax.com...
> On Fri, 13 Oct 2006 09:30:06 -0400, BubbaGump <> wrote:
>
>>On Fri, 13 Oct 2006 07:49:17 -0400, BubbaGump <> wrote:
>>
>>>Is there a race in the "devqueue" example that comes with the WDM book
>>>with regard to causing I/O's to be stopped or aborted? It looks like
>>>the testing of the abortstatus/stallcount flags and the setting of the
>>>CurrentIrp pointer is done in the wrong order. Even in service pack
>>>3, couldn't this happen:
>>>
>>> - StartPacket sees abortstatus, stallcount, and CurrentIrp are
>>>all clear
>>> - HandleQueryStop calls StallAllRequests to increment stallcount
>>> - HandleQueryStop calls WaitForCurrentIrps which sees that
>>>CurrentIrp is clear and so it doesn't wait
>>> - HandleQueryStop releases the device's resources and passes down
>>>the IRP_MN_QUERY_STOP_DEVICE to allow it to be completed
>>
>>(yes, I realize the device's resources wouldn't actually be released
>>until IRP_MN_STOP_DEVICE, but that could also occur after the
>>IRP_MN_QUERY_STOP_DEVICE completes and before StartPacket sets
>>CurrentIrp)
>>
>>
>>> - StartPacket sets CurrentIrp and initiates I/O after the
>>>device's resources have already been released (bad)
>>>
>>>Is this a test-and-set race on the part of StartPacket? To fix it,
>>>shouldn't it be made into a set-then-test (opposite order), where
>>>StartPacket would set CurrentIrp or some other equivalent flag
>>>_before_ testing abortstatus and stallcount?
>>
>>...and if, after setting this "I/O in progress" flag, StartPacket
>>notices that abortstatus or stallcount are set then it would back off
>>by queueing the IRP and clearing CurrentIrp.
>>
>>(With a test-and-set there may be two winners, and that's just bad.
>>With a set-then-test there may be two losers momentarily, but if the
>>stop path will always wait and the StartPacket path will always back
>>off then the StartPacket path will eventually be left as the sole
>>winner)
>
> Never mind. I didn't notice the single line of code that tests
> CurrentIrp in WaitForCurrentIrps is done under the protection of a
> spinlock, which will make it block if StartPacket has begun running
> until the point when CurrentIrp might be set, so WaitForCurrentIrps
> would actually see the final value and wait.
>
> (Before this, I had actually wondered why that single line of code
> that tests CurrentIrp needed to be surrounded by a lock. I guess this
> is it.)
>