From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-dy1-f182.google.com (mail-dy1-f182.google.com [74.125.82.182]) (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 CAFB94CA29A for ; Fri, 15 May 2026 15:35:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.82.182 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778859306; cv=none; b=dDJ/1RVN0wBE1wAsZ/qJTD3a0TIjGXn8FXnlJKHHf6QwJEFmXaQGTJa1191bWk2pbKSsoZ4LRZdFuexcZ1nVhTnB+kA2c2Dq2j0v/JGereUsLXIUYmi3cXy/Njx7pPMEVbguzqbl6i0rTQsiNquLZR9pdIQkNe4EkZrldKx5pHA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778859306; c=relaxed/simple; bh=v2JNmcpByx39XKC+11E6OssrXqdngtd1d7/XmxIPVxQ=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=KRzjWOhmrcJE7gGOm3HbNa5b+wM/Tk4ypeQzQ2eck20ooc6iF6ObvamKVdonhcLCZlvNqPZ5dj5jwJ0sDEn9aP5V3+cVkHhpS9HE/K3D0+/f4QxbypkFCJcr8kxS4IHk6pMC3cQt/9JsPgb8DMLOm1F0Y8NlTjGHem+p7CB2Juo= 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=Rm6U0Tlh; arc=none smtp.client-ip=74.125.82.182 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="Rm6U0Tlh" Received: by mail-dy1-f182.google.com with SMTP id 5a478bee46e88-2f7ca62a3c4so9794660eec.0 for ; Fri, 15 May 2026 08:35:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1778859301; x=1779464101; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=AA7kMpri2bwTCWgHKxeeXnaMWABeWfUan218sTmdqOU=; b=Rm6U0TlhoXHaVz5C21ajYqytanLnc1vGX24tXuxGAvxbNMO93C4Wu/tQf0peSsKaUV QqCBbLuTGX8pQ7o6h5Le1eorS5uPElGfFztu/Gy940gG/Ltz902SZXPIN5u9YpvPqbfG ZBVNnVm65pm6cx9N/L/Dt/frh3GB+cC6TFulEVeAPcl9nXbUoXdrDgIbQyrEKXQQCz9G igGPvu5N74RBrE17d5pSL7jK9TDOKjEJYxFdUP0LaaHHR7t2MIrS3yZ7xjpUonUciFKe 88XFSARdDFkSVvkvoDIc8oOuzTtGbRJdLOg58Lm2k/oEQunUYwqUL8mrRhuFdKa9mU5i znmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778859301; x=1779464101; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=AA7kMpri2bwTCWgHKxeeXnaMWABeWfUan218sTmdqOU=; b=Ycb7p9ht27jfUar4J25DuVZ8tk7mLMNh9w5lQRo6G55zAyac9TjATaxELXibaAh413 OgDJXtRoYScfpYPVrIsT+tFufrp0E4qlY+LHrU1GiwC+QT9f1BXqI11nVASgGn3yTStu UBK32R3F8Urs+K2/KFqhSQHPcZ9Sggxfigbjq3UyLc9ZR+401F2m8FvsZj8yq2WAgrtl Ksh6enqYE9iqGDy4AB56boSFNTkKutFqnxV0++kV2Z74616L0gvGbuW69pDYgUQIzGJI WE8e4h6DMo5nFVxSZdIStdDELcHbN0/ntnRQW74F+IKi9im79dXnlRg+2dqmrR+h2Fm8 /INQ== X-Gm-Message-State: AOJu0Yw/csfapXPhzjHqdlg43elaqaguT2Y24LnZyeCXNJRgVAu6BR/c a0yVfnFVPHPTmiBX08OdzMVOFVDa1OHs9LqRw2cRLsm1QzZmuLHVM5ZLR65wndxVQc8= X-Gm-Gg: Acq92OHIIk62TcCuPrvaf7lirj1Opu7Bs1UX+BGiY0Wc0/zRTnuXQGE0OwBRXoTVQyD gqidIbO7upOMvpn9MHMF98Dj7wkVnLniMQhggtuk+6c0RgGmDBiuTKeTrRPVZn+D62HVbAKDbyD 7sbFsQ+NY1ogMFxbC7ixteAYKrUp585bPHFiEs9ceDLLRPV1UsjTS/2Ztskb/Qy08w503riBETt NgqCJnCb6RvTFazP+7yob714jqAeJvmXaAQjp6NqSk2J3Xp8ETWEhfMF8SXtfa8gJRTfPeLImFb zPthk6BGiS3/0o4rpA4GTKgMUYJX14ef7GS0gdQWrcY/MZxOQfMyr7lD4tLJmSyYUFr15tgbagk hXok+tK07W2zIQJMKr3v/V82BIrOGCkWFrw8Ku+JGRcV3GETSFD0sqNpVBLqGv1ItDNTOhnLrb3 oOhMlOfO7modDPt8fSCdWAAE2jZ4mXu/zHvRgQDZbvQknCR5I9MSI/fDjG+cBqk0sMY9niU1bq/ EDUSJXOBpiPTaIkw/zsf7hJHbQdoa7TSFegVs0hB/bGUtWR9mNN3duLmQGdb0jv7b+aE4V1BJ7v Uc6KHycsb9f5oTId+N0wLNLs+x8v4AHUDJL4TWiQAQ== X-Received: by 2002:a05:7301:678d:b0:2f2:5726:db31 with SMTP id 5a478bee46e88-303986c518fmr2129275eec.35.1778859300400; Fri, 15 May 2026 08:35:00 -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 5a478bee46e88-30293e2e686sm8470324eec.5.2026.05.15.08.34.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 May 2026 08:35:00 -0700 (PDT) From: Manish Khadka To: linux-input@vger.kernel.org Cc: Jiri Kosina , Benjamin Tissoires , linux-kernel@vger.kernel.org Subject: [PATCH] HID: appleir: fix UAF on pending key_up_timer in remove() Date: Fri, 15 May 2026 21:19:50 +0545 Message-ID: <20260515153450.76146-1-maskmemanish@gmail.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-input@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 timer_delete_sync() drains it: CPU0 CPU1 appleir_raw_event() mod_timer(...) <- timer armed for 125 ms ... USB disconnect appleir_remove() hid_hw_stop() hid_disconnect() hidinput_disconnect() input_unregister_device() <- input_dev freed key_up_tick() key_up() input_report_key( appleir->input_dev, ...) ^^^^^^^^^ UAF timer_delete_sync() <- too late A simple reorder is not sufficient. Putting timer_delete_sync() 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. Introduce a 'removing' flag on struct appleir, gated by the existing spinlock. appleir_remove() sets the flag under the lock and then drains the timer; appleir_raw_event() and key_up_tick() bail out early if the flag is set, so no path can arm or run the timer after remove() has drained it. The keyrepeat branch of appleir_raw_event() previously called key_down() and mod_timer() 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 that 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 --- drivers/hid/hid-appleir.c | 40 ++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/drivers/hid/hid-appleir.c b/drivers/hid/hid-appleir.c index 5e8ced7bc05a..0e51f1e16653 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,12 +234,17 @@ 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; } @@ -318,8 +328,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); - hid_hw_stop(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_delete_sync(&appleir->key_up_timer); + hid_hw_stop(hid); } static const struct hid_device_id appleir_devices[] = { -- 2.43.0