[PATCH 0/9] MR4519: gdi32/emf: Recording improvements to the record alignment
Spotted after EMR_STRETCHDIBITS crash in https://bugs.winehq.org/show_bug.cgi?id=54603. Bitmap size triggers 4 bytes alignment assert. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/gdi32/emfdc.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index 231b4ae217d..83e1b27d27b 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -30,6 +30,15 @@ WINE_DEFAULT_DEBUG_CHANNEL(enhmetafile); +static inline UINT aligned_size(UINT size) +{ + return (size + 3) & ~3; +} + +static inline void pad_record(void *record, UINT size) +{ + if (size & 3) memset((char *)record + size, 0, 4 - (size & 3)); +} struct emf { @@ -1159,7 +1168,7 @@ BOOL EMFDC_PolyDraw( DC_ATTR *dc_attr, const POINT *pts, const BYTE *types, DWOR size = use_small_emr ? offsetof( EMRPOLYDRAW16, apts[count] ) : offsetof( EMRPOLYDRAW, aptl[count] ); - size += (count + 3) & ~3; + size += aligned_size(count); if (!(emr = HeapAlloc( GetProcessHeap(), 0, size ))) return FALSE; @@ -1169,7 +1178,7 @@ BOOL EMFDC_PolyDraw( DC_ATTR *dc_attr, const POINT *pts, const BYTE *types, DWOR types_dest = store_points( emr->aptl, pts, count, use_small_emr ); memcpy( types_dest, types, count ); - if (count & 3) memset( types_dest + count, 0, 4 - (count & 3) ); + pad_record( types_dest, count ); if (!emf->path) get_points_bounds( &emr->rclBounds, pts, count, 0 ); @@ -2510,7 +2519,7 @@ BOOL WINAPI GdiComment( HDC hdc, UINT bytes, const BYTE *buffer ) if (!(dc_attr = get_dc_attr( hdc )) || !get_dc_emf( dc_attr )) return FALSE; - rounded_size = (bytes+3) & ~3; + rounded_size = aligned_size(bytes); total = offsetof(EMRGDICOMMENT,Data) + rounded_size; emr = HeapAlloc(GetProcessHeap(), 0, total); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/gdi32/emfdc.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index 83e1b27d27b..a3e263ae2a0 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -2514,20 +2514,19 @@ BOOL WINAPI GdiComment( HDC hdc, UINT bytes, const BYTE *buffer ) { DC_ATTR *dc_attr; EMRGDICOMMENT *emr; - UINT total, rounded_size; + UINT total; BOOL ret; if (!(dc_attr = get_dc_attr( hdc )) || !get_dc_emf( dc_attr )) return FALSE; - rounded_size = aligned_size(bytes); - total = offsetof(EMRGDICOMMENT,Data) + rounded_size; + total = offsetof(EMRGDICOMMENT,Data) + aligned_size(bytes); emr = HeapAlloc(GetProcessHeap(), 0, total); emr->emr.iType = EMR_GDICOMMENT; emr->emr.nSize = total; emr->cbData = bytes; - memset(&emr->Data[bytes], 0, rounded_size - bytes); memcpy(&emr->Data[0], buffer, bytes); + pad_record(&emr->Data[0], bytes); ret = emfdc_record( get_dc_emf( dc_attr ), &emr->emr ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/gdi32/emfdc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index a3e263ae2a0..0d53fe8d553 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -2522,6 +2522,9 @@ BOOL WINAPI GdiComment( HDC hdc, UINT bytes, const BYTE *buffer ) total = offsetof(EMRGDICOMMENT,Data) + aligned_size(bytes); emr = HeapAlloc(GetProcessHeap(), 0, total); + if (!emr) + return FALSE; + emr->emr.iType = EMR_GDICOMMENT; emr->emr.nSize = total; emr->cbData = bytes; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/gdi32/emfdc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index 0d53fe8d553..c8b7327ce5b 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -2638,6 +2638,7 @@ static struct emf *emf_create( HDC hdc, const RECT *rect, const WCHAR *descripti DWORD size = 0, length = 0; DC_ATTR *dc_attr; struct emf *emf; + void *ptr; if (!(dc_attr = get_dc_attr( hdc )) || !(emf = HeapAlloc( GetProcessHeap(), 0, sizeof(*emf) ))) return NULL; @@ -2649,7 +2650,7 @@ static struct emf *emf_create( HDC hdc, const RECT *rect, const WCHAR *descripti length += 3; length *= 2; } - size = sizeof(ENHMETAHEADER) + (length + 3) / 4 * 4; + size = sizeof(ENHMETAHEADER) + aligned_size(length); if (!(emf->emh = HeapAlloc( GetProcessHeap(), 0, size )) || !(emf->handles = HeapAlloc( GetProcessHeap(), 0, @@ -2674,7 +2675,8 @@ static struct emf *emf_create( HDC hdc, const RECT *rect, const WCHAR *descripti emf->emh->nDescription = length / 2; emf->emh->offDescription = length ? sizeof(ENHMETAHEADER) : 0; - memcpy( (char *)emf->emh + sizeof(ENHMETAHEADER), description, length ); + ptr = memcpy( (char *)emf->emh + sizeof(ENHMETAHEADER), description, length ); + pad_record( ptr, length ); emf_reset( dc_attr, rect ); return emf; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/gdi32/emfdc.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index c8b7327ce5b..a93696d0446 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -1899,13 +1899,14 @@ BOOL EMFDC_StretchDIBits( DC_ATTR *dc_attr, INT x_dst, INT y_dst, INT width_dst, UINT bmi_size, img_size, payload_size, emr_size; BITMAPINFOHEADER bih; BITMAPINFO *bi; + void *ptr; /* calculate the size of the colour table and the image */ if (!emf_parse_user_bitmapinfo( &bih, &info->bmiHeader, usage, TRUE, &bmi_size, &img_size )) return 0; /* check for overflows */ - payload_size = bmi_size + img_size; + payload_size = aligned_size(bmi_size) + aligned_size(img_size); if (payload_size < bmi_size) return 0; emr_size = sizeof (EMRSTRETCHDIBITS) + payload_size; @@ -1914,14 +1915,6 @@ BOOL EMFDC_StretchDIBits( DC_ATTR *dc_attr, INT x_dst, INT y_dst, INT width_dst, /* allocate record */ if (!(emr = HeapAlloc(GetProcessHeap(), 0, emr_size ))) return 0; - /* write a bitmap info header (with colours) to the record */ - bi = (BITMAPINFO *)&emr[1]; - bi->bmiHeader = bih; - emf_copy_colours_from_user_bitmapinfo( bi, info, usage ); - - /* write bitmap bits to the record */ - memcpy ( (BYTE *)&emr[1] + bmi_size, bits, img_size ); - /* fill in the EMR header at the front of our piece of memory */ emr->emr.iType = EMR_STRETCHDIBITS; emr->emr.nSize = emr_size; @@ -1937,7 +1930,7 @@ BOOL EMFDC_StretchDIBits( DC_ATTR *dc_attr, INT x_dst, INT y_dst, INT width_dst, emr->iUsageSrc = usage; emr->offBmiSrc = sizeof (EMRSTRETCHDIBITS); emr->cbBmiSrc = bmi_size; - emr->offBitsSrc = emr->offBmiSrc + bmi_size; + emr->offBitsSrc = emr->offBmiSrc + aligned_size(bmi_size); emr->cbBitsSrc = img_size; emr->cxSrc = width_src; @@ -1948,6 +1941,16 @@ BOOL EMFDC_StretchDIBits( DC_ATTR *dc_attr, INT x_dst, INT y_dst, INT width_dst, emr->rclBounds.right = x_dst + width_dst - 1; emr->rclBounds.bottom = y_dst + height_dst - 1; + /* write a bitmap info header (with colours) to the record */ + bi = (BITMAPINFO *)((BYTE *)emr + emr->offBmiSrc); + bi->bmiHeader = bih; + emf_copy_colours_from_user_bitmapinfo( bi, info, usage ); + pad_record( bi, emr->cbBmiSrc ); + + /* write bitmap bits to the record */ + ptr = memcpy ( (BYTE *)emr + emr->offBitsSrc, bits, img_size ); + pad_record( ptr, emr->cbBitsSrc ); + /* save the record we just created */ ret = emfdc_record( get_dc_emf( dc_attr ), &emr->emr ); if (ret) emfdc_update_bounds( get_dc_emf( dc_attr ), &emr->rclBounds ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/gdi32/emfdc.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index a93696d0446..fdd915a106f 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -1968,6 +1968,7 @@ BOOL EMFDC_SetDIBitsToDevice( DC_ATTR *dc_attr, INT x_dst, INT y_dst, DWORD widt UINT src_height, stride; BITMAPINFOHEADER bih; BITMAPINFO *bi; + void *ptr; /* calculate the size of the colour table and the image */ if (!emf_parse_user_bitmapinfo( &bih, &info->bmiHeader, usage, TRUE, @@ -1993,7 +1994,7 @@ BOOL EMFDC_SetDIBitsToDevice( DC_ATTR *dc_attr, INT x_dst, INT y_dst, DWORD widt } /* check for overflows */ - payload_size = bmi_size + img_size; + payload_size = aligned_size(bmi_size) + aligned_size(img_size); if (payload_size < bmi_size) return 0; emr_size = sizeof (EMRSETDIBITSTODEVICE) + payload_size; @@ -2002,14 +2003,6 @@ BOOL EMFDC_SetDIBitsToDevice( DC_ATTR *dc_attr, INT x_dst, INT y_dst, DWORD widt /* allocate record */ if (!(emr = HeapAlloc( GetProcessHeap(), 0, emr_size ))) return FALSE; - /* write a bitmap info header (with colours) to the record */ - bi = (BITMAPINFO *)&emr[1]; - bi->bmiHeader = bih; - emf_copy_colours_from_user_bitmapinfo( bi, info, usage ); - - /* write bitmap bits to the record */ - memcpy ( (BYTE *)&emr[1] + bmi_size, bits, img_size ); - emr->emr.iType = EMR_SETDIBITSTODEVICE; emr->emr.nSize = emr_size; emr->rclBounds.left = x_dst; @@ -2024,12 +2017,22 @@ BOOL EMFDC_SetDIBitsToDevice( DC_ATTR *dc_attr, INT x_dst, INT y_dst, DWORD widt emr->cySrc = height; emr->offBmiSrc = sizeof(EMRSETDIBITSTODEVICE); emr->cbBmiSrc = bmi_size; - emr->offBitsSrc = sizeof(EMRSETDIBITSTODEVICE) + bmi_size; + emr->offBitsSrc = sizeof(EMRSETDIBITSTODEVICE) + aligned_size(bmi_size); emr->cbBitsSrc = img_size; emr->iUsageSrc = usage; emr->iStartScan = startscan; emr->cScans = lines; + /* write a bitmap info header (with colours) to the record */ + bi = (BITMAPINFO *)((BYTE *)emr + emr->offBmiSrc); + bi->bmiHeader = bih; + emf_copy_colours_from_user_bitmapinfo( bi, info, usage ); + pad_record( bi, bmi_size ); + + /* write bitmap bits to the record */ + ptr = memcpy ( (BYTE *)emr + emr->offBitsSrc, bits, img_size ); + pad_record( ptr, img_size ); + if ((ret = emfdc_record( get_dc_emf( dc_attr ), (EMR*)emr ))) emfdc_update_bounds( get_dc_emf( dc_attr ), &emr->rclBounds ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/gdi32/emfdc.c | 16 +++++----------- include/wingdi.h | 8 ++++++++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index fdd915a106f..5fc0109f78f 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -1194,26 +1194,20 @@ BOOL EMFDC_PolyDraw( DC_ATTR *dc_attr, const POINT *pts, const BYTE *types, DWOR INT EMFDC_ExtEscape( DC_ATTR *dc_attr, INT escape, INT input_size, const char *input, INT output_size, char *output) { - struct EMREXTESCAPE - { - EMR emr; - DWORD escape; - DWORD size; - BYTE data[1]; - } *emr; + EMREXTESCAPE *emr; size_t size; if (escape == QUERYESCSUPPORT) return 0; - size = FIELD_OFFSET( struct EMREXTESCAPE, data[input_size] ); + size = FIELD_OFFSET( EMREXTESCAPE, EscData[input_size] ); size = (size + 3) & ~3; if (!(emr = HeapAlloc( GetProcessHeap(), 0, size ))) return 0; emr->emr.iType = EMR_EXTESCAPE; emr->emr.nSize = size; - emr->escape = escape; - emr->size = input_size; - memcpy(emr->data, input, input_size); + emr->iEscape = escape; + emr->cbEscData = input_size; + memcpy(emr->EscData, input, input_size); emfdc_record( get_dc_emf( dc_attr ), &emr->emr ); HeapFree( GetProcessHeap(), 0, emr ); if (output_size && output) return 0; diff --git a/include/wingdi.h b/include/wingdi.h index ff0bbb6fc85..ef1b3e310aa 100644 --- a/include/wingdi.h +++ b/include/wingdi.h @@ -2341,6 +2341,14 @@ typedef struct { EXTLOGPEN elp; } EMREXTCREATEPEN, *PEMREXTCREATEPEN; +typedef struct tagEMREXTESCAPE +{ + EMR emr; + INT iEscape; + INT cbEscData; + BYTE EscData[1]; +} EMREXTESCAPE, *PEMREXTESCAPE, EMRDRAWESCAPE, *PEMRDRAWESCAPE; + typedef struct { EMR emr; POINTL ptlStart; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/wineps.drv/printproc.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/dlls/wineps.drv/printproc.c b/dlls/wineps.drv/printproc.c index 46ebe0b8e04..b957a0d3bbd 100644 --- a/dlls/wineps.drv/printproc.c +++ b/dlls/wineps.drv/printproc.c @@ -2773,15 +2773,9 @@ static int WINAPI hmf_proc(HDC hdc, HANDLETABLE *htable, } case EMR_EXTESCAPE: { - const struct EMREXTESCAPE - { - EMR emr; - DWORD escape; - DWORD size; - BYTE data[1]; - } *p = (const struct EMREXTESCAPE *)rec; + const EMREXTESCAPE *p = (const EMREXTESCAPE *)rec; - PSDRV_ExtEscape(data->ctx, p->escape, p->size, p->data, 0, NULL); + PSDRV_ExtEscape(data->ctx, p->iEscape, p->cbEscData, p->EscData, 0, NULL); return 1; } case EMR_GRADIENTFILL: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/gdi32/emfdc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index 5fc0109f78f..63110f57eeb 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -1199,8 +1199,7 @@ INT EMFDC_ExtEscape( DC_ATTR *dc_attr, INT escape, INT input_size, const char *i if (escape == QUERYESCSUPPORT) return 0; - size = FIELD_OFFSET( EMREXTESCAPE, EscData[input_size] ); - size = (size + 3) & ~3; + size = aligned_size(FIELD_OFFSET( EMREXTESCAPE, EscData[input_size] )); if (!(emr = HeapAlloc( GetProcessHeap(), 0, size ))) return 0; emr->emr.iType = EMR_EXTESCAPE; @@ -1208,6 +1207,7 @@ INT EMFDC_ExtEscape( DC_ATTR *dc_attr, INT escape, INT input_size, const char *i emr->iEscape = escape; emr->cbEscData = input_size; memcpy(emr->EscData, input, input_size); + pad_record(emr->EscData, input_size); emfdc_record( get_dc_emf( dc_attr ), &emr->emr ); HeapFree( GetProcessHeap(), 0, emr ); if (output_size && output) return 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
Piotr Caban (@piotr) commented about dlls/gdi32/emfdc.c:
INT EMFDC_ExtEscape( DC_ATTR *dc_attr, INT escape, INT input_size, const char *input, INT output_size, char *output) { - struct EMREXTESCAPE - { - EMR emr; - DWORD escape; - DWORD size; - BYTE data[1]; - } *emr; Could you please also get rid of custom EMREXTESCAPE definition in dlls/wineps.drv/printproc.c?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4519#note_53885
On Mon Nov 27 10:07:41 2023 +0000, Piotr Caban wrote:
Could you please also get rid of custom EMREXTESCAPE definition in dlls/wineps.drv/printproc.c? I did, commit 8 does that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4519#note_53899
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4519
participants (4)
-
Huw Davies (＠huw) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Piotr Caban (＠piotr)