[PATCH v4 1/8] server: Actually set initial status in set_async_direct_result handler.

[Jin-oh Kang]

On Sat, Mar 5, 2022, 4:47 AM Jin-oh Kang <jinoh.kang.kr at gmail.com> wrote:

> On Sat, Mar 5, 2022, 4:05 AM Jinoh Kang <jinoh.kang.kr at gmail.com> wrote:
>
>> On 3/5/22 02:47, Zebediah Figura wrote:
>> > Right, but I think it's possible to trigger this assert now. For a
>> rather pathological case, consider a client which calls
>> set_async_direct_result on an async which is currently being serviced by a
>> device driver and has not yet reached get_next_device_request.
>>
>> For that to be possible, something has to terminate the async so that
>> set_async_direct_result does not reject it.
>> The easiest way to do so is to cancel it, or make it timeout somehow.
>>
>> In fact, I think we have discovered an _existing_ bug: the client can use
>> set_async_direct_result to interfere with cancelled asyncs (device-backed
>> or not).
>>
>> Further investigation revealed some other bugs, which is tangentially
>> related:
>>
>> 1. An async may time out before the client calls set_async_direct_result
>> to report back the status of initial I/O.  In this case, the timeout is
>> ignored.
>> 2. There is a time window where alerted asyncs ignore IoCancel/IoCancelEx
>> requests.  If async_terminate( async, STATUS_ALERTED ) is called, the async
>> will ignore any cancellation requests until the APC_ASYNC_IO routine
>> returns.
>>    Note that this bug has existed *before* set_async_direct_result, and I
>> think this one may need to be addressed first.
>>
>> So what I think we need here is a mechanism to tell alerted asyncs apart
>> from irreversibly-terminated asyncs.
>>
>
> To make it clear, it should be "restartable" vs. "non-restartable" asyncs.
> See below...
>
> This will solve all of the problems above:
>>
>> - It's no longer possible to call set_async_direct_result on a device
>> async in the first place, since it will now reject irreversibly-terminated
>> asyncs while still accepting alerted asyncs (which is not possible for
>> device asyncs).
>>
>
> Actually it's possible for devive asyncs to be in alerted state, but note
> that it is *not* supposed to be restartable.
>

On the second thought, the output copy APC call gets fake STATUS_ALERTED,
so I think it doesn't apply here.


> - Timeout and cancellation works as expected: set_async_direct_result will
>> treat timed out asyncs as irreversibly-terminated.  (the client needs to be
>> modified to handle this case explicitly, though.)
>>
>> Any thoughts?
>>
>> >
>> > (And as mentioned, we should probably be calling
>> async_set_initial_status() in async_set_result, in which case this could be
>> triggered much less pathologically by the controlling process dying.)
>>
>> That has been split off as patch 2/8.
>>
>>
>> --
>> Sincerely,
>> Jinoh Kang
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220305/dfa97030/attachment.htm>