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
next prev 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