[PATCH] rpcrt4: wait_async_request: return error if we received an error
Marcus Meissner
marcus at jet.franken.de
Wed Oct 3 07:57:39 CDT 2012
On Tue, Oct 02, 2012 at 01:19:37PM +0200, Jacek Caban wrote:
> Hi Marcus,
>
> On 10/01/12 23:00, Marcus Meissner wrote:
> > Hi,
> >
> > Various coverity issues complain about user-after-free scenarios,
> > all involving this code path.
> >
> > I strongly think if call_ret signals error, we also need to return
> > an error condition to avoid the callers from proceeding as if nothing
> > happened.
> >
> > Second reiteration with Jaceks comment applied
>
> Sorry for not catching this later, I was concentrated on your comment
> instead of the code, but the important thing is that true call_ret means
> success (and wininet doing request asynchronously is signalled as an
> error). It means that in this case we want to return RPC_S_OK. What is
> the exact problem?
Ok, my fix was likely bad.
Coverity is spotting use-after-free scenarios.
CID 715805
rpcrt4_http_prepare_in_pipe()
/* prepare in pipe */
status = send_echo_request(in_request, async_data, cancel_event);
if (status != RPC_S_OK) return status;
... here it thinks async_data might be returned freed in the non-async case ...
memset(&buffers_in, 0, sizeof(buffers_in));
buffers_in.dwStructSize = sizeof(buffers_in);
/* FIXME: get this from the registry */
buffers_in.dwBufferTotal = 1024 * 1024 * 1024; /* 1Gb */
prepare_async_request(async_data);
... but it is again referenced here ...
ret = HttpSendRequestExW(in_request, &buffers_in, NULL, 0, 0);
status = wait_async_request(async_data, ret, cancel_event);
if (status != RPC_S_OK) return status;
send_echo_request() looks like:
static RPC_STATUS send_echo_request(HINTERNET req, RpcHttpAsyncData *async_data, HANDLE cancel_event)
{
DWORD bytes_read;
BYTE buf[20];
BOOL ret;
RPC_STATUS status;
prepare_async_request(async_data);
ret = HttpSendRequestW(req, NULL, 0, NULL, 0);
status = wait_async_request(async_data, ret, cancel_event);
if (status != RPC_S_OK) return status;
status = rpcrt4_http_check_response(req);
if (status != RPC_S_OK) return status;
InternetReadFile(req, buf, sizeof(buf), &bytes_read);
/* FIXME: do something with retrieved data */
return RPC_S_OK;
}
so with the bumped reference count via prepare_async_request I think it might be safe here.
(and so is a coverity misdetection I think now)
Ciao, Marcus
More information about the wine-devel
mailing list