[PATCH v3 0/1] MR4930: wineps.drv: Avoid invalid unaligned accesses.

[Martin Storsjö]

While armv7 generally tolerates unaligned loads/stores in most
cases, the compiler is free to use the ldrd/strd instructions,
for loading/storing two consecutive 32 bit registers, and this
requires the destination to be aligned to a 4 byte boundary.

When packing a number of variable length structures, make sure
that each actual struct gets aligned at the right address
boundary.

This fixes crashes in DllMain of wineps.drv, when built for
armv7, since 351e58dc2d0aafe19294cbeaed9cd30ae965d591.

Signed-off-by: Martin Storsjö <martin(a)martin.st>

--
  v3: wineps.drv: Avoid invalid unaligned accesses.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4930

[PATCH v3 1/1] wineps.drv: Avoid invalid unaligned accesses.

[Martin Storsjö]

From: Martin Storsjö <martin(a)martin.st>

While armv7 generally tolerates unaligned loads/stores in most
cases, the compiler is free to use the ldrd/strd instructions,
for loading/storing two consecutive 32 bit registers, and this
requires the destination to be aligned to a 4 byte boundary.

When packing a number of variable length structures, make sure
that each actual struct gets aligned at the right address
boundary.

This fixes crashes in DllMain of wineps.drv, when built for
armv7, since 351e58dc2d0aafe19294cbeaed9cd30ae965d591.

Signed-off-by: Martin Storsjö <martin(a)martin.st>
---
 dlls/wineps.drv/init.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/dlls/wineps.drv/init.c b/dlls/wineps.drv/init.c
index 8537ba287d1..10e8e77e52d 100644
--- a/dlls/wineps.drv/init.c
+++ b/dlls/wineps.drv/init.c
@@ -133,6 +133,10 @@ static BOOL import_ntf_from_reg(void)
     return ret;
 }
 
+#define DWORD_ALIGN(x) (((x) + 3) & ~3)
+#define ntf_strsize(str) DWORD_ALIGN(strlen(str) + 1)
+#define ntf_wcssize(str) DWORD_ALIGN((wcslen(str) + 1) * sizeof(WCHAR))
+
 static BOOL convert_afm_to_ntf(void)
 {
     int i, count, size, off, metrics_size;
@@ -176,8 +180,8 @@ static BOOL convert_afm_to_ntf(void)
 
             list = (void *)(data + header->glyph_set_off + sizeof(*list) * count);
             list->name_off = off + sizeof(*glyph_set);
-            list->size = sizeof(*glyph_set) + strlen(glyph_set_name) + 1 + sizeof(*cp) +
-                sizeof(short) * afmle->afm->NumofMetrics;
+            list->size = sizeof(*glyph_set) + ntf_strsize(glyph_set_name) + sizeof(*cp) +
+                DWORD_ALIGN(sizeof(short) * afmle->afm->NumofMetrics);
             list->off = off;
             size += list->size;
             new_data = realloc(data, size);
@@ -196,7 +200,7 @@ static BOOL convert_afm_to_ntf(void)
             glyph_set->name_off = sizeof(*glyph_set);
             glyph_set->glyph_count = afmle->afm->NumofMetrics;
             glyph_set->cp_count = 1;
-            glyph_set->cp_off = glyph_set->name_off + strlen(glyph_set_name) + 1;
+            glyph_set->cp_off = glyph_set->name_off + ntf_strsize(glyph_set_name);
             glyph_set->glyph_set_off = glyph_set->cp_off + sizeof(*cp);
             strcpy(data + off + glyph_set->name_off, glyph_set_name);
             cp = (void *)(data + off + glyph_set->cp_off);
@@ -205,12 +209,11 @@ static BOOL convert_afm_to_ntf(void)
                 *(WCHAR*)(data + off + glyph_set->glyph_set_off + i * sizeof(short)) = afmle->afm->Metrics[i].UV;
             off = size;
 
-            metrics_size = sizeof(IFIMETRICS) +
-                (wcslen(afmle->afm->FamilyName) + 1) * sizeof(WCHAR);
+            metrics_size = sizeof(IFIMETRICS) + ntf_wcssize(afmle->afm->FamilyName);
             list = (void *)(data + header->font_mtx_off + sizeof(*list) * count);
             list->name_off = off + sizeof(*font_mtx);
-            list->size = sizeof(*font_mtx) + strlen(afmle->afm->FontName) + 1 +
-                strlen(glyph_set_name) + 1 + metrics_size +
+            list->size = sizeof(*font_mtx) + ntf_strsize(afmle->afm->FontName) +
+                ntf_strsize(glyph_set_name) + metrics_size +
                 (afmle->afm->IsFixedPitch ? 0 : sizeof(*width_range) * afmle->afm->NumofMetrics);
             list->off = off;
             size += list->size;
@@ -227,9 +230,9 @@ static BOOL convert_afm_to_ntf(void)
             font_mtx = (void *)(data + off);
             font_mtx->size = size - off;
             font_mtx->name_off = sizeof(*font_mtx);
-            font_mtx->glyph_set_name_off = font_mtx->name_off + strlen(afmle->afm->FontName) + 1;
+            font_mtx->glyph_set_name_off = font_mtx->name_off + ntf_strsize(afmle->afm->FontName);
             font_mtx->glyph_count = afmle->afm->NumofMetrics;
-            font_mtx->metrics_off = font_mtx->glyph_set_name_off + strlen(glyph_set_name) + 1;
+            font_mtx->metrics_off = font_mtx->glyph_set_name_off + ntf_strsize(glyph_set_name);
             font_mtx->width_count = afmle->afm->IsFixedPitch ? 0 : afmle->afm->NumofMetrics;
             font_mtx->width_off = font_mtx->metrics_off + metrics_size;
             font_mtx->def_width = afmle->afm->Metrics[0].WX;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/4930

[Martin Storsjö]

On Thu Jan 25 09:15:02 2024 +0000, Martin Storsjö wrote:
> changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/4930/diffs?diff_id=95811&start_sha=ae3ab8febeb3e35b4b2322e5f9316b6a779be432#f1b980cffb519eeeb891953aa5c30bfd49f0881d_137_136)
> Thanks, that sounds like a great update, making it even clearer. I updated the patch with this, and also used `DWORD_ALIGN` for `sizeof(short) * NumofMetrics`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4930#note_58955

[PATCH v3 0/1] MR4930: wineps.drv: Avoid invalid unaligned accesses. - approved

[Piotr Caban (＠piotr)]

This merge request was approved by Piotr Caban.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4930

[PATCH v3 0/1] MR4930: wineps.drv: Avoid invalid unaligned accesses. - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4930