[PATCH 0/2] MR4964: winebus.sys: Fix Unity crashing with certain input devices connected.

[Arek Hiler (＠ivyl)]

Affected games: Phasmophobia, Lethal Company.

The games do not use joystick input but Unity Input subsystem is initialized and that's enough to cause stack overflow when it tries to parse 8 bit hat switches. Most joysticks and some mice have hats.

I own 2 Thrustmaster devices and this fixes the problem with them plugged in. I've also seen [report descriptors shared by Proton users](https://github.com/ValveSoftware/Proton/issues/7284) that reported the crash. The real devices always use 4 bits only.

This MR was tested using a few dinput test programs (upstream Wine) and Elite Dangerous (change was applied on top of the Proton tree). Singular hat works. I don't have anything with more than that though.

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

[PATCH 1/2] winebus.sys: Fix units used for hat switches.

[Arkadiusz Hiler]

From: Arkadiusz Hiler <ahiler(a)codeweavers.com>

The 0xe nibble value is reserved and makes hid-decode from hid-tools
crash while tryign to parse the descriptor.

0x0 is the correct way of expressing no special units are used.
---
 dlls/winebus.sys/hid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/winebus.sys/hid.c b/dlls/winebus.sys/hid.c
index 6d2b484ccca..733f9a51f96 100644
--- a/dlls/winebus.sys/hid.c
+++ b/dlls/winebus.sys/hid.c
@@ -233,7 +233,7 @@ BOOL hid_device_add_hatswitch(struct unix_device *iface, INT count)
         LOGICAL_MAXIMUM(1, 8),
         REPORT_SIZE(1, 8),
         REPORT_COUNT(4, count),
-        UNIT(1, 0x0e /* none */),
+        UNIT(1, 0x0), /* None */
         INPUT(1, Data|Var|Abs|Null),
     };
 
-- 
GitLab


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

[PATCH 2/2] winebus.sys: Use 4 bits for hat switches.

[Arkadiusz Hiler]

From: Arkadiusz Hiler <ahiler(a)codeweavers.com>

Declaring them as 8 bits crashes game due to how Unity's native hid
support parses the data.
---
 dlls/winebus.sys/hid.c | 66 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 54 insertions(+), 12 deletions(-)

diff --git a/dlls/winebus.sys/hid.c b/dlls/winebus.sys/hid.c
index 733f9a51f96..7e661953639 100644
--- a/dlls/winebus.sys/hid.c
+++ b/dlls/winebus.sys/hid.c
@@ -215,7 +215,7 @@ static BOOL hid_device_add_hatswitch_count(struct unix_device *iface, BYTE count
     {
         if (!iface->hid_device_state.hatswitch_count) iface->hid_device_state.hatswitch_start = offset;
         iface->hid_device_state.hatswitch_count += count;
-        iface->hid_device_state.bit_size += 8 * count;
+        iface->hid_device_state.bit_size += 4 * count;
         return TRUE;
     }
 
@@ -224,6 +224,7 @@ static BOOL hid_device_add_hatswitch_count(struct unix_device *iface, BYTE count
 
 BOOL hid_device_add_hatswitch(struct unix_device *iface, INT count)
 {
+    BOOL ret;
     struct hid_report_descriptor *desc = &iface->hid_report_descriptor;
     const BYTE template[] =
     {
@@ -231,16 +232,27 @@ BOOL hid_device_add_hatswitch(struct unix_device *iface, INT count)
         USAGE(1, HID_USAGE_GENERIC_HATSWITCH),
         LOGICAL_MINIMUM(1, 1),
         LOGICAL_MAXIMUM(1, 8),
-        REPORT_SIZE(1, 8),
+        REPORT_SIZE(1, 4),
         REPORT_COUNT(4, count),
         UNIT(1, 0x0), /* None */
         INPUT(1, Data|Var|Abs|Null),
     };
+    const BYTE template_pad[] =
+    {
+        INPUT(1, Cnst|Ary|Abs),
+    };
 
     if (!hid_device_add_hatswitch_count(iface, count))
         return FALSE;
 
-    return hid_report_descriptor_append(desc, template, sizeof(template));
+    ret = hid_report_descriptor_append(desc, template, sizeof(template));
+
+    if (ret && count % 2) {
+        ret = hid_report_descriptor_append(desc, template_pad, sizeof(template_pad));
+        iface->hid_device_state.bit_size += 4;
+    }
+
+    return ret;
 }
 
 static BOOL hid_device_add_axis_count(struct unix_device *iface, BOOL rel, BYTE count,
@@ -1431,36 +1443,66 @@ static void hatswitch_compose(LONG x, LONG y, BYTE *value)
     else if (x < 0 && y < 0) *value = 8;
 }
 
+static BYTE hatswitch_get(BYTE report_byte, LONG which)
+{
+    if (which)
+        return report_byte >> 4;
+    else
+        return report_byte & 0x0f;
+}
+
+static void hatswitch_set(BYTE *report_byte, BYTE which, BYTE new_value)
+{
+    if (which)
+    {
+        new_value <<= 4;
+        *report_byte &= 0xf;
+    }
+    else
+        *report_byte &= 0xf0;
+
+    *report_byte |= new_value;
+}
+
 BOOL hid_device_set_hatswitch_x(struct unix_device *iface, ULONG index, LONG new_x)
 {
     struct hid_device_state *state = &iface->hid_device_state;
-    ULONG offset = state->hatswitch_start + index;
+    ULONG offset = state->hatswitch_start + index / 2;
+    BYTE value;
     LONG x, y;
     if (index > state->hatswitch_count) return FALSE;
-    hatswitch_decompose(state->report_buf[offset], &x, &y);
-    hatswitch_compose(new_x, y, &state->report_buf[offset]);
+    value = hatswitch_get(state->report_buf[offset], index % 2);
+    hatswitch_decompose(value, &x, &y);
+    hatswitch_compose(new_x, y, &value);
+    hatswitch_set(&state->report_buf[offset], index % 2, value);
     return TRUE;
 }
 
 BOOL hid_device_set_hatswitch_y(struct unix_device *iface, ULONG index, LONG new_y)
 {
     struct hid_device_state *state = &iface->hid_device_state;
-    ULONG offset = state->hatswitch_start + index;
+    ULONG offset = state->hatswitch_start + index / 2;
+    BYTE value;
     LONG x, y;
     if (index > state->hatswitch_count) return FALSE;
-    hatswitch_decompose(state->report_buf[offset], &x, &y);
-    hatswitch_compose(x, new_y, &state->report_buf[offset]);
+    value = hatswitch_get(state->report_buf[offset], index % 2);
+    hatswitch_decompose(value, &x, &y);
+    hatswitch_compose(x, new_y, &value);
+    hatswitch_set(&state->report_buf[offset], index % 2, value);
     return TRUE;
 }
 
 BOOL hid_device_move_hatswitch(struct unix_device *iface, ULONG index, LONG x, LONG y)
 {
     struct hid_device_state *state = &iface->hid_device_state;
-    ULONG offset = state->hatswitch_start + index;
+    ULONG offset = state->hatswitch_start + index / 2;
+    BYTE value;
     LONG old_x, old_y;
     if (index > state->hatswitch_count) return FALSE;
-    hatswitch_decompose(state->report_buf[offset], &old_x, &old_y);
-    hatswitch_compose(old_x + x, old_y + y, &state->report_buf[offset]);
+    value = hatswitch_get(state->report_buf[offset], index % 2);
+    hatswitch_decompose(value, &old_x, &old_y);
+    hatswitch_compose(old_x + x, old_y + y, &value);
+    hatswitch_set(&state->report_buf[offset], index % 2, value);
     return TRUE;
 }
 
-- 
GitLab

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

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/hid.c:
> {
>          if (!iface->hid_device_state.hatswitch_count) iface->hid_device_state.hatswitch_start = offset;
>          iface->hid_device_state.hatswitch_count += count;
> -        iface->hid_device_state.bit_size += 8 * count;
> +        iface->hid_device_state.bit_size += 4 * count;
>          return TRUE;
>      }
>
> return FALSE;

You would need to handle the padding here.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4964#note_59481

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/hid.c:
> USAGE(1, HID_USAGE_GENERIC_HATSWITCH),
>          LOGICAL_MINIMUM(1, 1),
>          LOGICAL_MAXIMUM(1, 8),
> -        REPORT_SIZE(1, 8),
> +        REPORT_SIZE(1, 4),
>          REPORT_COUNT(4, count),
>          UNIT(1, 0x0), /* None */
>          INPUT(1, Data|Var|Abs|Null),
>      };
> +    const BYTE template_pad[] =
> +    {
> +        INPUT(1, Cnst|Ary|Abs),

This probably also needs REPORT_SIZE / REPORT_COUNT as otherwise the previous values are used (which, if count wasn't 1, will add unnecessary padding), probably better to always specify both like it's done for the button padding.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4964#note_59482

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/hid.c:
> +    {
> +        INPUT(1, Cnst|Ary|Abs),
> +    };
>
> if (!hid_device_add_hatswitch_count(iface, count))
>          return FALSE;
>
> -    return hid_report_descriptor_append(desc, template, sizeof(template));
> +    ret = hid_report_descriptor_append(desc, template, sizeof(template));
> +
> +    if (ret && count % 2) {
> +        ret = hid_report_descriptor_append(desc, template_pad, sizeof(template_pad));
> +        iface->hid_device_state.bit_size += 4;
> +    }
> +
> +    return ret;
> Lets keep the style consistent.

```suggestion:-7+0
    if (!hid_report_descriptor_append(desc, template, sizeof(template)))
        return FALSE;

    if ((count % 2) && !hid_report_descriptor_append(desc, template_pad, sizeof(template_pad)))
        return FALSE;

    return TRUE;
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4964#note_59483

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/hid.c:
> */
>  static void hatswitch_decompose(BYTE value, LONG *x, LONG *y)
>  {
>      *x = *y = 0;
> What about passing the index here?

```suggestion:-2+0
static void hatswitch_decompose(BYTE value, UINT index, LONG *x, LONG *y)
{
    value = (index % 2) ? (value >> 4) : (value & 0x0f);
    *x = *y = 0;
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4964#note_59484

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/hid.c:
> else if (x < 0 && y < 0) *value = 8;
>  }

And here?

```suggestion:-11+0
static void hatswitch_compose(LONG x, LONG y, UINT index, BYTE *value)
{
    BYTE mask = (index % 2) ? 0x0f : 0xf0, shift = (index % 2) ? 4 : 0;
    if (x == 0 && y == 0) *value = (*value & mask) | (0 << shift);
    else if (x == 0 && y < 0) *value = (*value & mask) | (1 << shift);
    else if (x > 0 && y < 0) *value = (*value & mask) | (2 << shift);
    else if (x > 0 && y == 0) *value = (*value & mask) | (3 << shift);
    else if (x > 0 && y > 0) *value = (*value & mask) | (4 << shift);
    else if (x == 0 && y > 0) *value = (*value & mask) | (5 << shift);
    else if (x < 0 && y > 0) *value = (*value & mask) | (6 << shift);
    else if (x < 0 && y == 0) *value = (*value & mask) | (7 << shift);
    else if (x < 0 && y < 0) *value = (*value & mask) | (8 << shift);
}
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4964#note_59485