public inbox for linux-input@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: "Günther Noack" <gnoack@google.com>,
	"Benjamin Tissoires" <bentiss@kernel.org>,
	"Sasha Levin" <sashal@kernel.org>,
	jikos@kernel.org, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.19-6.1] HID: apple: avoid memory leak in apple_report_fixup()
Date: Tue, 10 Mar 2026 05:01:23 -0400	[thread overview]
Message-ID: <20260310090145.2709021-23-sashal@kernel.org> (raw)
In-Reply-To: <20260310090145.2709021-1-sashal@kernel.org>

From: Günther Noack <gnoack@google.com>

[ Upstream commit 239c15116d80f67d32f00acc34575f1a6b699613 ]

The apple_report_fixup() function was returning a
newly kmemdup()-allocated buffer, but never freeing it.

The caller of report_fixup() does not take ownership of the returned
pointer, but it *is* permitted to return a sub-portion of the input
rdesc, whose lifetime is managed by the caller.

Assisted-by: Gemini-CLI:Google Gemini 3
Signed-off-by: Günther Noack <gnoack@google.com>
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

This is very revealing. The caller's flow is:

1. `buf = kmemdup(start, size, ...)` — makes a copy of rdesc
2. `start = device->driver->report_fixup(device, buf, &size)` — calls
   the fixup, passing `buf`
3. `start = kmemdup(start, size, ...)` — copies whatever `report_fixup`
   returned
4. `kfree(buf)` — frees the original buf

**The old buggy code**: `apple_report_fixup` did `rdesc = kmemdup(rdesc
+ 1, ...)` which allocated a NEW buffer and returned it. The caller then
does `kmemdup(start, size)` (copies it again) and `kfree(buf)` (frees
the ORIGINAL `buf`). But the intermediate buffer allocated by
`apple_report_fixup` is never freed — leaked.

**The fix**: Instead of `kmemdup(rdesc + 1, ...)`, just do `rdesc =
rdesc + 1`. This returns a pointer into the caller's `buf`, which the
caller will properly copy and then free.

This is safe because:
- The caller already makes a copy of whatever is returned
  (`kmemdup(start, size)`)
- The caller then frees `buf`
- Since `rdesc + 1` points into `buf`, it's valid until `kfree(buf)`
  which happens AFTER the copy

## Analysis

**What problem the commit solves**: Every time `apple_report_fixup()` is
called for a Magic Keyboard with the `APPLE_RDESC_BATTERY` quirk, a
`kmemdup()` allocation leaks memory. This happens at device probe time,
so each time a Magic Keyboard is connected (or the driver is reloaded),
memory is leaked.

**Meets stable kernel rules**:
1. **Obviously correct and tested** — Yes, the fix simply changes
   `kmemdup(rdesc + 1, ...)` to `rdesc + 1`. The caller already copies
   the returned buffer, so the intermediate allocation was unnecessary.
2. **Fixes a real bug** — Yes, a real memory leak.
3. **Important issue** — Memory leak that occurs on every device probe
   for affected Apple Magic Keyboards. While not a crash, it's a
   persistent resource leak.
4. **Small and contained** — Yes, a 3-line change (removes kmemdup +
   error check, replaces with pointer arithmetic).
5. **No new features** — Correct, pure bug fix.

**Risk vs benefit**: Very low risk. The change simplifies the code by
removing an allocation. The caller's code in `hid-core.c` explicitly
handles this case (comment says "report_fixup() returns a sub-portion of
the input rdesc"). Benefit is fixing a real memory leak affecting Apple
keyboard users.

**Affected versions**: The buggy code was introduced in commit
`6e143293e17a7` which first appeared in v5.17-rc1. All stable trees from
5.17+ are affected (6.1.y, 6.6.y, 6.12.y, etc.).

**Dependencies**: None. The fix is self-contained and applies cleanly.

## Verification

- `git show 6e143293e17a7` confirmed this is the commit that introduced
  the buggy `kmemdup` call in `apple_report_fixup()`, first appearing in
  v5.17-rc1
- Read `hid-core.c:1285-1304` confirmed the caller does NOT take
  ownership of the returned pointer — it copies it with `kmemdup(start,
  size)` and then frees `buf`, meaning any separate allocation by
  `report_fixup` is leaked
- The caller's comment at line 1298-1301 explicitly states
  "report_fixup() returns a static read-only memory... we have no idea
  if that memory needs to be cleaned up" — confirming the caller cannot
  and does not free the report_fixup return value
- Read the fixed code at `hid-apple.c:691-692` confirming `rdesc = rdesc
  + 1` correctly returns a sub-portion of the input buffer, which the
  caller handles properly
- The fix removes 3 lines and adds 1 line — minimal change with clear
  semantics

**YES**

 drivers/hid/hid-apple.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 2f9a2e07c4263..9dcb252c5d6c7 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -689,9 +689,7 @@ static const __u8 *apple_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		hid_info(hdev,
 			 "fixing up Magic Keyboard battery report descriptor\n");
 		*rsize = *rsize - 1;
-		rdesc = kmemdup(rdesc + 1, *rsize, GFP_KERNEL);
-		if (!rdesc)
-			return NULL;
+		rdesc = rdesc + 1;
 
 		rdesc[0] = 0x05;
 		rdesc[1] = 0x01;
-- 
2.51.0


  parent reply	other threads:[~2026-03-10  9:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260310090145.2709021-1-sashal@kernel.org>
2026-03-10  9:01 ` [PATCH AUTOSEL 6.19-6.6] HID: asus: add xg mobile 2023 external hardware support Sasha Levin
2026-03-10  9:01 ` [PATCH AUTOSEL 6.19-5.10] HID: mcp2221: cancel last I2C command on read error Sasha Levin
2026-03-10  9:01 ` [PATCH AUTOSEL 6.19-5.10] HID: asus: avoid memory leak in asus_report_fixup() Sasha Levin
2026-03-10  9:01 ` [PATCH AUTOSEL 6.19-5.10] platform/x86: touchscreen_dmi: Add quirk for y-inverted Goodix touchscreen on SUPI S10 Sasha Levin
2026-03-10  9:01 ` Sasha Levin [this message]
2026-03-10  9:01 ` [PATCH AUTOSEL 6.19-6.12] HID: apple: Add EPOMAKER TH87 to the non-apple keyboards list Sasha Levin
2026-03-10  9:01 ` [PATCH AUTOSEL 6.19-5.15] HID: magicmouse: fix battery reporting for Apple Magic Trackpad 2 Sasha Levin
2026-03-10  9:01 ` [PATCH AUTOSEL 6.19-6.18] HID: intel-ish-hid: ipc: Add Nova Lake-H/S PCI device IDs Sasha Levin
2026-03-10  9:01 ` [PATCH AUTOSEL 6.19-5.15] HID: magicmouse: avoid memory leak in magicmouse_report_fixup() Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260310090145.2709021-23-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=bentiss@kernel.org \
    --cc=gnoack@google.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox