[PATCH 0/3] input: force feedback for trigger rumble motors

[PATCH 0/3] input: force feedback for trigger rumble motors

[Guillaume Casal]

Some gamepads carry rumble motors behind their triggers, in addition to
the usual two in the grips: Xbox controllers call them impulse triggers,
and handhelds such as the ASUS ROG Xbox Ally X have them too. The force
feedback API cannot reach those motors, because struct ff_rumble_effect
only carries a strong and a weak magnitude.

The first two patches are Daniel Bomar's, posted in April 2022 and never
merged:

  https://lore.kernel.org/lkml/20220410220449.5071-1-dbdaniel42@gmail.com/

They add trigger_left and trigger_right to struct ff_rumble_effect and
carry them through ff-memless. I have rebased them onto v7.2-rc5 with no
functional change and kept his authorship.

The third patch is new and addresses what I believe was the main gap in
that series: there was no way for userspace to discover whether a device
has trigger motors at all. It adds an FF_TRIGGER_RUMBLE capability bit
that drivers advertise and applications test in the EVIOCGBIT(EV_FF)
bitmap.

On ABI: struct ff_rumble_effect lives in a union sized by the larger
struct ff_periodic_effect, so the two new members add no size. On an
x86_64 build, struct ff_effect stays at 48 bytes and the union at 32,
of which the extended rumble struct uses 8.

Testing. I ran this on an ASUS ROG Xbox Ally X (USB 0b05:1b4c) under
SteamOS, kernel 6.16.12, where the patches apply unchanged. A memless
test device advertising the capability shows bit 0x62 in its
EVIOCGBIT(EV_FF) bitmap, and the four magnitudes arrive at the driver
independently:

  triggers only : strong=    0 weak=    0 trigger_left=65535 trigger_right=65535
  grips only    : strong=65535 weak=65535 trigger_left=    0 trigger_right=    0
  left trigger  : strong=    0 weak=    0 trigger_left=65535 trigger_right=    0
  right trigger : strong=    0 weak=    0 trigger_left=    0 trigger_right=65535

Wired into that handheld's HID driver, this drives the four actuators
separately on real hardware: the triggers buzz while the grips stay
silent, confirmed both by feel and by capturing the USB traffic. That
driver is not in mainline, so its patch is not part of this series; I
will send it to its maintainers separately.

To be explicit about what was and was not tested: on v7.2-rc5 the series
applies cleanly, drivers/input/ff-memless.o builds, and checkpatch.pl
reports no warnings, but I have not booted that kernel. The runtime
testing described above was done on 6.16.12, where the affected code is
identical.

Daniel Bomar (2):
  input: uapi: Add trigger_left and trigger_right to ff_rumble_effect struct
  input: ff-memless: Add trigger left/right in ml_combine_effects

Guillaume Casal (1):
  input: Add FF_TRIGGER_RUMBLE capability bit

 drivers/input/ff-memless.c | 8 +++++++-
 include/uapi/linux/input.h | 5 +++++
 2 files changed, 12 insertions(+), 1 deletion(-)

[PATCH 1/3] input: uapi: Add trigger_left and trigger_right to ff_rumble_effect struct

[Guillaume Casal]

From: Daniel Bomar <dbdaniel42@gmail.com>

This adds two variables to control the trigger motors to the struct that
is passed in from userspace. This is ABI compatible because it does not
change the total size of the union that contains this struct: the union
is sized by struct ff_periodic_effect, which is larger.

Originally posted in 2022 and never merged:
https://lore.kernel.org/lkml/20220410220449.5071-1-dbdaniel42@gmail.com/

Rebased onto v7.2-rc5 with no functional change; the kernel-doc entries for
the two new members are the only addition.

Signed-off-by: Daniel Bomar <dbdaniel42@gmail.com>
Signed-off-by: Guillaume Casal <guillaume.casal.42@gmail.com>
---
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -420,6 +420,8 @@
  * struct ff_rumble_effect - defines parameters of a periodic force-feedback effect
  * @strong_magnitude: magnitude of the heavy motor
  * @weak_magnitude: magnitude of the light one
+ * @trigger_left: magnitude of the motor behind the left trigger
+ * @trigger_right: magnitude of the motor behind the right trigger
  *
  * Some rumble pads have two motors of different weight. Strong_magnitude
  * represents the magnitude of the vibration generated by the heavy one.
@@ -427,6 +429,8 @@
 struct ff_rumble_effect {
 	__u16 strong_magnitude;
 	__u16 weak_magnitude;
+	__u16 trigger_left;
+	__u16 trigger_right;
 };
 
 /**

[PATCH 2/3] input: ff-memless: Add trigger left/right in ml_combine_effects

[Guillaume Casal]

From: Daniel Bomar <dbdaniel42@gmail.com>

Combine the new trigger magnitudes alongside the existing strong and weak
magnitudes, so that drivers using input_ff_create_memless() receive them
the same way.

Originally posted in 2022 and never merged:
https://lore.kernel.org/lkml/20220410220449.5071-1-dbdaniel42@gmail.com/

Rebased onto v7.2-rc5 with no functional change.

Signed-off-by: Daniel Bomar <dbdaniel42@gmail.com>
Signed-off-by: Guillaume Casal <guillaume.casal.42@gmail.com>
---
diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -243,7 +243,7 @@
 			       int gain)
 {
 	struct ff_effect *new = state->effect;
-	unsigned int strong, weak, i;
+	unsigned int strong, weak, trigger_left, trigger_right, i;
 	int x, y;
 	s16 level;
 
@@ -269,6 +269,8 @@
 	case FF_RUMBLE:
 		strong = (u32)new->u.rumble.strong_magnitude * gain / 0xffff;
 		weak = (u32)new->u.rumble.weak_magnitude * gain / 0xffff;
+		trigger_left = (u32)new->u.rumble.trigger_left * gain / 0xffff;
+		trigger_right = (u32)new->u.rumble.trigger_right * gain / 0xffff;
 
 		if (effect->u.rumble.strong_magnitude + strong)
 			effect->direction = ml_calculate_direction(
@@ -287,6 +289,10 @@
 			    0xffffU);
 		effect->u.rumble.weak_magnitude =
 			min(weak + effect->u.rumble.weak_magnitude, 0xffffU);
+		effect->u.rumble.trigger_left =
+			min(trigger_left + effect->u.rumble.trigger_left, 0xffffU);
+		effect->u.rumble.trigger_right =
+			min(trigger_right + effect->u.rumble.trigger_right, 0xffffU);
 		break;
 
 	case FF_PERIODIC:

[PATCH 3/3] input: Add FF_TRIGGER_RUMBLE capability bit

[Guillaume Casal]

The preceding patches let userspace ask for rumble in the motors located
under the triggers, but give it no way to find out whether the device has
such motors at all. Without that, an application can only set the new
magnitudes and hope, and it cannot tell a device that ignores them from
one that has no trigger motors.

Add FF_TRIGGER_RUMBLE as a capability bit. Drivers that fill in the
trigger magnitudes advertise it with

	input_set_capability(dev, EV_FF, FF_TRIGGER_RUMBLE);

and userspace tests for it in the EVIOCGBIT(EV_FF) bitmap, exactly as it
already tests for FF_RUMBLE.

It is deliberately not an effect type. The magnitudes keep travelling
inside struct ff_rumble_effect and FF_RUMBLE remains the transport, so
FF_EFFECT_MIN and FF_EFFECT_MAX are untouched. Placing it at 0x62, above
FF_AUTOCENTER, keeps it out of the effect id space whose upper bound
FF_MAX_EFFECTS already documents. Unlike its neighbours FF_GAIN and
FF_AUTOCENTER it is read-only: it reports what the hardware has rather
than something userspace sets, and it is never used as an effect id.

Signed-off-by: Guillaume Casal <guillaume.casal.42@gmail.com>
---
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -528,6 +528,7 @@
 
 #define FF_GAIN		0x60
 #define FF_AUTOCENTER	0x61
+#define FF_TRIGGER_RUMBLE	0x62
 
 /*
  * ff->playback(effect_id = FF_GAIN) is the first effect_id to

Re: [PATCH 1/3] input: uapi: Add trigger_left and trigger_right to ff_rumble_effect struct

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Adding `trigger_left` and `trigger_right` to `struct ff_rumble_effect` within a union breaks backward compatibility and causes random device actuation for legacy userspace applications.

Pre-existing issues:
- [High] 4 bytes of uninitialized kernel stack memory are leaked to userspace via the `uinput` subsystem when a 32-bit compat application sends an `EVIOCSFF` ioctl.
--

--- Patch [1]: [PATCH 1/3] input: uapi: Add trigger_left and trigger_right to ff_rumble_effect struct ---
commit 47b93f39a2f9b455e85b8381fa40a96ca5aaac36
Author: Daniel Bomar <dbdaniel42@gmail.com>

input: uapi: Add trigger_left and trigger_right to ff_rumble_effect struct

This adds two variables to control the trigger motors to the struct that
is passed in from userspace. It reuses existing union padding to maintain
the total size of struct ff_effect.

Link: https://lore.kernel.org/lkml/20220410220449.5071-1-dbdaniel42@gmail.com/

> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 6aa703fcfcfb7..3d62603a387b9 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
[ ... ]
> @@ -427,6 +429,8 @@ struct ff_periodic_effect {
>  struct ff_rumble_effect {
>  	__u16 strong_magnitude;
>  	__u16 weak_magnitude;
> +	__u16 trigger_left;
> +	__u16 trigger_right;
>  };

[Severity: High]
Does this break backward compatibility for legacy userspace applications?

Because struct ff_rumble_effect resides within a union inside struct ff_effect,
extending its size from 4 to 8 bytes does not change the overall size of
struct ff_effect, and the EVIOCSFF ioctl number remains identical.

Legacy userspace applications typically allocate struct ff_effect on the stack
without zero-initializing the entire union padding. When calling EVIOCSFF for
a rumble effect, they leave uninitialized stack memory in the trailing bytes
that now correspond to trigger_left and trigger_right.

Could an updated driver read these uninitialized fields and actuate the trigger
motors based on random stack garbage?

[Severity: High]
This isn't a bug introduced by this patch, but does the input subsystem leak
uninitialized kernel stack memory to userspace during 32-bit compat ioctls?

In drivers/input/evdev.c:evdev_do_ioctl(), an uninitialized 48-byte effect
variable is declared on the stack:

	struct ff_effect effect;

When a 32-bit compat application calls EVIOCSFF, input_ff_effect_from_user() in
drivers/input/input-compat.c copies only 44 bytes from userspace:

		if (copy_from_user(compat_effect, buffer,
				   sizeof(struct ff_effect_compat)))
			return -EFAULT;

This leaves 4 bytes of the effect structure untouched, containing uninitialized
kernel stack data.

If this effect is uploaded to a virtual uinput device, uinput_dev_upload_effect()
stores a pointer to this stack variable. When a 64-bit uinput daemon process
calls the UI_BEGIN_FF_UPLOAD ioctl in
drivers/input/misc/uinput.c:uinput_ioctl_handler():

		retval = uinput_ff_upload_to_user(p, &ff_up);

it copies the entire 48-byte struct ff_effect back to userspace, reliably
disclosing 4 bytes of uninitialized kernel stack memory per ioctl call. Could
this be exploited to read kernel memory layout?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730095317.767418-1-guillaume.casal.42@gmail.com?part=1

Re: [PATCH 2/3] input: ff-memless: Add trigger left/right in ml_combine_effects

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Extending `struct ff_rumble_effect` breaks backward compatibility for legacy userspace applications by interpreting uninitialized union padding as valid rumble magnitudes.
--

--- Patch [2]: [PATCH 2/3] input: ff-memless: Add trigger left/right in ml_combine_effects ---
commit 3748260413928654275c7c17fc4ca45fccc0a9ee
Author: Daniel Bomar <dbdaniel42@gmail.com>

input: ff-memless: Add trigger left/right in ml_combine_effects

Combine the new trigger magnitudes alongside the existing strong and weak
magnitudes, so that drivers using input_ff_create_memless() receive them
the same way.

> diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
> index d1fefd1dfc0de..f6e7f5e93c687 100644
> --- a/drivers/input/ff-memless.c
> +++ b/drivers/input/ff-memless.c

[ ... ]

> @@ -269,6 +269,8 @@ static void ml_combine_effects(struct ff_effect *effect,
>  	case FF_RUMBLE:
>  		strong = (u32)new->u.rumble.strong_magnitude * gain / 0xffff;
>  		weak = (u32)new->u.rumble.weak_magnitude * gain / 0xffff;
> +		trigger_left = (u32)new->u.rumble.trigger_left * gain / 0xffff;
> +		trigger_right = (u32)new->u.rumble.trigger_right * gain / 0xffff;

[Severity: Critical]
Does interpreting these new fields break backward compatibility for legacy
userspace applications?

Because struct ff_rumble_effect is part of a union dominated by larger
structs in struct ff_effect, extending its size did not change the total size
of struct ff_effect.

Legacy applications allocating struct ff_effect on the stack, setting
type = FF_RUMBLE, and initializing only strong_magnitude and weak_magnitude
could unknowingly pass uninitialized bytes to the kernel.

Since this code actively scales and applies trigger_left and trigger_right,
wouldn't those uninitialized bytes from the stack result in random trigger
vibrations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730095317.767418-1-guillaume.casal.42@gmail.com?part=2

Re: [PATCH 3/3] input: Add FF_TRIGGER_RUMBLE capability bit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Extending `struct ff_rumble_effect` within the `ff_effect` union breaks backward compatibility for older userspace applications that use `EVIOCSFF` by exposing uninitialized memory as trigger magnitudes.
- [Low] Missing `FF_TRIGGER_RUMBLE` string representation in the `force` array in `hid-debug.c`.
- [Low] `FF_TRIGGER_RUMBLE` is not documented in the Force Feedback API documentation.
--

--- Patch [3]: [PATCH 3/3] input: Add FF_TRIGGER_RUMBLE capability bit ---
commit 454627a6a6ad091c91ae3a0477831268ea8fe178
Author: Guillaume Casal <guillaume.casal.42@gmail.com>

input: Add FF_TRIGGER_RUMBLE capability bit

This commit adds the FF_TRIGGER_RUMBLE capability bit to allow userspace to
discover if a device has motors under the triggers. The capability is added
as a read-only bit at 0x62 to avoid conflicting with the effect id space.

> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 3d62603a387b9..677e9505a4aa3 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -528,6 +528,7 @@ struct ff_effect {
>  
>  #define FF_GAIN		0x60
>  #define FF_AUTOCENTER	0x61
> +#define FF_TRIGGER_RUMBLE	0x62

[Severity: High]
Does extending struct ff_rumble_effect (as mentioned in the commit message)
cause a regression for older userspace applications using EVIOCSFF?

Legacy applications frequently allocate struct ff_effect on the stack,
initialize only known fields like strong_magnitude and weak_magnitude, and
pass the struct to the EVIOCSFF ioctl.

In drivers/input/evdev.c:evdev_do_ioctl():

    case EVIOC_MASK_SIZE(EVIOCSFF):
        if (input_ff_effect_from_user(p, size, &effect))
            return -EFAULT;

This copies the entire union size from userspace. If uninitialized stack
garbage is copied, it could be interpreted as trigger_left and trigger_right.

Then in drivers/input/ff-memless.c:ml_combine_effects():

    trigger_left = (u32)new->u.rumble.trigger_left * gain / 0xffff;
    trigger_right = (u32)new->u.rumble.trigger_right * gain / 0xffff;

These uninitialized fields are actively read and processed into the final
effect playback state. Could this result in random, unintended vibration of
the trigger motors?

[Severity: Low]
Is the FF_TRIGGER_RUMBLE string representation missing in the force array?

In drivers/hid/hid-debug.c:force:

    static const char *force[FF_CNT] = {
        [FF_RUMBLE] = "FF_RUMBLE",

Without adding [FF_TRIGGER_RUMBLE] = "FF_TRIGGER_RUMBLE" here,
names[EV_FF][0x62] evaluates to NULL. This causes hid_resolv_event() to fall
back to printing the raw hex value ('0062') instead of 'FF_TRIGGER_RUMBLE'
in debugfs.

[Severity: Low]
Is FF_TRIGGER_RUMBLE missing from the Force Feedback API documentation?

Documentation/input/ff.rst:Querying device capabilities lists supported
EV_FF capability bits:

    - FF_GAIN    gain is adjustable
    - FF_AUTOCENTER  autocenter is adjustable

Since this commit introduces a new capability, should it be documented here
so userspace developers know how to discover trigger rumble support?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730095317.767418-1-guillaume.casal.42@gmail.com?part=3