Re: [PATCH] Input: synaptics - add transparent pass-through mode for TrackPoint

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Self-deadlock and Use-After-Free/Double-Free of the pass-through serio port during parent driver unbind or port unregistration.
- [Critical] Type confusion in synaptics_pt_open() leads to arbitrary memory corruption when toggling transparent mode.
- [High] Bypassed mutex synchronization in synaptics_pt_write() causes concurrent hardware command injection and data races.
--

--- Patch [1]: [PATCH] Input: synaptics - add transparent pass-through mode for TrackPoint ---

commit 1a99c1461a4c3bd59a184b3551c27088c1845e41
Author: laserology oss [off-list ref]

Input: synaptics - add transparent pass-through mode for TrackPoint

This patch reworks an earlier implementation to add a transparent pass-through
mode for Synaptics touchpads with a TrackPoint. It documents the interface and
modifies the protocol handling to support relaying the raw byte stream of the
pass-through guest.

[ ... ]

 static int synaptics_pt_write(struct serio *serio, u8 c)
 {
 	struct psmouse *parent = psmouse_from_serio(serio->parent);
+	struct synaptics_data *priv = parent->private;
 	u8 rate_param = SYN_PS_CLIENT_CMD; /* indicates that we want pass-through port */
 	int error;
 
+	if (READ_ONCE(priv->transparent_mode))
+		return parent->ps2dev.serio->write(parent->ps2dev.serio, c);
+

[Severity: High]
Does this bypass the parent's ps2_cmd_mutex? Since the child port uses its
own isolated ps2_cmd_mutex, returning parent->ps2dev.serio->write(...)
directly could allow concurrent hardware commands. This could interleave raw
PS/2 bytes if userspace triggers a command on the parent device (like
changing transparent_mode) while the child is writing.

 	error = ps2_sliced_command(&parent->ps2dev, c);
 	if (error)
 		return error;

[ ... ]

[Severity: High]
Does this create a data race if a driver like serio_raw is bound to the child
port? Unbinding serio_raw invokes synaptics_pt_stop() without holding the
global psmouse_mutex. Modifying hardware state and priv->transparent_mode
here could race with synaptics_set_transparent_mode() running concurrently
on the parent device.

[Severity: Critical]
Can this cast cause memory corruption? If userspace binds a non-psmouse
driver (like serio_raw) to the pass-through port, the driver data points to a
different structure (e.g., struct serio_raw). This caches a bogus pointer in
priv->pt_psmouse, which is later dereferenced in
synaptics_set_transparent_mode().

 
 	return 0;
 }

[ ... ]

+	/*
+	 * The touchpad is stopped for the duration of the mode switch, but the
+	 * pass-through guest keeps streaming. It may therefore be left with a
+	 * partially assembled packet from the previous mode. Drop it so the
+	 * stream restarts cleanly in the new mode and no stale bytes are
+	 * misparsed as pointer motion or spurious button clicks.
+	 */
+	if (READ_ONCE(priv->pt_port)) {
+		struct psmouse *child = READ_ONCE(priv->pt_psmouse);
+
+		if (child && child->state == PSMOUSE_ACTIVATED)
+			psmouse_set_state(child, PSMOUSE_ACTIVATED);

[Severity: Critical]
If a different driver (like serio_raw) is bound, this could interpret
arbitrary memory as a struct psmouse. By manipulating the serio_raw queue
buffer to match PSMOUSE_ACTIVATED and injecting a pointer, could this result
in psmouse_set_state() dereferencing an attacker-controlled pointer to
acquire a spinlock?

+	}
+
+	return len;
+}

[ ... ]

[Severity: Critical]
Can this cause a use-after-free, double-free, and self-deadlock?

If the parent port is unregistered (e.g. physical removal), the serio core
traverses the tree depth-first and synchronously destroys the child
pass-through port, freeing its memory. When synaptics_disconnect() is then
called for the parent, calling serio_unregister_port() and kfree() on the
dangling priv->pt_serio pointer could use freed memory and deadlock on
serio_mutex, which is already held by the core's unregistration path.

Alternatively, if userspace manually unbinds the parent driver via sysfs:

psmouse_disconnect() (acquires psmouse_mutex)
  synaptics_disconnect()
    serio_unregister_port()
      (unbinds child driver)
      psmouse_disconnect() for child
        (tries to acquire psmouse_mutex)

Could this sequence lead to a permanent self-deadlock on psmouse_mutex?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809154420.369073-1-development@laserology.net?part=1