[PATCH] HID: logitech-hidpp: fix race condition when accessing stale stack pointer

[PATCH] HID: logitech-hidpp: fix race condition when accessing stale stack pointer

[Benoit Sevens]

From: Benoît Sevens <bsevens@google.com>

The driver uses hidpp->send_receive_buf to point to a stack-allocated
buffer in the synchronous command path (__do_hidpp_send_message_sync).
However, this pointer is not cleared when the function returns.

If an event is processed (e.g. by a different thread) while the
send_mutex is held by a new command, but before that command has
updated send_receive_buf, the handler (hidpp_raw_hidpp_event) will
observe that the mutex is locked and dereference the stale pointer.

This results in an out-of-bounds access on a different thread's kernel
stack (or a NULL pointer dereference on the very first command).

Fix this by:
1. Clearing hidpp->send_receive_buf to NULL before releasing the mutex
   in the synchronous command path.
2. Moving the assignment of the local 'question' and 'answer' pointers
   inside the mutex_is_locked() block in the handler, and adding
   a NULL check before dereferencing.

Signed-off-by: Benoît Sevens <bsevens@google.com>
---
 drivers/hid/hid-logitech-hidpp.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index e871f1729d4b..42f7ea5b25dc 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -306,21 +306,22 @@ static int __do_hidpp_send_message_sync(struct hidpp_device *hidpp,
 	if (ret) {
 		dbg_hid("__hidpp_send_report returned err: %d\n", ret);
 		memset(response, 0, sizeof(struct hidpp_report));
-		return ret;
+		goto out;
 	}
 
 	if (!wait_event_timeout(hidpp->wait, hidpp->answer_available,
 				5*HZ)) {
 		dbg_hid("%s:timeout waiting for response\n", __func__);
 		memset(response, 0, sizeof(struct hidpp_report));
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
+		goto out;
 	}
 
 	if (response->report_id == REPORT_ID_HIDPP_SHORT &&
 	    response->rap.sub_id == HIDPP_ERROR) {
 		ret = response->rap.params[1];
 		dbg_hid("%s:got hidpp error %02X\n", __func__, ret);
-		return ret;
+		goto out;
 	}
 
 	if ((response->report_id == REPORT_ID_HIDPP_LONG ||
@@ -328,10 +329,14 @@ static int __do_hidpp_send_message_sync(struct hidpp_device *hidpp,
 	    response->fap.feature_index == HIDPP20_ERROR) {
 		ret = response->fap.params[1];
 		dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret);
-		return ret;
+		goto out;
 	}
 
-	return 0;
+	ret = 0;
+
+out:
+	hidpp->send_receive_buf = NULL;
+	return ret;
 }
 
 /*
@@ -3840,8 +3845,7 @@ static int hidpp_input_configured(struct hid_device *hdev,
 static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 		int size)
 {
-	struct hidpp_report *question = hidpp->send_receive_buf;
-	struct hidpp_report *answer = hidpp->send_receive_buf;
+	struct hidpp_report *question, *answer;
 	struct hidpp_report *report = (struct hidpp_report *)data;
 	int ret;
 	int last_online;
@@ -3851,6 +3855,12 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 	 * previously sent command.
 	 */
 	if (unlikely(mutex_is_locked(&hidpp->send_mutex))) {
+		question = hidpp->send_receive_buf;
+		answer = hidpp->send_receive_buf;
+
+		if (!question)
+			return 0;
+
 		/*
 		 * Check for a correct hidpp20 answer or the corresponding
 		 * error
-- 
2.53.0.1118.gaef5881109-goog

Re: [PATCH] HID: logitech-hidpp: fix race condition when accessing stale stack pointer

[Jiri Kosina]

On Wed, 1 Apr 2026, Benoit Sevens wrote:

> From: Beno=C3=AEt Sevens <bsevens@google.com>
> 
> The driver uses hidpp->send_receive_buf to point to a stack-allocated
> buffer in the synchronous command path (__do_hidpp_send_message_sync).
> However, this pointer is not cleared when the function returns.
> 
> If an event is processed (e.g. by a different thread) while the
> send_mutex is held by a new command, but before that command has
> updated send_receive_buf, the handler (hidpp_raw_hidpp_event) will
> observe that the mutex is locked and dereference the stale pointer.
> 
> This results in an out-of-bounds access on a different thread's kernel
> stack (or a NULL pointer dereference on the very first command).
> 
> Fix this by:
> 1. Clearing hidpp->send_receive_buf to NULL before releasing the mutex
>    in the synchronous command path.
> 2. Moving the assignment of the local 'question' and 'answer' pointers
>    inside the mutex_is_locked() block in the handler, and adding
>    a NULL check before dereferencing.

Now applied.

Benjamin had some ideas on further cleanup (allocating with __free__ 
instead of using stack pointer), but that'd be a little bigger cleanup, so 
let's keep that separate.

Thanks,

-- 
Jiri Kosina
SUSE Labs

[STABLE v6.18-v6.6][PATCH] HID: logitech-hidpp: fix race condition when accessing stale stack pointer

[Lee Jones]

Dear Stable,

On Thu, 09 Apr 2026, Jiri Kosina wrote:

> On Wed, 1 Apr 2026, Benoit Sevens wrote:
> 
> > From: Beno=C3=AEt Sevens <bsevens@google.com>
> > 
> > The driver uses hidpp->send_receive_buf to point to a stack-allocated
> > buffer in the synchronous command path (__do_hidpp_send_message_sync).
> > However, this pointer is not cleared when the function returns.
> > 
> > If an event is processed (e.g. by a different thread) while the
> > send_mutex is held by a new command, but before that command has
> > updated send_receive_buf, the handler (hidpp_raw_hidpp_event) will
> > observe that the mutex is locked and dereference the stale pointer.
> > 
> > This results in an out-of-bounds access on a different thread's kernel
> > stack (or a NULL pointer dereference on the very first command).
> > 
> > Fix this by:
> > 1. Clearing hidpp->send_receive_buf to NULL before releasing the mutex
> >    in the synchronous command path.
> > 2. Moving the assignment of the local 'question' and 'answer' pointers
> >    inside the mutex_is_locked() block in the handler, and adding
> >    a NULL check before dereferencing.
> 
> Now applied.
> 
> Benjamin had some ideas on further cleanup (allocating with __free__ 
> instead of using stack pointer), but that'd be a little bigger cleanup, so 
> let's keep that separate.

Please could we have this commit backported through to linux-6.6.y.

e2aaf2d3ad92 ("HID: logitech-hidpp: fix race condition when accessing stale stack pointer")

I will follow-up with a backported version for the older releases.

-- 
Lee Jones

Re: [STABLE v6.18-v6.6][PATCH] HID: logitech-hidpp: fix race condition when accessing stale stack pointer

[Greg KH]

On Tue, Sep 22, 2026 at 08:54:00AM +0100, Lee Jones wrote:
> Dear Stable,
> 
> On Thu, 09 Apr 2026, Jiri Kosina wrote:
> 
> > On Wed, 1 Apr 2026, Benoit Sevens wrote:
> > 
> > > From: Beno=C3=AEt Sevens <bsevens@google.com>
> > > 
> > > The driver uses hidpp->send_receive_buf to point to a stack-allocated
> > > buffer in the synchronous command path (__do_hidpp_send_message_sync).
> > > However, this pointer is not cleared when the function returns.
> > > 
> > > If an event is processed (e.g. by a different thread) while the
> > > send_mutex is held by a new command, but before that command has
> > > updated send_receive_buf, the handler (hidpp_raw_hidpp_event) will
> > > observe that the mutex is locked and dereference the stale pointer.
> > > 
> > > This results in an out-of-bounds access on a different thread's kernel
> > > stack (or a NULL pointer dereference on the very first command).
> > > 
> > > Fix this by:
> > > 1. Clearing hidpp->send_receive_buf to NULL before releasing the mutex
> > >    in the synchronous command path.
> > > 2. Moving the assignment of the local 'question' and 'answer' pointers
> > >    inside the mutex_is_locked() block in the handler, and adding
> > >    a NULL check before dereferencing.
> > 
> > Now applied.
> > 
> > Benjamin had some ideas on further cleanup (allocating with __free__ 
> > instead of using stack pointer), but that'd be a little bigger cleanup, so 
> > let's keep that separate.
> 
> Please could we have this commit backported through to linux-6.6.y.
> 
> e2aaf2d3ad92 ("HID: logitech-hidpp: fix race condition when accessing stale stack pointer")
> 
> I will follow-up with a backported version for the older releases.

Now queued up, thanks.

greg k-h