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

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>
Signed-off-by: Jiri Kosina <jkosina@suse.com>

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index ab5d676cbb029d53bebfb8f5c666293af699678b..8bcd43f17b07b586c57ea90d6288874883c92f82 100644 (file)
--- 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;
 }
 
 /*
@@ -3843,8 +3848,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;
@@ -3854,6 +3858,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