[PATCH] dlls/{mciwave, winmm}: properly handle mmioRead/mmioWrite in case of errors
In C, an inequality comparison between a signed and an unsigned integer ends up with an unsigned comparison. So, force signed comparison to catch both case of errors and partial read operation. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52628 Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/mciwave/mciwave.c | 3 ++- dlls/winmm/mmio.c | 8 +++----- dlls/winmm/playsound.c | 3 ++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dlls/mciwave/mciwave.c b/dlls/mciwave/mciwave.c index 1d9abce84a3..3bb271f66ec 100644 --- a/dlls/mciwave/mciwave.c +++ b/dlls/mciwave/mciwave.c @@ -291,7 +291,8 @@ static DWORD WAVE_mciReadFmt(WINE_MCIWAVE* wmw, const MMCKINFO* pckMainRIFF) if (!pwfx) return MCIERR_OUT_OF_MEMORY; r = mmioRead(wmw->hFile, (HPSTR)pwfx, mmckInfo.cksize); - if (r < sizeof(PCMWAVEFORMAT)) { + /* forcing signed comparison to catch errors (negative value) as well */ + if (r < (LONG)sizeof(PCMWAVEFORMAT)) { HeapFree(GetProcessHeap(), 0, pwfx); return MCIERR_INVALID_FILE; } diff --git a/dlls/winmm/mmio.c b/dlls/winmm/mmio.c index 352da62a215..3bcde1fe34b 100644 --- a/dlls/winmm/mmio.c +++ b/dlls/winmm/mmio.c @@ -1231,10 +1231,8 @@ MMRESULT WINAPI mmioDescend(HMMIO hmmio, LPMMCKINFO lpck, while (TRUE) { - LONG ix; - - ix = mmioRead(hmmio, (LPSTR)lpck, 3 * sizeof(DWORD)); - if (ix < 2*sizeof(DWORD)) + /* forcing signed comparison to catch errors as well */ + if (mmioRead(hmmio, (LPSTR)lpck, 3 * sizeof(DWORD)) < (LONG)(2*sizeof(DWORD))) { mmioSeek(hmmio, dwOldPos, SEEK_SET); WARN("return ChunkNotFound\n"); @@ -1336,7 +1334,7 @@ MMRESULT WINAPI mmioCreateChunk(HMMIO hmmio, MMCKINFO* lpck, UINT uFlags) ix = mmioWrite(hmmio, (LPSTR)lpck, size); TRACE("after mmioWrite ix = %ld req = %ld, errno = %d\n", ix, size, errno); - if (ix < size) { + if (ix != size) { mmioSeek(hmmio, dwOldPos, SEEK_SET); WARN("return CannotWrite\n"); return MMIOERR_CANNOTWRITE; diff --git a/dlls/winmm/playsound.c b/dlls/winmm/playsound.c index 0858f958583..95131bddce6 100644 --- a/dlls/winmm/playsound.c +++ b/dlls/winmm/playsound.c @@ -361,7 +361,8 @@ static DWORD WINAPI proc_PlaySound(LPVOID arg) lpWaveFormat = HeapAlloc(GetProcessHeap(), 0, mmckInfo.cksize); if (!lpWaveFormat) goto errCleanUp; - if (mmioRead(hmmio, (HPSTR)lpWaveFormat, mmckInfo.cksize) < sizeof(PCMWAVEFORMAT)) + /* forcing signed comparison to catch errors as well */ + if (mmioRead(hmmio, (HPSTR)lpWaveFormat, mmckInfo.cksize) < (LONG)sizeof(PCMWAVEFORMAT)) goto errCleanUp; TRACE("wFormatTag=%04X !\n", lpWaveFormat->wFormatTag);
On Mon, Mar 28, 2022 at 02:38:01PM +0200, Eric Pouech wrote:
In C, an inequality comparison between a signed and an unsigned integer ends up with an unsigned comparison. So, force signed comparison to catch both case of errors and partial read operation.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52628 Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com>
--- dlls/mciwave/mciwave.c | 3 ++- dlls/winmm/mmio.c | 8 +++----- dlls/winmm/playsound.c | 3 ++-
Let's do one patch per module.
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/dlls/mciwave/mciwave.c b/dlls/mciwave/mciwave.c index 1d9abce84a3..3bb271f66ec 100644 --- a/dlls/mciwave/mciwave.c +++ b/dlls/mciwave/mciwave.c @@ -291,7 +291,8 @@ static DWORD WAVE_mciReadFmt(WINE_MCIWAVE* wmw, const MMCKINFO* pckMainRIFF) if (!pwfx) return MCIERR_OUT_OF_MEMORY;
r = mmioRead(wmw->hFile, (HPSTR)pwfx, mmckInfo.cksize); - if (r < sizeof(PCMWAVEFORMAT)) { + /* forcing signed comparison to catch errors (negative value) as well */ + if (r < (LONG)sizeof(PCMWAVEFORMAT)) {
I don't like the extra comment, but I don't have a much better idea either. What do you think about something like this instead? if (r < 0 || r < sizeof(PCMWAVEFORMAT)) { HeapFree(GetProcessHeap(), 0, pwfx); return MCIERR_INVALID_FILE; } It's still not great, but I think this makes it more clear how we are using the API. Andrew
participants (2)
-
Andrew Eikum -
Eric Pouech