From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-dl1-f53.google.com (mail-dl1-f53.google.com [74.125.82.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 85F71403124 for ; Fri, 15 May 2026 17:33:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.82.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778866382; cv=none; b=slC//D76/eHFXmZ4LjBynE+3VN9X2b0yIXNWSt3nSgOPUewTEWM4MDIUv0Dvs7ZrQ5A7LFCsAe1zEgzLVS+6NMUXw9IsV7X1rUzFDrX+ai/GlGuyLF90XTY3+q7EvJsOURexVgCNlPJT3/wuT5ECA1UWPv0TX4EZN+OoR3smx2Y= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778866382; c=relaxed/simple; bh=783I+Vcfn3/xrbaFwH/tYNw/Ih8gykDMsCMRePiJrG8=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=S8EywYWx8VW4TyKjenjUvonH/KC7WB1bRfoQi0ajOchCGFRCH/oW/jKYePVmaJgEWU0AQarD+K8/r3H+wgYufVJPwICmgN9bvh5qLGR34B8QekArGei1lqeqi8HwC23xp5Eruy6W4MZJApejzsUAftdGZsFBnnTEdcTOcEacbSs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=sGuufS01; arc=none smtp.client-ip=74.125.82.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="sGuufS01" Received: by mail-dl1-f53.google.com with SMTP id a92af1059eb24-12c19d23b19so21584c88.0 for ; Fri, 15 May 2026 10:33:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1778866380; x=1779471180; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=qHPjPDY3Oupzg9SEW7RsWWVh+sZnvMhJ/Xrr485aLTk=; b=sGuufS01/RK5+mO3Ws61Dzq1B17SIhf78w5zPLvgrAeJ0dvS2/5uverTRd0HF2TPWB qOaFG5wGk1ZnzYpPBuytSvQ2TelM05ZPXznBNKrXTQ+Ozc34YY0QVBwB+vj+fvIee9lh lN38A/ZVDQ0C8YBtdn0k4IzveEk+b0JZrqPH+EIoHdtXJ5aaw1jcvF85Sd5EYKxEv6TX HO5j1R9+xrmqf+/cjZL+kcbcW0I9qpXC3icbxkPbEeyROgRnIL9G9D5t1xJDvWURoSjW Va7wFkAJgEwXALWfUJ8/q0IHuzFClkBgjD2gplQVkxYG9AeS7Cy9EzFMg3ZMoZ485O62 z0sQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778866380; x=1779471180; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=qHPjPDY3Oupzg9SEW7RsWWVh+sZnvMhJ/Xrr485aLTk=; b=DHcnlXcVzip5YjTX5dl5g/CfuGWPFO5as7Ephe28zxKOrg+j3PJWHu7w7YvQv+9Rit DjajU3a2TJKnPAybInmqKT4XeCeyKDrIUlLFPwzq+buaifZf/0tJe02qFSL8XHeRFE1a re9oPCTHu9MTcTRPewNnFzKqrwwlBH/GoDCo6WsQVs1saTO48gn5gSD3SspNU4jkZ1IH LsWmKSE2VWY/FI+m9J+iPKfayDPwr5g9Y/Rn2U81wZf+aHjH14o0EyuAh+CsdQP8sCNY WVSXd/BdYb+c8ZthFEGjtEvGEmTRGf1rAX1R08OJMxQxoQvT8Bl6ud/1EXveMZeCPqD4 qoZA== X-Forwarded-Encrypted: i=1; AFNElJ+iMOB3qu6ccLjl0Jwfz9ijfjMT/kc4sGCzT1xRQ70jUY04XengizHcVAtYoIpLz96KTdWA3bDiOa0OxzU=@vger.kernel.org X-Gm-Message-State: AOJu0YzXt6ZnoGvzUceKZjN4zqdSfnM2+UYknZRzXcOrbTtKvcBIe6KS uq+drOPHhFMDDtWd5xcDj/HdjahrBo3DePEwkgHH1KGRVIar2Df3l4x6 X-Gm-Gg: Acq92OHxTkjC5SlzyaqbCVSn5plSH29Y4S892OPk0AfIDriseHys/SjfqXZZf1qlj1Q SQ0/cqbIoXeoxRJlMjW1sMS2MKyWTgHHPGJBZLMikluMjijOD4G8FYnh2XdFhNWdJ3IxKpSbF8S Ij53+WJ3MyNUI23ATkJt5gXconYyOi4Yadw5Z3pqQL73mAjbX7z8sWnNIpKio4QuySc1yJY1nZM WeHMwsbtbI5/L+lbUYY6JKHu5i4X/RUCQxWjogRaMl1WfXZNLkZu8hGsuNy28vsoxOh3rn4ELdV rDx4hb8iWGQCPQ9fVTN5+h2uYZle6EPG42WWXMDl4lLwBY/BfNOqG7tQ3SbIXHtKBD3807lXTgV ZY7L9hPzoRgY4ie2MtO/Wb3PFwC31wvRMhHfgMl/BmilBFyy/kRNlT1ZluPWlQgfGWMD9+WD9TX eOm8cImbW0A8X3/g8EdFmVGR3gyEZleFaKGm1fUeRpTeEGLlyyXIlulDt2Ns7Y8RGV3t8eLeKDe +HLlERdjUpt/GMLu6QQmX8kcKXx6QE7M5/K3T937T/RO5aUCwhNgq8kF3STVIc4k52pyKDNOwi0 Tk+0961MmJpxJ6wis+1vBG38zRcuuRM= X-Received: by 2002:a05:7022:692:b0:128:d375:f1cc with SMTP id a92af1059eb24-1350441bb46mr2382031c88.12.1778866379529; Fri, 15 May 2026 10:32:59 -0700 (PDT) Received: from lima-kernel-dev.google.com (ec2-13-52-214-175.us-west-1.compute.amazonaws.com. [13.52.214.175]) by smtp.gmail.com with ESMTPSA id a92af1059eb24-134cc33aac9sm8543634c88.14.2026.05.15.10.32.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 May 2026 10:32:59 -0700 (PDT) From: Manish Khadka To: linux-input@vger.kernel.org Cc: Jiri Kosina , Benjamin Tissoires , linux-kernel@vger.kernel.org Subject: [PATCH v2] HID: appleir: fix UAF on pending key_up_timer in remove() Date: Fri, 15 May 2026 23:17:52 +0545 Message-ID: <20260515173252.77757-1-maskmemanish@gmail.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260515160218.4D39EC2BCB0@smtp.kernel.org> References: <20260515160218.4D39EC2BCB0@smtp.kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit appleir_remove() runs hid_hw_stop() before timer_delete_sync(). hid_hw_stop() synchronously unregisters the HID input device via hid_disconnect() -> hidinput_disconnect() -> input_unregister_device(), which drops the last reference and frees the underlying input_dev when no userspace handle holds it open. key_up_tick() reads appleir->input_dev and calls input_report_key() / input_sync() on it. The timer is armed from appleir_raw_event() with a HZ/8 (~125 ms) timeout on every keydown and key-repeat report. If a key was pressed shortly before the device is disconnected, the timer can fire after hid_hw_stop() has freed input_dev but before the teardown drains it. A simple reorder is not sufficient. Putting the timer drain first still leaves a window where a USB URB completion (raw_event) running during hid_hw_stop() can call mod_timer() and re-arm the timer, which then fires after hidinput_disconnect() has freed input_dev. The same URB-completion window also lets raw_event() reach key_up(), key_down() and battery_flat() directly, all of which dereference appleir->input_dev. Introduce a 'removing' flag on struct appleir, gated by the existing spinlock. appleir_remove() sets the flag under the lock and then shuts down the timer with timer_shutdown_sync(), which both drains any in-flight callback and permanently disables further mod_timer() calls. appleir_raw_event() and key_up_tick() bail out early if the flag is set, so no path can arm or run the timer, or dereference appleir->input_dev, after remove() has started tearing down. The keyrepeat and flatbattery branches of appleir_raw_event() previously called into the input layer without holding the spinlock; take it now so the flag check is well-defined. This incidentally closes a pre-existing read-side race on appleir->current_key in the keyrepeat branch. This bug is structurally a sibling of commit 4db2af929279 ("HID: appletb-kbd: fix UAF in inactivity-timer cleanup path") and has been present since the driver was introduced. Fixes: 9a4a5574ce42 ("HID: appleir: add support for Apple ir devices") Cc: stable@vger.kernel.org Signed-off-by: Manish Khadka --- v1 -> v2: - Address Sashiko AI review feedback: * [Critical] Gate the flatbattery branch in appleir_raw_event() under the existing spinlock + removing flag so battery_flat(), which does dev_err(&appleir->input_dev->dev, ...), cannot UAF during hid_hw_stop(). * [Medium] Use timer_shutdown_sync() instead of timer_delete_sync() so the timer is permanently disabled in addition to drained, providing belt-and-suspenders against any future arming site that bypasses the removing flag. Thanks to Sashiko AI review for both points. drivers/hid/hid-appleir.c | 45 ++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/drivers/hid/hid-appleir.c b/drivers/hid/hid-appleir.c index 5e8ced7bc05a..adaa44a858ed 100644 --- a/drivers/hid/hid-appleir.c +++ b/drivers/hid/hid-appleir.c @@ -109,9 +109,10 @@ struct appleir { struct hid_device *hid; unsigned short keymap[ARRAY_SIZE(appleir_key_table)]; struct timer_list key_up_timer; /* timer for key up */ - spinlock_t lock; /* protects .current_key */ + spinlock_t lock; /* protects .current_key, .removing */ int current_key; /* the currently pressed key */ int prev_key_idx; /* key index in a 2 packets message */ + bool removing; /* set during teardown; gates input_dev access */ }; static int get_key(int data) @@ -172,7 +173,7 @@ static void key_up_tick(struct timer_list *t) unsigned long flags; spin_lock_irqsave(&appleir->lock, flags); - if (appleir->current_key) { + if (!appleir->removing && appleir->current_key) { key_up(hid, appleir, appleir->current_key); appleir->current_key = 0; } @@ -195,6 +196,10 @@ static int appleir_raw_event(struct hid_device *hid, struct hid_report *report, int index; spin_lock_irqsave(&appleir->lock, flags); + if (appleir->removing) { + spin_unlock_irqrestore(&appleir->lock, flags); + goto out; + } /* * If we already have a key down, take it up before marking * this one down @@ -229,17 +234,25 @@ static int appleir_raw_event(struct hid_device *hid, struct hid_report *report, appleir->prev_key_idx = 0; if (!memcmp(data, keyrepeat, sizeof(keyrepeat))) { - key_down(hid, appleir, appleir->current_key); - /* - * Remote doesn't do key up, either pull them up, in the test - * above, or here set a timer which pulls them up after 1/8 s - */ - mod_timer(&appleir->key_up_timer, jiffies + HZ / 8); + spin_lock_irqsave(&appleir->lock, flags); + if (!appleir->removing) { + key_down(hid, appleir, appleir->current_key); + /* + * Remote doesn't do key up, either pull them up, in + * the test above, or here set a timer which pulls them + * up after 1/8 s + */ + mod_timer(&appleir->key_up_timer, jiffies + HZ / 8); + } + spin_unlock_irqrestore(&appleir->lock, flags); goto out; } if (!memcmp(data, flatbattery, sizeof(flatbattery))) { - battery_flat(appleir); + spin_lock_irqsave(&appleir->lock, flags); + if (!appleir->removing) + battery_flat(appleir); + spin_unlock_irqrestore(&appleir->lock, flags); /* Fall through */ } @@ -318,8 +331,20 @@ static int appleir_probe(struct hid_device *hid, const struct hid_device_id *id) static void appleir_remove(struct hid_device *hid) { struct appleir *appleir = hid_get_drvdata(hid); + unsigned long flags; + + /* + * Mark the driver as tearing down so that any concurrent raw_event + * (e.g. from a USB URB completion that hid_hw_stop() has not yet + * killed) and the key_up_timer softirq stop touching input_dev + * before hid_hw_stop() frees it via hidinput_disconnect(). + */ + spin_lock_irqsave(&appleir->lock, flags); + appleir->removing = true; + spin_unlock_irqrestore(&appleir->lock, flags); + + timer_shutdown_sync(&appleir->key_up_timer); hid_hw_stop(hid); - timer_delete_sync(&appleir->key_up_timer); } static const struct hid_device_id appleir_devices[] = { -- 2.43.0