HID: pidff: Do not send effect envelope if it's empty

Envelope struct is always initialized, but the envelope itself is
optional as described in USB PID Device class definition 1.0.

5.1.1.1 Type Specific Block Offsets
...
4) Effects that do not use Condition Blocks use 1 Parameter Block and
an *optional* Envelope Block.

Sending out "empty" envelope breaks force feedback on some devices with
games that use SINE effect + offset to emulate constant force effect, as
well as generally breaking Constant/Periodic effects. One of the affected
brands is Moza Racing.

This change prevents the envelope from being sent if it contains all
0 values while keeping the old behavior of only sending it, if it differs
from the old one.

Changes in v6:
- Simplify the checks to make them clearer
- Fix possible null pointer dereference while calling
  pidff_needs_set_envelope

Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Reviewed-by: Michał Kopeć <michal@nozomi.space>
Reviewed-by: Paul Dino Jones <paul@spacefreak18.xyz>
Tested-by: Paul Dino Jones <paul@spacefreak18.xyz>
Tested-by: Cristóferson Bueno <cbueno81@gmail.com>
Tested-by: Pablo Cisneros <patchkez@protonmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>

diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 5fe4422bb5bad0c902d508fb2faa3fccc5c4bf05..a01c1b2ab2f4c8dbb4ab9c8ae68b7169ee2fbe65 100644 (file)
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -262,10 +262,22 @@ static void pidff_set_envelope_report(struct pidff_device *pidff,
 static int pidff_needs_set_envelope(struct ff_envelope *envelope,
                                    struct ff_envelope *old)
 {
-       return envelope->attack_level != old->attack_level ||
-              envelope->fade_level != old->fade_level ||
+       bool needs_new_envelope;
+       needs_new_envelope = envelope->attack_level  != 0 ||
+                            envelope->fade_level    != 0 ||
+                            envelope->attack_length != 0 ||
+                            envelope->fade_length   != 0;
+
+       if (!needs_new_envelope)
+               return false;
+
+       if (!old)
+               return needs_new_envelope;
+
+       return envelope->attack_level  != old->attack_level  ||
+              envelope->fade_level    != old->fade_level    ||
               envelope->attack_length != old->attack_length ||
-              envelope->fade_length != old->fade_length;
+              envelope->fade_length   != old->fade_length;
 }
 
 /*
@@ -580,11 +592,9 @@ static int pidff_upload_effect(struct input_dev *dev, struct ff_effect *effect,
                        pidff_set_effect_report(pidff, effect);
                if (!old || pidff_needs_set_constant(effect, old))
                        pidff_set_constant_force_report(pidff, effect);
-               if (!old ||
-                   pidff_needs_set_envelope(&effect->u.constant.envelope,
-                                       &old->u.constant.envelope))
-                       pidff_set_envelope_report(pidff,
-                                       &effect->u.constant.envelope);
+               if (pidff_needs_set_envelope(&effect->u.constant.envelope,
+                                       old ? &old->u.constant.envelope : NULL))
+                       pidff_set_envelope_report(pidff, &effect->u.constant.envelope);
                break;
 
        case FF_PERIODIC:
@@ -619,11 +629,9 @@ static int pidff_upload_effect(struct input_dev *dev, struct ff_effect *effect,
                        pidff_set_effect_report(pidff, effect);
                if (!old || pidff_needs_set_periodic(effect, old))
                        pidff_set_periodic_report(pidff, effect);
-               if (!old ||
-                   pidff_needs_set_envelope(&effect->u.periodic.envelope,
-                                       &old->u.periodic.envelope))
-                       pidff_set_envelope_report(pidff,
-                                       &effect->u.periodic.envelope);
+               if (pidff_needs_set_envelope(&effect->u.periodic.envelope,
+                                       old ? &old->u.periodic.envelope : NULL))
+                       pidff_set_envelope_report(pidff, &effect->u.periodic.envelope);
                break;
 
        case FF_RAMP:
@@ -637,11 +645,9 @@ static int pidff_upload_effect(struct input_dev *dev, struct ff_effect *effect,
                        pidff_set_effect_report(pidff, effect);
                if (!old || pidff_needs_set_ramp(effect, old))
                        pidff_set_ramp_force_report(pidff, effect);
-               if (!old ||
-                   pidff_needs_set_envelope(&effect->u.ramp.envelope,
-                                       &old->u.ramp.envelope))
-                       pidff_set_envelope_report(pidff,
-                                       &effect->u.ramp.envelope);
+               if (pidff_needs_set_envelope(&effect->u.ramp.envelope,
+                                       old ? &old->u.ramp.envelope : NULL))
+                       pidff_set_envelope_report(pidff, &effect->u.ramp.envelope);
                break;
 
        case FF_SPRING: