serialize file-contents request state and poison streams after timeout

- Extract lock_mutex() for the WAIT_OBJECT_0/WAIT_ABANDONED idiom shared by
  take_req_fdata, the request-serialization acquire, and the response handler.
- Collapse the acquire/send/take/release cycle into
  cliprdr_request_filecontents_sync(), used by CliprdrStream_Read and the size
  probe in CliprdrStream_New.
- Publish req_f_stream_id_expected/req_f_size_requested under req_f_mutex in the
  sender and read them under the same lock in the response handler, removing the
  cross-thread data race on those fields.
- Poison a stream (m_failed) after a request fails/times out, so a late response
  carrying a previous offset's bytes cannot satisfy a later same-stream read.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
rustdesk
2026-07-04 18:07:55 +08:00
parent bb8ffef7f0
commit 3ab9d4e46d

View File

@@ -235,6 +235,9 @@ struct _CliprdrStream
// Unique id sent as the CLIPRDR streamId; also keys the peer's cached stream in // Unique id sent as the CLIPRDR streamId; also keys the peer's cached stream in
// wf_cliprdr_server_file_contents_request(). // wf_cliprdr_server_file_contents_request().
UINT32 m_streamId; UINT32 m_streamId;
// Set after a request on this stream fails/times out; poisons later reads so a
// late response cannot be mistaken for the current read (streamId is per-stream).
BOOL m_failed;
}; };
typedef struct _CliprdrStream CliprdrStream; typedef struct _CliprdrStream CliprdrStream;
@@ -414,12 +417,20 @@ static ULONG STDMETHODCALLTYPE CliprdrStream_Release(IStream *This)
} }
} }
// Acquire `mutex`, returning TRUE if it is held on return. WAIT_ABANDONED still
// transfers ownership; WAIT_FAILED / WAIT_TIMEOUT do not, so the caller must then not
// release the mutex.
static BOOL lock_mutex(HANDLE mutex, DWORD timeoutMillis)
{
DWORD wait = WaitForSingleObject(mutex, timeoutMillis);
return (wait == WAIT_OBJECT_0 || wait == WAIT_ABANDONED);
}
// Take ownership of the pending file-contents response buffer, clearing the shared // Take ownership of the pending file-contents response buffer, clearing the shared
// slot under req_f_mutex. The caller must free the returned buffer. // slot under req_f_mutex. The caller must free the returned buffer.
static void take_req_fdata(wfClipboard *clipboard, char **data, ULONG *size) static void take_req_fdata(wfClipboard *clipboard, char **data, ULONG *size)
{ {
DWORD wait = WaitForSingleObject(clipboard->req_f_mutex, INFINITE); if (!lock_mutex(clipboard->req_f_mutex, INFINITE))
if (wait != WAIT_OBJECT_0 && wait != WAIT_ABANDONED)
{ {
*data = NULL; *data = NULL;
*size = 0; *size = 0;
@@ -439,21 +450,42 @@ static void take_req_fdata(wfClipboard *clipboard, char **data, ULONG *size)
static BOOL acquire_req_f_request_mutex(wfClipboard *clipboard) static BOOL acquire_req_f_request_mutex(wfClipboard *clipboard)
{ {
DWORD timeoutMillis; DWORD timeoutMillis;
DWORD wait;
if (!clipboard || !clipboard->context || !clipboard->req_f_request_mutex) if (!clipboard || !clipboard->context || !clipboard->req_f_request_mutex)
return FALSE; return FALSE;
// Long enough for a predecessor's full request cycle plus a margin of its own. // Long enough for a predecessor's full request cycle plus a margin of its own.
timeoutMillis = (clipboard->context->ResponseWaitTimeoutSecs + 1) * 2000; timeoutMillis = (clipboard->context->ResponseWaitTimeoutSecs + 1) * 2000;
wait = WaitForSingleObject(clipboard->req_f_request_mutex, timeoutMillis); return lock_mutex(clipboard->req_f_request_mutex, timeoutMillis);
return (wait == WAIT_OBJECT_0 || wait == WAIT_ABANDONED); }
// Run one full file-contents request cycle under req_f_request_mutex: acquire, send,
// wait for the response, and take ownership of the response buffer into *outData (which
// the caller must free). Returns the send/wait status; CHANNEL_RC_OK on success.
static UINT cliprdr_request_filecontents_sync(wfClipboard *clipboard, UINT32 connID, UINT32 streamId,
ULONG index, UINT32 flag, DWORD positionHigh,
DWORD positionLow, ULONG nreq, char **outData,
ULONG *outSize)
{
UINT rc;
*outData = NULL;
*outSize = 0;
if (!acquire_req_f_request_mutex(clipboard))
return ERROR_INTERNAL_ERROR;
rc = cliprdr_send_request_filecontents(clipboard, connID, streamId, index, flag, positionHigh,
positionLow, nreq);
take_req_fdata(clipboard, outData, outSize);
ReleaseMutex(clipboard->req_f_request_mutex);
return rc;
} }
static HRESULT STDMETHODCALLTYPE CliprdrStream_Read(IStream *This, void *pv, ULONG cb, static HRESULT STDMETHODCALLTYPE CliprdrStream_Read(IStream *This, void *pv, ULONG cb,
ULONG *pcbRead) ULONG *pcbRead)
{ {
int ret; UINT ret;
ULONG req_size = 0; ULONG req_size = 0;
char *req_data = NULL; char *req_data = NULL;
CliprdrStream *instance = (CliprdrStream *)This; CliprdrStream *instance = (CliprdrStream *)This;
@@ -465,22 +497,22 @@ static HRESULT STDMETHODCALLTYPE CliprdrStream_Read(IStream *This, void *pv, ULO
clipboard = (wfClipboard *)instance->m_pData; clipboard = (wfClipboard *)instance->m_pData;
*pcbRead = 0; *pcbRead = 0;
// A prior request on this stream failed/timed out; a late response could satisfy
// this read with a previous offset's bytes, so fail every later read instead.
if (instance->m_failed)
return E_FAIL;
if (instance->m_lOffset.QuadPart >= instance->m_lSize.QuadPart) if (instance->m_lOffset.QuadPart >= instance->m_lSize.QuadPart)
return S_FALSE; return S_FALSE;
if (!acquire_req_f_request_mutex(clipboard)) ret = cliprdr_request_filecontents_sync(clipboard, instance->m_connID, instance->m_streamId,
return E_FAIL;
ret = cliprdr_send_request_filecontents(clipboard, instance->m_connID, instance->m_streamId,
instance->m_lIndex, FILECONTENTS_RANGE, instance->m_lIndex, FILECONTENTS_RANGE,
instance->m_lOffset.HighPart, instance->m_lOffset.HighPart, instance->m_lOffset.LowPart,
instance->m_lOffset.LowPart, cb); cb, &req_data, &req_size);
take_req_fdata(clipboard, &req_data, &req_size);
ReleaseMutex(clipboard->req_f_request_mutex);
if (ret != CHANNEL_RC_OK) if (ret != CHANNEL_RC_OK)
{ {
instance->m_failed = TRUE;
free(req_data); free(req_data);
return E_FAIL; return E_FAIL;
} }
@@ -718,18 +750,12 @@ static CliprdrStream *CliprdrStream_New(UINT32 connID, ULONG index, void *pData,
if (((instance->m_Dsc.dwFlags & FD_FILESIZE) == 0) && !isDir) if (((instance->m_Dsc.dwFlags & FD_FILESIZE) == 0) && !isDir)
{ {
/* get content size of this stream */ /* get content size of this stream */
if (acquire_req_f_request_mutex(clipboard)) if (cliprdr_request_filecontents_sync(clipboard, instance->m_connID,
instance->m_streamId, instance->m_lIndex,
FILECONTENTS_SIZE, 0, 0, 8, &req_data,
&req_sz) == CHANNEL_RC_OK)
{ {
if (cliprdr_send_request_filecontents(clipboard, instance->m_connID, success = TRUE;
instance->m_streamId, instance->m_lIndex,
FILECONTENTS_SIZE, 0, 0,
8) == CHANNEL_RC_OK)
{
success = TRUE;
}
take_req_fdata(clipboard, &req_data, &req_sz);
ReleaseMutex(clipboard->req_f_request_mutex);
} }
if (req_data != NULL && req_sz >= sizeof(LONGLONG)) if (req_data != NULL && req_sz >= sizeof(LONGLONG))
@@ -1890,13 +1916,9 @@ UINT cliprdr_send_request_filecontents(wfClipboard *clipboard, UINT32 connID, UI
{ {
return rc; return rc;
} }
clipboard->req_f_received = FALSE;
clipboard->req_f_response_ok = FALSE;
clipboard->req_f_size_requested = nreq;
fileContentsRequest.connID = connID; fileContentsRequest.connID = connID;
fileContentsRequest.streamId = streamId; fileContentsRequest.streamId = streamId;
clipboard->req_f_stream_id_expected = streamId;
fileContentsRequest.listIndex = index; fileContentsRequest.listIndex = index;
fileContentsRequest.dwFlags = flag; fileContentsRequest.dwFlags = flag;
fileContentsRequest.nPositionLow = positionlow; fileContentsRequest.nPositionLow = positionlow;
@@ -1904,6 +1926,17 @@ UINT cliprdr_send_request_filecontents(wfClipboard *clipboard, UINT32 connID, UI
fileContentsRequest.cbRequested = nreq; fileContentsRequest.cbRequested = nreq;
fileContentsRequest.clipDataId = 0; fileContentsRequest.clipDataId = 0;
fileContentsRequest.msgFlags = 0; fileContentsRequest.msgFlags = 0;
// Publish the per-request expectations under req_f_mutex so the channel-thread
// response handler reads a consistent (stream id, requested size) pair.
if (!lock_mutex(clipboard->req_f_mutex, INFINITE))
return ERROR_INTERNAL_ERROR;
clipboard->req_f_received = FALSE;
clipboard->req_f_response_ok = FALSE;
clipboard->req_f_size_requested = nreq;
clipboard->req_f_stream_id_expected = streamId;
ReleaseMutex(clipboard->req_f_mutex);
rc = clipboard->context->ClientFileContentsRequest(clipboard->context, &fileContentsRequest); rc = clipboard->context->ClientFileContentsRequest(clipboard->context, &fileContentsRequest);
if (rc != ERROR_SUCCESS) if (rc != ERROR_SUCCESS)
{ {
@@ -3372,7 +3405,6 @@ wf_cliprdr_server_file_contents_response(CliprdrClientContext *context,
wfClipboard *clipboard = NULL; wfClipboard *clipboard = NULL;
UINT rc = ERROR_INTERNAL_ERROR; UINT rc = ERROR_INTERNAL_ERROR;
BOOL locked = FALSE; BOOL locked = FALSE;
DWORD wait;
do do
{ {
@@ -3389,19 +3421,21 @@ wf_cliprdr_server_file_contents_response(CliprdrClientContext *context,
break; break;
} }
// A response for another stream is ignored without waking the waiter. // req_f_mutex also guards req_f_stream_id_expected / req_f_size_requested, so
if (fileContentsResponse->streamId != clipboard->req_f_stream_id_expected) // take it before reading them.
return CHANNEL_RC_OK; if (!lock_mutex(clipboard->req_f_mutex, INFINITE))
// WAIT_ABANDONED still means the mutex is held; WAIT_FAILED means it is not,
// so it must not be released.
wait = WaitForSingleObject(clipboard->req_f_mutex, INFINITE);
locked = (wait == WAIT_OBJECT_0 || wait == WAIT_ABANDONED);
if (!locked)
{ {
rc = ERROR_INTERNAL_ERROR; rc = ERROR_INTERNAL_ERROR;
break; break;
} }
locked = TRUE;
// A response for another stream is ignored without waking the waiter.
if (fileContentsResponse->streamId != clipboard->req_f_stream_id_expected)
{
ReleaseMutex(clipboard->req_f_mutex);
return CHANNEL_RC_OK;
}
// Free any leftover buffer from a prior request that was never consumed. // Free any leftover buffer from a prior request that was never consumed.
free(clipboard->req_fdata); free(clipboard->req_fdata);