Input: synaptics - fix crash when enabling pass-through port

PATCH In review Dmitry Torokhov 2 participants 5 messages Jan 17, 2025 – Jan 21, 2025

Subsystems (2)

HID Input drivers

lore

Share & tools

lore

Dmitry Torokhov

author maintainer

2025-01-17 17:23 UTC #0 lore raw

PATCH 1/1 Input: synaptics - fix crash when enabling pass-through port

When enabling a pass-through port an interrupt might come before psmouse driver binds to the pass-through port. However synaptics sub-driver tries to access psmouse instance presumably associated with the pass-through port to figure out if only 1 byte of response or entire protocol packet needs to be forwarded to the pass-through port and may crash if psmouse instance has not been attached to the port yet.

Fix the crash by introducing open() and close() methods for the port and check if the port is open before trying to access psmouse instance. Because psmouse calls serio_open() only after attaching psmouse instance to serio port instance this prevents the potential crash.

Reported-by Takashi Iwai <[email protected]>

Fixes 100e16959c3c ("Input: libps2 - attach ps2dev instances as serio port's drvdata")

Link https://bugzilla.suse.com/show_bug.cgi?id=1219522

Cc [email protected]

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/mouse/synaptics.c | 56 ++++++++++++++++++++++++---------
 drivers/input/mouse/synaptics.h |  1 +
 2 files changed, 43 insertions(+), 14 deletions(-)

drivers/input/mouse/synaptics.c +42 ~~-14~~

index 2735f86c23cc..aba57abe6978 100644
@@ -665,23 +665,50 @@ static void synaptics_pt_stop(struct serio *serio)
 	priv->pt_port = NULL;
 }
 
+static int synaptics_pt_open(struct serio *serio)
+{
+	struct psmouse *parent = psmouse_from_serio(serio->parent);
+	struct synaptics_data *priv = parent->private;
+
+	guard(serio_pause_rx)(parent->ps2dev.serio);
+	priv->pt_port_open = true;
+
+	return 0;
+}
+
+static void synaptics_pt_close(struct serio *serio)
+{
+	struct psmouse *parent = psmouse_from_serio(serio->parent);
+	struct synaptics_data *priv = parent->private;
+
+	guard(serio_pause_rx)(parent->ps2dev.serio);
+	priv->pt_port_open = false;
+}
+
 static int synaptics_is_pt_packet(u8 *buf)
 {
 	return (buf[0] & 0xFC) == 0x84 && (buf[3] & 0xCC) == 0xC4;
 }
 
-static void synaptics_pass_pt_packet(struct serio *ptport, u8 *packet)
+static void synaptics_pass_pt_packet(struct synaptics_data *priv, u8 *packet)
 {
-	struct psmouse *child = psmouse_from_serio(ptport);
+	struct serio *ptport;
 
-	if (child && child->state == PSMOUSE_ACTIVATED) {
-		serio_interrupt(ptport, packet[1], 0);
-		serio_interrupt(ptport, packet[4], 0);
-		serio_interrupt(ptport, packet[5], 0);
-		if (child->pktsize == 4)
-			serio_interrupt(ptport, packet[2], 0);
-	} else {
-		serio_interrupt(ptport, packet[1], 0);
+	ptport = priv->pt_port;
+	if (!ptport)
+		return;
+
+	serio_interrupt(ptport, packet[1], 0);
+
+	if (priv->pt_port_open) {
+		struct psmouse *child = psmouse_from_serio(ptport);
+
+		if (child->state == PSMOUSE_ACTIVATED) {
+			serio_interrupt(ptport, packet[4], 0);
+			serio_interrupt(ptport, packet[5], 0);
+			if (child->pktsize == 4)
+				serio_interrupt(ptport, packet[2], 0);
+		}
 	}
 }

@@ -720,6 +747,8 @@ static void synaptics_pt_create(struct psmouse *psmouse)
 	serio->write = synaptics_pt_write;
 	serio->start = synaptics_pt_start;
 	serio->stop = synaptics_pt_stop;
+	serio->open = synaptics_pt_open;
+	serio->close = synaptics_pt_close;
 	serio->parent = psmouse->ps2dev.serio;
 
 	psmouse->pt_activate = synaptics_pt_activate;

@@ -1216,11 +1245,10 @@ static psmouse_ret_t synaptics_process_byte(struct psmouse *psmouse)
 
 		if (SYN_CAP_PASS_THROUGH(priv->info.capabilities) &&
 		    synaptics_is_pt_packet(psmouse->packet)) {
-			if (priv->pt_port)
-				synaptics_pass_pt_packet(priv->pt_port,
-							 psmouse->packet);
-		} else
+			synaptics_pass_pt_packet(priv, psmouse->packet);
+		} else {
 			synaptics_process_packet(psmouse);
+		}
 
 		return PSMOUSE_FULL_PACKET;
 	}

drivers/input/mouse/synaptics.h +1 -0

index 899aee598632..3853165b6b3a 100644
@@ -188,6 +188,7 @@ struct synaptics_data {
 	bool disable_gesture;			/* disable gestures */
 
 	struct serio *pt_port;			/* Pass-through serio port */
+	bool pt_port_open;
 
 	/*
 	 * Last received Advanced Gesture Mode (AGM) packet. An AGM packet

Signature · 5 lines

2.48.0.rc2.279.g1de40edade-goog


-- 
Dmitry

Takashi Iwai

Reviewed-by on v1 1/1

2025-01-18 08:46 UTC #1 lore raw

Quoted 13 lines · Dmitry Torokhov

When enabling a pass-through port an interrupt might come before psmouse driver binds to the pass-through port. However synaptics sub-driver tries to access psmouse instance presumably associated with the pass-through port to figure out if only 1 byte of response or entire protocol packet needs to be forwarded to the pass-through port and may crash if psmouse instance has not been attached to the port yet.
Fix the crash by introducing open() and close() methods for the port and check if the port is open before trying to access psmouse instance. Because psmouse calls serio_open() only after attaching psmouse instance to serio port instance this prevents the potential crash.

Quoted 5 lines

Reported-by Takashi Iwai <[email protected]>
Fixes 100e16959c3c ("Input: libps2 - attach ps2dev instances as serio port's drvdata")
Link https://bugzilla.suse.com/show_bug.cgi?id=1219522
Cc [email protected]
Signed-off-by Dmitry Torokhov <[email protected]>

Reviewed-by Takashi Iwai <[email protected]>

Thanks!

Takashi

Takashi Iwai

2025-01-18 09:33 UTC #2 lore raw

Quoted 16 lines · Takashi Iwai

Quoted 13 lines · Dmitry Torokhov
When enabling a pass-through port an interrupt might come before psmouse driver binds to the pass-through port. However synaptics sub-driver tries to access psmouse instance presumably associated with the pass-through port to figure out if only 1 byte of response or entire protocol packet needs to be forwarded to the pass-through port and may crash if psmouse instance has not been attached to the port yet.
Fix the crash by introducing open() and close() methods for the port and check if the port is open before trying to access psmouse instance. Because psmouse calls serio_open() only after attaching psmouse instance to serio port instance this prevents the potential crash.

Quoted 7 lines

Quoted 5 lines
Reported-by Takashi Iwai <[email protected]>
Fixes 100e16959c3c ("Input: libps2 - attach ps2dev instances as serio port's drvdata")
Link https://bugzilla.suse.com/show_bug.cgi?id=1219522
Cc [email protected]
Signed-off-by Dmitry Torokhov <[email protected]>
Reviewed-by Takashi Iwai <[email protected]>

BTW, backporting this patch to stable kernels wouldn't be trivial; it needs the recent changes for guard(), too.

Takashi

Dmitry Torokhov

author maintainer

2025-01-21 05:25 UTC #3 lore raw

Quoted 18 lines · Takashi Iwai

Quoted 16 lines · Takashi Iwai
Quoted 13 lines · Dmitry Torokhov
When enabling a pass-through port an interrupt might come before psmouse driver binds to the pass-through port. However synaptics sub-driver tries to access psmouse instance presumably associated with the pass-through port to figure out if only 1 byte of response or entire protocol packet needs to be forwarded to the pass-through port and may crash if psmouse instance has not been attached to the port yet.
Fix the crash by introducing open() and close() methods for the port and check if the port is open before trying to access psmouse instance. Because psmouse calls serio_open() only after attaching psmouse instance to serio port instance this prevents the potential crash.

Quoted 7 lines

Quoted 7 lines
Quoted 5 lines
Reported-by Takashi Iwai <[email protected]>
Fixes 100e16959c3c ("Input: libps2 - attach ps2dev instances as serio port's drvdata")
Link https://bugzilla.suse.com/show_bug.cgi?id=1219522
Cc [email protected]
Signed-off-by Dmitry Torokhov <[email protected]>
Reviewed-by Takashi Iwai <[email protected]>

Thank you for the review.

Quoted 3 lines

BTW, backporting this patch to stable kernels wouldn't be trivial; it needs the recent changes for guard(), too.

It is as simple as changing it to pairs of serio_pause_rx()/serio_continue_rx() so pretty trivial.

Thanks.

Signature · 1 line

Dmitry

Takashi Iwai

2025-01-21 07:00 UTC #4 lore raw

Quoted 28 lines · Dmitry Torokhov

Quoted 18 lines · Takashi Iwai
Quoted 16 lines · Takashi Iwai
Quoted 13 lines · Dmitry Torokhov
When enabling a pass-through port an interrupt might come before psmouse driver binds to the pass-through port. However synaptics sub-driver tries to access psmouse instance presumably associated with the pass-through port to figure out if only 1 byte of response or entire protocol packet needs to be forwarded to the pass-through port and may crash if psmouse instance has not been attached to the port yet.
Fix the crash by introducing open() and close() methods for the port and check if the port is open before trying to access psmouse instance. Because psmouse calls serio_open() only after attaching psmouse instance to serio port instance this prevents the potential crash.
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
Reported-by Takashi Iwai <[email protected]>
Fixes 100e16959c3c ("Input: libps2 - attach ps2dev instances as serio port's drvdata")
Link https://bugzilla.suse.com/show_bug.cgi?id=1219522
Cc [email protected]
Signed-off-by Dmitry Torokhov <[email protected]>
Reviewed-by Takashi Iwai <[email protected]>

Quoted 8 lines

Thank you for the review.
Quoted 3 lines
BTW, backporting this patch to stable kernels wouldn't be trivial; it needs the recent changes for guard(), too.
It is as simple as changing it to pairs of serio_pause_rx()/serio_continue_rx() so pretty trivial.

Yes, but for applying it to stable kernels, you'd need either modify the patch itself or pick up the preliminary patches. It won't be obvious for stable maintainers unless you explicitly order so.

thanks,

Takashi