[PATCH v3 0/1] MR7231: rpcrt4: Change some buffer wrap checks to satisfy Clang 20.
Soon-to-be-released Clang 20 introduced a [`-Wtautological-compare`](https://github.com/llvm/llvm-project/commit/6d34cfac53b993a6cdf3d6669e017eac...) warning which these checks trigger. Update them in a similar fashion to the example shown in the [Clang doc](https://github.com/llvm/llvm-project/blob/release/20.x/clang/docs/ReleaseNot...) (see "To avoid pointer addition overflow..."). -- v3: rpcrt4: Pointer arithmetic fixes (-Wtautological-compare). https://gitlab.winehq.org/wine/wine/-/merge_requests/7231
From: William Horvath <william(a)horvath.blog> --- dlls/rpcrt4/ndr_marshall.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/dlls/rpcrt4/ndr_marshall.c b/dlls/rpcrt4/ndr_marshall.c index c68eb0ee57e..ddb69001911 100644 --- a/dlls/rpcrt4/ndr_marshall.c +++ b/dlls/rpcrt4/ndr_marshall.c @@ -711,8 +711,8 @@ static inline ULONG safe_multiply(ULONG a, ULONG b) static inline void safe_buffer_increment(MIDL_STUB_MESSAGE *pStubMsg, ULONG size) { - if ((pStubMsg->Buffer + size < pStubMsg->Buffer) || /* integer overflow of pStubMsg->Buffer */ - (pStubMsg->Buffer + size > (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength)) + if (((unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength) < pStubMsg->Buffer || /* integer overflow of pStubMsg->BufferSize */ + size > ((unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength) - pStubMsg->Buffer) RpcRaiseException(RPC_X_BAD_STUB_DATA); pStubMsg->Buffer += size; } @@ -732,8 +732,8 @@ static inline void safe_buffer_length_increment(MIDL_STUB_MESSAGE *pStubMsg, ULO * to do so */ static inline void safe_copy_from_buffer(MIDL_STUB_MESSAGE *pStubMsg, void *p, ULONG size) { - if ((pStubMsg->Buffer + size < pStubMsg->Buffer) || /* integer overflow of pStubMsg->Buffer */ - (pStubMsg->Buffer + size > pStubMsg->BufferEnd)) + if (pStubMsg->BufferEnd < pStubMsg->Buffer || /* integer overflow of pStubMsg->Buffer */ + size > (pStubMsg->BufferEnd - pStubMsg->Buffer)) { ERR("buffer overflow - Buffer = %p, BufferEnd = %p, size = %lu\n", pStubMsg->Buffer, pStubMsg->BufferEnd, size); @@ -748,8 +748,8 @@ static inline void safe_copy_from_buffer(MIDL_STUB_MESSAGE *pStubMsg, void *p, U /* copies data to the buffer, checking that there is enough space to do so */ static inline void safe_copy_to_buffer(MIDL_STUB_MESSAGE *pStubMsg, const void *p, ULONG size) { - if ((pStubMsg->Buffer + size < pStubMsg->Buffer) || /* integer overflow of pStubMsg->Buffer */ - (pStubMsg->Buffer + size > (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength)) + if (((unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength) < pStubMsg->Buffer || /* integer overflow of pStubMsg->Buffer */ + size > ((unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength) - pStubMsg->Buffer) { ERR("buffer overflow - Buffer = %p, BufferEnd = %p, size = %lu\n", pStubMsg->Buffer, (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength, @@ -767,8 +767,8 @@ static void validate_string_data(MIDL_STUB_MESSAGE *pStubMsg, ULONG bufsize, ULO ULONG i; /* verify the buffer is safe to access */ - if ((pStubMsg->Buffer + bufsize < pStubMsg->Buffer) || - (pStubMsg->Buffer + bufsize > pStubMsg->BufferEnd)) + if (pStubMsg->BufferEnd < pStubMsg->Buffer || + bufsize > (pStubMsg->BufferEnd - pStubMsg->Buffer)) { ERR("bufsize 0x%lx exceeded buffer end %p of buffer %p\n", bufsize, pStubMsg->BufferEnd, pStubMsg->Buffer); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7231
On Mon Feb 17 18:00:51 2025 +0000, William Horvath wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/7231/diffs?diff_id=158136&start_sha=294e2338a222d73247412b989272c08d3e01a6d9#3149510afd8a6b07ec8f8b4b4a493ffae70d2105_734_735) That's a good point. I've updated each case to validate the pointer order before subtraction, without casting to `SIZE_T`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7231#note_94889
participants (2)
-
William Horvath -
William Horvath (＠whrvt)