Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.18-5.10] fbdev: udlfb: validate vendor descriptor items
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:47 ` Sasha Levin
  2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] fbdev: maxine: fix 64-bit build error Sasha Levin
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-10-19 13:47 UTC (permalink / raw)
  To: patches, stable
  Cc: Pengpeng Hou, Helge Deller, Sasha Levin, bernie, linux-fbdev,
	dri-devel, linux-kernel

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

[ Upstream commit 3cc2fad376ac7360b4518277aa923752624db59c ]

dlfb_parse_vendor_descriptor() walks key-length-value items inside the
DisplayLink vendor descriptor.

Require each item to contain its key, length and declared value bytes
before reading item-specific fields such as max_area.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

This is a USB descriptor bounds-check for `udlfb`. The buggy parser is
in this 6.12.111 tree, the patch applies cleanly, and the failure mode
is an out-of-bounds read from device-controlled data.

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[fbdev/udlfb] [validate] [require each vendor-descriptor TLV
item to contain key, length, and declared value bytes before reading
fields such as max_area]**

**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- Signed-off-by: Pengpeng Hou \<pengpeng@iscas.ac.cn\> (author)
- Signed-off-by: Helge Deller \<deller@gmx.de\> (fbdev maintainer;
  applied)
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, Link:,
  or Cc: stable tags (absence is expected for this review)
- Ignore any pipeline “Upstream commit …” / Sasha Levin SOB markers;
  they are not evidence of selection

**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- Bug: `dlfb_parse_vendor_descriptor()` walks DisplayLink vendor-
  descriptor key-length-value items without checking that key, length,
  and value bytes are still inside the descriptor.
- Symptom/failure mode: out-of-bounds read of `max_area` (and of the
  next key/length) when the remaining buffer is shorter than the
  declared item.
- Version information: none in the message. The parser has been in this
  file for many years (see Phase 3).
- Root cause: missing remaining-length checks before reading item
  fields.

**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not disguised. “Validate” is an explicit bounds-check fix for a
USB descriptor parser. It is not cleanup, a new feature, or refactor-
only.

## PHASE 2: DIFF ANALYSIS

**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/video/fbdev/udlfb.c` — **+16 / −6** (22-line change)
- Function: `dlfb_parse_vendor_descriptor()`
- Scope: single-file surgical fix

**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Current code in this tree (`drivers/video/fbdev/udlfb.c` around the
parse loop):

```1588:1612:drivers/video/fbdev/udlfb.c
                while (desc < desc_end) {
                        u8 length;
                        u16 key;

                        key = *desc++;
                        key |= (u16)*desc++ << 8;
                        length = *desc++;

                        switch (key) {
                        case 0x0200: { /* max_area */
                                u32 max_area = *desc++;
                                max_area |= (u32)*desc++ << 8;
                                max_area |= (u32)*desc++ << 16;
                                max_area |= (u32)*desc++ << 24;
                                ...
                        }
                        ...
                        desc += length;
                }
```

Record, per hunk:
- Before: if `desc` is within `desc_end`, it always consumes 3 bytes
  (key+length) and, for key `0x0200`, 4 more value bytes, with no
  remaining-length test. Then it still does `desc += length`.
- After: require at least 3 bytes left; parse key with
  `get_unaligned_le16()`; reject if `length` exceeds remaining bytes;
  for `max_area`, require `length >= 4` and read via
  `get_unaligned_le32(value)` without advancing past the value twice;
  then `desc += length`.
- Path: USB probe / firmware descriptor parse (initialization), not a
  hot path.

**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- Category: **memory safety / out-of-bounds read** of USB descriptor
  data (also a logic correction of the 2018 max_area pointer walk).
- Mechanism: a truncated or hostile descriptor can make `desc_end -
  desc` smaller than 3 (key+length) or smaller than `length` /
  `sizeof(u32)`. The old loop still reads `key`, `length`, and
  `max_area`. The fix stops at `unrecognized` instead of reading past
  the buffer. `unrecognized` already exists and still returns true
  (driver still loads with the default SKU limit).

**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Quality: obviously correct TLV remaining-length checks;
  `get_unaligned_le16/le32` matches USB little-endian layout and the
  pre-2018 `le16_to_cpu`/`le32_to_cpu` reads. `linux/unaligned.h` is
  already included.
- Regression risk: low. Failures take the existing `unrecognized` path,
  which logs and still succeeds. The kernel is built with `-funsigned-
  char`, so switching from byte-or-shifts to `get_unaligned_le*` is not
  a signed-char behavior change. Restoring “read value, then `desc +=
  length`” matches the original 2010 parser and the DRM `udl` parser in
  this tree.

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Loop introduced in `18dffdf8913ab9` (“staging: udlfb: enhance EDID and
  mode handling support”, 2010-09-05). Original max_area read did
  **not** advance `desc` before `desc += length`.
- Current byte-by-byte form introduced by `115e77597efcc9` (“video:
  udlfb: Fix unaligned access”, 2018-03-12), which is an ancestor of
  this HEAD. That commit replaced `le16_to_cpu`/`le32_to_cpu` with
  `*desc++` and accidentally advanced `desc` through the 4-byte value
  **and** still did `desc += length`.
- Bug class is old (2010 missing bounds; 2018 worse walk). Present in
  6.12.

**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No Fixes: tag. Not applicable beyond that. The intended target
is the long-standing parser, last reshaped by `115e77597efcc9`, which
**is** in this tree.

**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Recent udlfb commits in this tree are independent (`add
vm_ops…`, `avoid divide-by-zero…`). This is a standalone v1 patch, not
“X/Y”. No prerequisite refactor is required.

**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Pengpeng Hou has no other udlfb commits in this tree. Helge
Deller (fbdev maintainer) applied it. Related DRM work: `df6dc12e5e6dd`
(“drm/udl: Validate length in vendor-descriptor parser”) on
`origin/master` is the sibling hardening of the same protocol; it is
**not** an ancestor of this HEAD.

**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. Uses existing `get_unaligned_le16`/`get_unaligned_le32`
from `include/linux/unaligned.h` (already included as
`<linux/unaligned.h>`). `git apply --check` of `3cc2fad376ac7` on this
tree succeeded. Standalone.

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- Commit on `origin/master`: `3cc2fad376ac7360b4518277aa923752624db59c`
  (not in this HEAD).
- `b4 dig -c 3cc2fad376ac7`:
  https://patch.msgid.link/20260706093038.80131-1-pengpeng@iscas.ac.cn
- `b4 dig -a`: **v1 only** (no v2/v3). Applied version is that v1.
- Maintainer Helge Deller first asked whether LE reads were valid, then
  retracted: “Your patch is of course correct! The patch is now added to
  fbdev git tree.”
- No NAK. No reviewer request for Cc: stable (not a negative).
- An automated Sashiko note flagged a **pre-existing** `%11ph` 11-byte
  dump when `total_len` can be 6; that is **not** introduced or fixed
  here.

**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): Pengpeng Hou; Bernie Thompson; Helge Deller;
linux-fbdev; dri-devel; linux-kernel. Subsystem maintainer was on Cc and
applied the patch.

**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla tag. Same bug class as DRM udl: Thomas
Zimmermann’s `drm/udl` series (lore, April 2025) states the current
parser “might read more bytes than available” / “out-of-bounds reads of
the allocated buffer.” That is independent confirmation of the protocol
bug, not a userspace crash report for udlfb.

**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone single patch. No later origin/master follow-up on
`udlfb.c` after `3cc2fad376ac7`. DRM equivalent is a larger 2025
refactor (`udl_parse_key_value_pair`); not a dependency of this fbdev
patch.

**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: WebFetch of lore.kernel.org/stable returned 403. **UNVERIFIED**
whether stable@ discussed this subject. No in-tree evidence of a prior
rejection. The commit is simply not in this 6.12 checkout.

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `dlfb_parse_vendor_descriptor()` only.

**Step 5.2: TRACE CALLERS**
Record: Sole caller is `dlfb_usb_probe()`
(`drivers/video/fbdev/udlfb.c`). Bound via `.probe = dlfb_usb_probe` for
USB VID `0x17e9` + vendor-specific interface. Context: process-context
USB probe, not IRQ.

**Step 5.3: TRACE CALLEES**
Record: `usb_get_descriptor()`, `usb_get_extra_descriptor()`,
`dev_info`/`dev_warn`/`dev_err`, `kfree`. New: `get_unaligned_le16`,
`get_unaligned_le32`. No new locks or allocations.

**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Plug in / enumerate a DisplayLink USB device (or a device
spoofing VID `0x17e9`) → `dlfb_usb_probe` → parser. Reachable without
extra syscalls beyond USB enumeration. Unprivileged userspace cannot
call it directly, but a physical USB device can.

**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: `drivers/gpu/drm/udl/udl_main.c` `udl_parse_vendor_descriptor()`
in this tree still has the unbounded walk (`desc += length` with no
remaining-length check). Same protocol bug. That does not block this
udlfb-only fix.

## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE

**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: Yes. Tree is **6.12.111** (`git describe HEAD` →
`p-6.12-3-gc504cf0baa3d8`; `VERSION/PATCHLEVEL/SUBLEVEL` 6.12.111).
Parser at `dlfb_parse_vendor_descriptor()` lines 1588–1612. `git merge-
base --is-ancestor 3cc2fad376ac7 HEAD` → **not in this tree**. Bug
present since 2010 / 2018, well before 6.12.

**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply.** `git show 3cc2fad376ac7 --
drivers/video/fbdev/udlfb.c | git apply --check` exit 0. Surrounding
function matches the patch context. `<linux/unaligned.h>` already
present.

**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: `git log --grep='validate vendor descriptor' --
drivers/video/fbdev/udlfb.c` empty. DRM `Validate length in vendor-
descriptor parser` is not in this HEAD. No alternate udlfb bounds fix
present.

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **fbdev USB framebuffer driver** (`CONFIG_FB_UDL`,
`drivers/video/fbdev/udlfb.c`). Criticality: **PERIPHERAL / driver-
specific**, but USB descriptor parsing is a standard untrusted-input
path. DisplayLink USB 2.0 adapters are real hardware; the Kconfig help
still describes this as the fbdev driver for those devices.

**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature. Parser largely unchanged since 2018 aside from later
mmap/div-by-zero fixes. Low churn favors a small bounds-check backport.

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Users with `CONFIG_FB_UDL` and a DisplayLink USB 2.0 device (or
a spoofed `0x17e9` device). Not universal.

**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Honest device with a well-formed descriptor: loop stays in-bounds
  (typical path).
- Truncated/malformed descriptor: OOB read of key/length/`max_area`.
- Malicious USB device matching the VID/class: can set `bLength` up to
  `MAX_VENDOR_DESCRIPTOR_SIZE` (256) so `desc_end` sits at the `kzalloc`
  edge, then emit a TLV `length` that walks off the buffer.
- Extra-descriptor fallback: `__usb_get_extra_descriptor()` bounds
  `bLength` to the extra blob, but item `length` can still push reads
  past that descriptor.
- Trigger is probe-time, not a timing race. Unprivileged software cannot
  hit it without USB device control.

**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **OOB read** of kernel memory during USB probe. Can KASAN-splat
or oops if the read crosses an allocation/page boundary (`total_len ==
256` is the sharp heap case). Severity: **HIGH** (crash / infoleak from
untrusted USB descriptor). Not a confirmed write, panic-on-boot for good
hardware, or filesystem corruption.

**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- Benefit: stops USB-descriptor OOB in a driver that is in this LTS
  tree; fail-open `unrecognized` path already exists.
- Risk: very low (16 insertions, existing helpers, existing error label,
  clean apply).
- Ratio: **favorable**.

## PHASE 9: FINAL SYNTHESIS

**Step 9.1: COMPILE THE EVIDENCE**

FOR:
- Real OOB read of device-controlled USB descriptor data
- Same class already called out for DRM udl (“out-of-bounds reads of the
  allocated buffer”)
- Small, surgical, obviously correct remaining-length checks
- Code and bug exist in this 6.12.111 tree; fix is not in it
- Applies cleanly; `get_unaligned_*` already available
- Maintainer applied v1 after confirming LE reads are correct
- `unrecognized` still allows probe (low chance of breaking real
  adapters)
- No new API/feature

AGAINST:
- Driver-specific (DisplayLink fbdev), not core MM/VFS
- No syzbot/CVE/user oops in the commit message
- Honest devices with valid descriptors do not hit the bug
- Pre-existing `%11ph` dump issue is unfixed (not a reason to reject
  this)

UNRESOLVED:
- lore.kernel.org/stable search HTTP 403
- Exact TLV `length` byte on every DisplayLink SKU was not captured from
  a live device in this session; parser semantics match 2010 + current
  DRM udl, which is enough

**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — bounds checks are local and
   reviewable; maintainer applied after LE confirmation.
2. Fixes a real bug that affects users? **PASS** — OOB read of USB
   descriptor memory; probe path for this driver.
3. Important issue? **PASS** — HIGH: crash/infoleak from untrusted USB
   descriptor (not a style cleanup).
4. Small and contained? **PASS** — one function, +16/−6.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — `git apply --check` clean on
   6.12.111

**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. It qualifies as a
normal small security/robustness fix.

**Step 9.4: DECISION**
Backport to **this** tree. The buggy parser is here; the fix is small
and applies; the impact is a USB OOB read.

Verification:
- [Phase 1] Parsed tags: author + Helge Deller SOB only; no
  Fixes/syzbot/Cc stable
- [Phase 2] Diff: +16/−6 in `dlfb_parse_vendor_descriptor()`; remaining-
  length checks + `get_unaligned_le16/le32`
- [Phase 2] Confirmed current 6.12 code still does unbounded `*desc++`
  key/length/max_area reads
- [Phase 3] Blame: loop from `18dffdf8913ab9` (2010); byte-walk from
  `115e77597efcc9` (2018), both ancestors of HEAD
- [Phase 3] `git merge-base --is-ancestor 3cc2fad376ac7 HEAD` → not
  present
- [Phase 3] `git apply --check` of `3cc2fad376ac7` → clean
- [Phase 4] `b4 dig -c 3cc2fad376ac7` → lore msgid
  `20260706093038.80131-1-pengpeng@iscas.ac.cn`; v1 only
- [Phase 4] Thread: Helge questioned LE, then “patch is of course
  correct” and applied; no NAK
- [Phase 4] Sibling DRM commit `df6dc12e5e6dd` documents the same OOB;
  not in this HEAD
- [Phase 5] Only caller: `dlfb_usb_probe` / VID `0x17e9`
- [Phase 5] `include/linux/unaligned.h` provides
  `get_unaligned_le16/le32`; already included
- [Phase 6] `git describe HEAD` = `p-6.12-3-gc504cf0baa3d8`; `make
  kernelversion` = 6.12.111
- [Phase 8] Failure mode: USB descriptor OOB read at probe; severity
  HIGH
- [Phase 8] Makefile has `-funsigned-char` (no signed-char regression
  from byte reads)
- UNVERIFIED: stable@ lore discussion (HTTP 403)

**YES**

 drivers/video/fbdev/udlfb.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
index 70ff38f35096a..1c82ee6984c8d 100644
--- a/drivers/video/fbdev/udlfb.c
+++ b/drivers/video/fbdev/udlfb.c
@@ -1586,19 +1586,29 @@ static int dlfb_parse_vendor_descriptor(struct dlfb_data *dlfb,
 		desc += 5; /* the fixed header we've already parsed */
 
 		while (desc < desc_end) {
+			char *value;
 			u8 length;
 			u16 key;
 
-			key = *desc++;
-			key |= (u16)*desc++ << 8;
+			if (desc_end - desc < sizeof(key) + sizeof(length))
+				goto unrecognized;
+
+			key = get_unaligned_le16(desc);
+			desc += sizeof(key);
 			length = *desc++;
 
+			if (length > desc_end - desc)
+				goto unrecognized;
+
+			value = desc;
 			switch (key) {
 			case 0x0200: { /* max_area */
-				u32 max_area = *desc++;
-				max_area |= (u32)*desc++ << 8;
-				max_area |= (u32)*desc++ << 16;
-				max_area |= (u32)*desc++ << 24;
+				u32 max_area;
+
+				if (length < sizeof(max_area))
+					goto unrecognized;
+
+				max_area = get_unaligned_le32(value);
 				dev_warn(&intf->dev,
 					 "DL chip limited to %d pixel modes\n",
 					 max_area);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 6.18-5.10] fbdev: maxine: fix 64-bit build error
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
  2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] fbdev: udlfb: validate vendor descriptor items Sasha Levin
@ 2026-10-19 13:47 ` Sasha Levin
  2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] fbdev: omap2: dsi: do not copy isr table Sasha Levin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-10-19 13:47 UTC (permalink / raw)
  To: patches, stable
  Cc: Randy Dunlap, Maciej W. Rozycki, Helge Deller, Sasha Levin,
	linux-fbdev, dri-devel, linux-kernel

From: Randy Dunlap <rdunlap@infradead.org>

[ Upstream commit 3adec5c640dc09dca5605043e138412fd5a0629d ]

The KSEG1ADDR() macro is only defined for non-64BIT builds.
Use the CKSEG1ADDR() macro instead.

In file included from ../drivers/video/fbdev/maxinefb.c:34:
../drivers/video/fbdev/maxinefb.c: In function 'maxinefb_ims332_write_register':
../include/video/maxinefb.h:16:41: error: implicit declaration of function 'KSEG1ADDR'; did you mean 'CKSEG1ADDR'? [-Wimplicit-function-declaration]
   16 | #define MAXINEFB_IMS332_ADDRESS         KSEG1ADDR(0x1c140000)
../drivers/video/fbdev/maxinefb.c:66:49: note: in expansion of macro 'MAXINEFB_IMS332_ADDRESS'
   66 |         register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;
../drivers/video/fbdev/maxinefb.c:66:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   66 |         register unsigned char *regs = (char *) MAXINEFB_IMS332_ADDRESS;

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Maciej W. Rozycki <macro@orcam.me.uk>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

This tree is **6.12.111** (`pending-6.12`). The buggy `KSEG1ADDR()` use
is still in `include/video/maxinefb.h`; the candidate is a two-line
build fix and belongs in this tree.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[fbdev/maxine] [fix] [64-bit build error]` — subsystem `fbdev:
maxine`, action verb **fix**, intent is to make the DECstation Maxine
framebuffer driver compile on 64-bit MIPS.

**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Signed-off-by: Randy Dunlap <rdunlap@infradead.org>` (author)
- `Acked-by: Maciej W. Rozycki <macro@orcam.me.uk>` (MIPS/DECstation
  expert)
- `Signed-off-by: Helge Deller <deller@gmx.de>` (fbdev maintainer)
- No `Fixes:` tag (expected for this review)
- No `Reported-by:`, `Tested-by:`, `Cc: stable`, or `Link:`
- Ignore any pipeline `Signed-off-by: Sasha Levin` / `[Upstream commit
  ...]`

**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `KSEG1ADDR()` is used in `include/video/maxinefb.h` but is
  only defined when `!CONFIG_64BIT` in
  `arch/mips/include/asm/addrspace.h`.
- **Symptom:** compile failure:
  `error: implicit declaration of function 'KSEG1ADDR'` plus `warning:
  cast to pointer from integer of different size`.
- **Root cause:** 32-bit-only KSEG macros used in a driver that Kconfig
  allows on 64-bit `MACH_DECSTATION`.
- **Fix:** use `CKSEG1ADDR()`, which exists for both 32-bit and 64-bit.
- **Version info:** none in the message; series cover says the code is
  “from the beginning of git time.”

**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. This is an explicit build-error fix, not a cleanup
in disguise. It is patch 03/03 of “fbdev: maxine: clean up build
issues”; patches 01/02 are warning/cleanup in `maxinefb.c` and are
independent of this header change.

---

## PHASE 2: DIFF ANALYSIS

**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `include/video/maxinefb.h` only
- Two `#define` substitutions: `KSEG1ADDR` → `CKSEG1ADDR`
- ~2 insertions / 2 deletions
- No functions added or rewritten; macros `MAXINEFB_IMS332_ADDRESS` and
  `DS5000_xx_ONBOARD_FBMEM_START`
- Scope: single-file surgical fix

**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- Before: both MMIO/FB base macros expand via `KSEG1ADDR()`, which does
  not exist under `CONFIG_64BIT`.
- After: same physical addresses mapped through `CKSEG1ADDR()`.
- Affected path: compile of `maxinefb.c` (register access and
  framebuffer base). Runtime on 32-bit is unchanged because the two
  macros are identical there.

**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- Category: **build / type correctness** (implicit declaration; 32-bit
  integer treated as a pointer on 64-bit).
- Mechanism: `KSEG1ADDR` is inside `#else /* !CONFIG_64BIT */`.
  `CKSEG1ADDR` is defined in both branches. On 32-bit, `CKSEG1ADDR(a)`
  is `(CPHYSADDR(a) | KSEG1)`, same as `KSEG1ADDR`. On 64-bit it uses
  `CKSEG1` (`0xffffffffa0000000`).

**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: compiler even suggested `CKSEG1ADDR`.
- Minimal; no API or struct changes.
- Regression risk: very low. 32-bit generated addresses match. 64-bit
  previously did not compile, so there is no working 64-bit runtime to
  regress.
- Acked by the DECstation/MIPS expert.

---

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1: BLAME THE CHANGED LINES**
Record: Both macros come from `1da177e4c3f41` (“Linux-2.6.12-rc2”,
2005). Present since git history began. `git merge-base --is-ancestor
1da177e4c3f41 HEAD` succeeds.

**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Not applicable. The broken use is the original
driver code, not a later regression.

**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record (this tree, path-limited):
- `e4f10ea729670 fbdev/maxinefb: Use fbdev I/O helpers`
- `6304da8a91da8 fbdev: Remove FBINFO_DEFAULT from static structs`
- `8c13c9de80b51 fbdev: maxinefb: Remove trailing whitespaces`
- plus older const/__initdata moves
None of these touch the `KSEG1ADDR` macros. Header last changed at git
genesis. Standalone vs series: submitted as 03/03, but this hunk does
not depend on 01/02.

**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Randy Dunlap has many fbdev build/Kconfig fixes in this tree
(`sh7760fb`, `hgafb`, `uvesafb`, `geode`, `atyfb`, …). No prior maxinefb
commit here. Helge Deller is the fbdev maintainer who merged it.

**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. `CKSEG1ADDR` has existed in `addrspace.h` for as long as
this tree’s MIPS headers. The patch applies to the current header as-is.
Cover letter patches 01/02 only change `maxinefb.c` (static, drop unused
`maxinefb_ims332_read_register`).

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c <sha>` cannot run: this commit is **not in the local
  tree**.
- `b4 dig` on Message-ID `20260813232934.2718291-4@infradead.org` also
  found no local commit.
- Original submission (13 Aug 2026): [PATCH 03/03] on linux-kernel;
  cover [PATCH 00/03] “fbdev: maxine: clean up build issues”. Randy:
  “Now builds cleanly on 32-bit and 64-bit MACH_DECSTATION.”
- Helge’s fbdev pull for v7.3-rc1 lists it under **Fixes**: “maxine: fix
  64-bit build error and code cleanups”.
- Review: Maciej Acked-by only; no NAK, no stable-tag request found in
  the retrieved thread text.

**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: To/Cc included Helge Deller, linux-fbdev, dri-devel, Maciej W.
Rozycki. Maciej acked. Helge merged.

**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla. Failure is a compiler error reproduced in
the commit message (typical Randy Dunlap / kbuild-style report).

**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Series of 3 (+ later two maxine cleanups in the same pull: init
return value, `MODULE_LICENSE()`). Only 03/03 is required to fix the
implicit-declaration **error**. 01/02 are warning-level.

**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search returned **403**. UNVERIFIED
whether stable@ already discussed this. Openwall/Indiana archives show
no stable nomination in the review replies retrieved.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: No functions modified. Macros used in
`maxinefb_ims332_write_register()`, `maxinefb_ims332_read_register()`,
and `maxinefb_init()`.

**Step 5.2: TRACE CALLERS**
Record:
- `MAXINEFB_IMS332_ADDRESS` / `DS5000_xx_ONBOARD_FBMEM_START`: only
  `drivers/video/fbdev/maxinefb.c`.
- `maxinefb_ims332_write_register`: `maxinefb_setcolreg()` and
  `maxinefb_init()` (cursor RAM wipe).
- `maxinefb_ims332_read_register`: defined, **never called** in this
  tree (that is series patch 02).
- `maxinefb_init`: `module_init`. Probe path after `mips_machtype ==
  MACH_DS5000_XX`.

**Step 5.3: TRACE CALLEES**
Record: Direct volatile MMIO via the mapped KSEG1/CKSEG1 addresses;
`register_framebuffer()` in init. No new callees.

**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable at boot on Personal DECstation 5000/xx with
`CONFIG_FB_MAXINE=y`. The **compile** failure is reachable for any
`MACH_DECSTATION` + `64BIT` + `FB_MAXINE` config, including 64-bit R4x00
DECstation kernels that can still select this bool (Kconfig does not
restrict it to R3000). Userspace does not trigger the compile error.

**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Other DECstation code already uses `CKSEG1ADDR` (e.g.
`drivers/net/ethernet/amd/declance.c`). Remaining `KSEG1ADDR` in fbdev
is `au1100fb.c` (Alchemy, 32-bit-oriented). This is the only use in
`include/video/`.

---

## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE

**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `include/video/maxinefb.h` lines 16 and 22 still use
`KSEG1ADDR`. `git describe HEAD` → `p-6.12-3-gc504cf0baa3d8`; `make
kernelversion` → **6.12.111**. Introduced at 2.6.12-rc2 genesis; present
for the entire 6.12 life.

**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply.** Local header matches the patch context exactly.
`CKSEG1ADDR` is already defined for both 32- and 64-bit in this tree’s
`addrspace.h`.

**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: `git log --grep="fix 64-bit build error" --
include/video/maxinefb.h` is empty. No equivalent fix in this checkout.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: `drivers/video/fbdev` + `include/video` — MIPS DECstation
onboard framebuffer. **PERIPHERAL** (vintage workstation). Still a real
in-tree driver, not staging.

**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: maxinefb itself is dormant (whitespace/helpers only).
MACH_DECSTATION is mature. fbdev still receives small fixes (this series
landed in the v7.3 fbdev pull).

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Config-specific: `CONFIG_MACH_DECSTATION` && `CONFIG_FB_MAXINE`
&& `CONFIG_64BIT`. 32-bit Maxine builds already compile. 64-bit
DECstation + this driver cannot build today.

**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Trigger is **building** that config, every time. Not a race.
Unprivileged userspace is irrelevant. Valid Kconfig: `MACH_DECSTATION`
selects `SYS_SUPPORTS_64BIT_KERNEL`; `CPU_R4X00` selects
`CPU_SUPPORTS_64BIT_KERNEL`; `FB_MAXINE` depends only on `FB=y &&
MACH_DECSTATION`.

**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Build error** — driver (and thus that kernel config) does not
compile. Local `Documentation/process/stable-kernel-rules.rst` lists “a
**build error** (but not for things marked CONFIG_BROKEN)” as a
qualifying bug. `FB_MAXINE` is not `BROKEN`. Implicit declaration is a
hard GCC error, independent of `CONFIG_WERROR` (which defaults to
`COMPILE_TEST` in this tree).

**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** 64-bit MACH_DECSTATION + Maxine fb builds again; 32-bit
  behavior unchanged.
- **Risk:** very low (2 equivalent-on-32-bit macro lines, expert ack).
- Ratio: favorable. Niche hardware, but build-break fixes are in-policy
  even for small drivers.

---

## PHASE 9: FINAL SYNTHESIS

**Step 9.1: COMPILE THE EVIDENCE**

FOR:
- Real compile failure with a pasted compiler log
- `KSEG1ADDR` confirmed undefined under `CONFIG_64BIT` in this tree
- `CKSEG1ADDR` is the portable replacement; 32-bit expansion is
  identical
- Two-line, single-file, applies cleanly
- Buggy code present since 2.6.12, still in 6.12.111
- Acked by Maciej W. Rozycki; merged by Helge Deller
- Stable rules in this tree explicitly allow build-error fixes
- Standalone vs the rest of the series

AGAINST:
- Very small user population (1990s Personal DECstation)
- Maxine silicon is R3000-class; 64-bit+FB_MAXINE is a Kconfig-legal
  combo more than a common production image
- No `Cc: stable` (not a negative per the review rules)

UNRESOLVED:
- Exact mainline SHA (commit not in this checkout)
- `b4 dig` / lore.kernel.org 403
- Whether stable@ already discussed it

**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — compiler suggestion, Maciej
   ack, 32-bit equivalence verified in `addrspace.h`
2. Fixes a real bug that affects users? **PASS** — cannot build a valid
   64-bit DECstation config with this driver
3. Important issue? **PASS** — build error, listed in this tree’s
   stable-kernel-rules
4. Small and contained? **PASS** — 2 lines, one header
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — identical context,
   prerequisite macro already present

**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **Build fix** — qualifies under the documented exception /
“build error” clause.

**Step 9.4: MAKE YOUR DECISION**
Backport. The bug exists here, the fix is the standard MIPS portable
mapping, and the stable rules call out build errors. Niche hardware does
not outweigh a zero-risk compile fix that applies cleanly.

---

### What the commit solves for this tree
On 6.12, `include/video/maxinefb.h` still maps IMS332 MMIO and onboard
FB memory with `KSEG1ADDR()`. That macro is compiled out for
`CONFIG_64BIT`, while `FB_MAXINE` can still be enabled on
`MACH_DECSTATION` 64-bit kernels. The result is an implicit-declaration
error and a 32-vs-64 pointer-cast warning. `CKSEG1ADDR()` is already the
32/64-safe helper used elsewhere on DECstation.

### Stable-rules fit
Obviously correct, two lines, no new API, no prerequisites missing in
6.12, not `CONFIG_BROKEN`.

### Risk vs benefit
Benefit is restoring a previously impossible 64-bit DECstation+maxinefb
build. Risk on 32-bit Maxine (the real hardware) is nil because the
macros match.

### Concerns
None that block backport: series mates 01/02 are not required; header
matches; `CKSEG1ADDR` exists.

---

Verification:
- [Phase 1] Parsed subject/tags/body: fix 64-bit build; SOB Randy
  Dunlap, Ack Maciej, SOB Helge Deller; no Fixes/Reported-by/Cc:stable
- [Phase 2] Diff: two `KSEG1ADDR` → `CKSEG1ADDR` in
  `include/video/maxinefb.h`
- [Phase 2] `addrspace.h`: `KSEG1ADDR` only in `#else` of
  `CONFIG_64BIT`; `CKSEG1ADDR` in both; 32-bit expansions identical
- [Phase 3] `git blame include/video/maxinefb.h`: lines from
  `1da177e4c3f41` (2.6.12-rc2)
- [Phase 3] `git merge-base --is-ancestor 1da177e4c3f41 HEAD`: yes
- [Phase 3] `git log --oneline -- include/video/maxinefb.h`: only
  genesis; no later equivalent fix
- [Phase 3] No `Fixes:` tag to follow
- [Phase 4] Cover + 03/03 on lists.openwall.net (2026-08-13); Maciej Ack
  on lkml.indiana.edu 2608.2/02341
- [Phase 4] Helge GIT PULL lists this under Fixes for v7.3-rc1
- [Phase 4] `b4 dig`: no local commit (candidate not in tree);
  lore.kernel.org 403
- [Phase 5] Macros only used from `maxinefb.c`; `FB_MAXINE` depends on
  `MACH_DECSTATION` only
- [Phase 5] `MACH_DECSTATION` selects `SYS_SUPPORTS_64BIT_KERNEL`;
  `CPU_R4X00` selects `CPU_SUPPORTS_64BIT_KERNEL`
- [Phase 6] `git describe HEAD` = `p-6.12-3-gc504cf0baa3d8`; `make
  kernelversion` = 6.12.111
- [Phase 6] Local file still has `KSEG1ADDR`; patch context matches
- [Phase 7] Peripheral fbdev driver, not staging
- [Phase 8] Failure: compile error; this tree’s stable-kernel-rules.rst
  line 19 includes “a build error”
- [Phase 8] `CONFIG_WERROR` defaults to `COMPILE_TEST`; implicit
  declaration is still a GCC error without it
- UNVERIFIED: exact upstream SHA; stable@ archives (lore 403); whether
  anyone already requested this for 6.12.y

**YES**

 include/video/maxinefb.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/video/maxinefb.h b/include/video/maxinefb.h
index 6aeb4acca2bdb..2b66b32f355cf 100644
--- a/include/video/maxinefb.h
+++ b/include/video/maxinefb.h
@@ -13,13 +13,13 @@
 /*
  * IMS332 video controller register base address
  */
-#define MAXINEFB_IMS332_ADDRESS		KSEG1ADDR(0x1c140000)
+#define MAXINEFB_IMS332_ADDRESS		CKSEG1ADDR(0x1c140000)
 
 /*
  * Begin of DECstation 5000/xx onboard framebuffer memory, default resolution
  * is 1024x768x8
  */
-#define DS5000_xx_ONBOARD_FBMEM_START	KSEG1ADDR(0x0a000000)
+#define DS5000_xx_ONBOARD_FBMEM_START	CKSEG1ADDR(0x0a000000)
 
 /*
  *      The IMS 332 video controller used in the DECstation 5000/xx series
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 6.18-5.10] fbdev: omap2: dsi: do not copy isr table
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
  2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] fbdev: udlfb: validate vendor descriptor items Sasha Levin
  2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] fbdev: maxine: fix 64-bit build error Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
  2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] fbcon: Fix KASAN slab-out-of-bounds Read in fbcon_prepare_logo Sasha Levin
  2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] fbdev: platinumfb: add error checking for ioremap calls Sasha Levin
  4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
  To: patches, stable
  Cc: Andreas Kemnade, sashiko-bot, Helge Deller, Sasha Levin,
	linux-omap, linux-fbdev, dri-devel, linux-kernel

From: Andreas Kemnade <andreas@kemnade.info>

[ Upstream commit 31875c51c31edec57b3620f278680c23a823763b ]

To te able to unregister stuff from isrs, the corresponding table was
copied.  Nobody seems to unregister stuff that way, so it does not help.
But there are stack-allocated objects passed to these isrs giving chances
of UAF of these objects if irqs are unregistered while they are handled,
so better do not copy that table.

Suggested-by: sashiko-bot@kernel.org
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

This is a UAF fix in the OMAP2 fbdev DSI IRQ path. The same change is
already in this 6.12 tree for the DRM copy of the driver; the fbdev copy
of the bug is still present.

===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================

**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[fbdev/omap2/dsi] [do not copy] [stop copying the DSI ISR table
before invoking handlers]`

**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Suggested-by: sashiko-bot@kernel.org` (static-analysis bot; this is
  the original trigger, not a user crash report)
- `Signed-off-by: Andreas Kemnade <andreas@kemnade.info>` (author)
- `Signed-off-by: Helge Deller <deller@gmx.de>` (fbdev maintainer)
- No `Fixes:` tag (expected for this review queue)
- No `Cc: stable@vger.kernel.org`
- No `Reported-by:`, `Tested-by:`, `Reviewed-by:`, or `Link:`
- Ignore `Signed-off-by: Sasha Levin` / `[Upstream commit ...]` if
  present in candidate packaging; they are not part of the technical
  decision

**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- Bug: ISR table was memcpy’d so handlers could unregister themselves,
  then the copy was walked after dropping `irq_lock`.
- Author claim: nothing unregisters from ISR context, so the copy is
  unused for its stated purpose.
- Failure mode: stack-allocated objects (`DECLARE_COMPLETION_ONSTACK`,
  `dsi_packet_sent_handler_data` on stack) are passed as ISR `arg`. If
  unregister/return happens while the IRQ handler still walks the copy,
  those objects are used after free.
- No kernel-version range in the message.
- Root cause: lockless copy of function+arg pointers that alias stack
  objects.

**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. It is explicitly a UAF/race fix, even
though the subject does not say “fix”.

===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================

**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/video/fbdev/omap2/omapfb/dss/dsi.c` (+1 / −8; 9 lines)
- Removes `struct dsi_isr_tables isr_tables_copy` from `struct dsi_data`
- In `omap_dsi_irq_handler()`: drop `memcpy`, call `dsi_handle_isrs()`
  on the live table **while `irq_lock` is held**, then unlock
- Scope: single-file surgical fix

**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- Before: lock → read/ack IRQ status → memcpy table → unlock →
  `dsi_handle_isrs(&copy)` → error/stats
- After: lock → read/ack IRQ status → `dsi_handle_isrs(&live table)` →
  unlock → error/stats
- Affected path: hardirq handler (`devm_request_irq(...,
  omap_dsi_irq_handler, IRQF_SHARED, ...)`)
- Unregister (`dsi_unregister_isr*`) takes `irq_lock`, so it now waits
  until handlers finish

**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- Category: use-after-free / race (memory safety)
- Mechanism: copy of ISR `arg` pointers outlives unregister of stack-
  allocated completions
- Verified stack objects passed into ISRs in this tree:
  - `dsi_sync_vc_vp()` / `dsi_sync_vc_l4()`:
    `DECLARE_COMPLETION_ONSTACK` + stack `dsi_packet_sent_handler_data`
  - `dsi_vc_send_bta_sync()`: stack completion registered on **two**
    tables (global + VC)
  - `dsi_enter_ulps()`: stack completion registered on CIO table
- Concrete UAF in `dsi_vc_send_bta_sync()`: `dsi_handle_isrs()` walks
  global table then VC table. First handler can `complete()`; waiter
  unregisters and returns; second handler still runs against the copy
  with a dangling stack pointer.
- Fix: keep `irq_lock` across handler dispatch so unregister cannot
  destroy the objects until handlers return.

**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct given the locking: unregister cannot proceed while
  handlers run.
- Minimal; no API change.
- Regression risk is low **if** no ISR unregisters itself (would
  deadlock on `irq_lock`). Verified all three callbacks
  (`dsi_completion_handler`, `dsi_packet_sent_handler_vp`,
  `dsi_packet_sent_handler_l4`) only `complete()` and/or read registers.
  None call `dsi_unregister_isr*`.
- `complete()` under a spinlock is valid.
- Holding the lock slightly longer in hardirq is register reads +
  `complete()`, not a sleep.

===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================

**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Copy logic and `isr_tables_copy` blamed to `f76ee892a99e6` (“omapfb:
  copy omapdss & displays for omapfb”, 2015-12-29).
- Pattern is as old as the fbdev DSS copy in this tree; originally
  inherited from OMAP DSS ISR support.
- Long-lived bug, not recent code.

**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` on the fbdev commit. The sibling DRM fix
`2c27a84ba8d1b` (`drm/omap: dsi: Do not copy isr table`) has `Fixes:
4ae2ddddf44cd ("OMAP: DSS2: DSI: Add ISR support")`. That original ISR
support is what fbdev copied.

**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Recent `dsi.c` changes in this tree are unrelated (platform remove,
  of_graph, timer, pm_runtime).
- Standalone; not patch N/M.
- Cover letter on lore points at a DRM twin:
  `https://lore.kernel.org/lkml/20260702-dsi-
  uaf-v2-1-dbb4aa0f0b8e@kemnade.info/`
- That DRM twin **is already in this tree** as `2c27a84ba8d1b`
  (contained in `v6.12.110~477`).

**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record:
- Andreas Kemnade is an OMAP/display contributor; same author wrote the
  DRM twin.
- Helge Deller (fbdev maintainer) applied the fbdev patch.
- Tomi Valkeinen applied the DRM twin.

**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- No new functions/types.
- Applies as a logic change on existing `omap_dsi_irq_handler` / `struct
  dsi_data`.
- Context mismatch only: mainline hunk context has `timer_delete()`;
  this tree still has `del_timer()` (`8fa7292fee5c5` timer rename is not
  in 6.12). Not a functional dependency.

===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================

**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 31875c51c31ed`: https://patch.msgid.link/20260702-fbomap-
  uaf-fix-v1-1-6417edf9d3e7@kemnade.info
- `b4 dig -a`: single revision **v1** only; that version was applied.
- Thread: author patch → sashiko-bot review → author reply → Helge
  Deller “applied.”
- No NAK. Deller accepted it.
- Bot comments were about **pre-existing sibling bugs** (DRM DSI, DISPC
  copy, unbind timers), not about this change being wrong.
- Author noted the DRM variant in the patch footer.

**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`):
- To: Helge Deller
- Cc: linux-omap, linux-fbdev, dri-devel, linux-kernel, sashiko-bot
- Appropriate lists; fbdev maintainer applied it.
- No human Reviewed-by besides maintainer application.

**Step 4.3: SEARCH FOR THE BUG REPORT**
Record:
- No syzbot/bugzilla user report. Trigger is sashiko-bot static
  analysis.
- Bug is still real: code in this tree matches the described UAF (stack
  completions + lockless copy).

**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record:
- DRM twin already backported here (`2c27a84ba8d1b`).
- fbdev DISPC still copies a stack `registered_isr` in `dispc-compat.c`
  (`omap_dispc_irq_handler`); same pattern, **separate** file, not
  required for this DSI patch to be correct.
- No revert of either DSI fix on `origin/master`.

**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: UNVERIFIED. `lore.kernel.org/stable/` and lore message URLs
returned HTTP 403. Thread was recovered via `b4 dig -m` instead. No
stable-list discussion could be fetched.

===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================

**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `omap_dsi_irq_handler()`; field removed from `struct dsi_data`.
Callers of `dsi_handle_isrs()` / `dsi_call_isrs()` unchanged except the
table pointer and lock scope.

**Step 5.2: TRACE CALLERS**
Record:
- `omap_dsi_irq_handler` ← `devm_request_irq` in DSI bind/probe (`dsi.c`
  ~5363). Hardirq, `IRQF_SHARED`.
- Register/unregister sites are process context, all take `irq_lock`.
- Sync/BTA/ULPS paths used during DSI frame update, command write, and
  ULPS enter (`dsi_sync_vc` from update/disable; `dsi_vc_send_bta_sync`
  as `.bta_sync`; `dsi_enter_ulps`).

**Step 5.3: TRACE CALLEES**
Record: `dsi_handle_isrs` → `dsi_call_isrs` → registered `isr(arg,
status)` → `complete()` / `REG_GET`. Then `dsi_handle_irq_errors` and
`dsi_collect_irq_stats` still after unlock.

**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable from normal display I/O on `CONFIG_FB_OMAP2=y` +
`CONFIG_FB_OMAP2_DSS_DSI=y` (MIPI DSI panel). Not a syscall fuzzer path,
but a real device IRQ + wait-for-completion path. Unprivileged userspace
can trigger it by updating the framebuffer on such hardware.

**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record:
- DRM `drivers/gpu/drm/omapdrm/dss/dsi.c` in **this tree** already calls
  `dsi_handle_isrs(&dsi->isr_tables, ...)` under `irq_lock` (the
  backported twin).
- fbdev `dispc-compat.c` still memcpy’s ISRs then drops the lock;
  leftover sibling, not a blocker for this patch.
- `isr_tables_copy` exists only in this fbdev `dsi.c` in this checkout.

===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================

**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- `git describe HEAD`: `p-6.12-3-gc504cf0baa3d8`
- `make kernelversion` / Makefile: **6.12.111**
- `git merge-base --is-ancestor 31875c51c31ed HEAD` → not an ancestor
  (fix not in this tree)
- Buggy code present: `isr_tables_copy` at `dsi.c:330`, memcpy at
  `dsi.c:841-848`
- Introduced here via `f76ee892a99e6` (v4.5-era copy)

**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record:
- `git apply --check` of `31875c51c31ed` fails only because context has
  `timer_delete` vs local `del_timer`.
- The two functional hunks (remove field; move `dsi_handle_isrs` before
  unlock) match this tree byte-for-byte aside from that nearby timer
  name.
- Expected difficulty: **minor context tweak**, not rework.

**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record:
- DRM equivalent already applied (`2c27a84ba8d1b`).
- No fbdev DSI equivalent. `FB_OMAP2` depends on `DRM_OMAP=n`, so omapfb
  users do **not** get the DRM fix.

===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================

**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: `drivers/video/fbdev/omap2` — SoC display driver. Criticality:
**PERIPHERAL** (OMAP2+ boards using legacy fbdev, not omapdrm). Still
in-tree and user-facing on that hardware.

**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature/legacy. Low churn; bug has been present since the 2015
omapdss copy. Maintainer (Deller) still takes fixes.

===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================

**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Driver-specific / config-specific: `CONFIG_FB_OMAP2` +
`CONFIG_FB_OMAP2_DSS_DSI` on OMAP2+ with MIPI DSI, and only when DRM
omap is disabled.

**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Trigger: DSI IRQ concurrent with unregister of a stack-backed ISR
  after `complete()` or timeout (`dsi_sync_vc_*` 10 ms, BTA 500 ms, ULPS
  1000 ms).
- Dual-ISR BTA path is the most realistic (two handlers, one stack
  object).
- Unprivileged userspace can drive framebuffer updates that hit
  `dsi_sync_vc`.

**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: Use-after-free of stack completions / handler-data → kernel oops
or memory corruption in hardirq. Severity: **HIGH**.

**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- Benefit: HIGH for omapfb+DSI users (prevents UAF in IRQ). Population
  is small, but the DRM twin was already taken into this same 6.12.y
  tree, which shows the bug class is treated as stable-worthy.
- Risk: LOW (9 lines, verified no self-unregister, `complete()` under
  spinlock is fine).
- Ratio: benefit outweighs risk.

===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================

**Step 9.1: COMPILE THE EVIDENCE**

FOR:
- Real UAF: lockless copy of pointers to `DECLARE_COMPLETION_ONSTACK`
  objects
- Dual-ISR BTA path makes the race practical, not only a 1-instruction
  window
- Small, obviously correct lock-scope change
- All ISR callbacks verified not to unregister (no self-deadlock)
- Code exists in this 6.12.111 tree; bug dates to 2015 copy
- fbdev maintainer applied v1 with no NAK
- Identical DRM fix already in this tree; omapfb users are the leftover
  affected set

AGAINST:
- No user `Reported-by` / syzbot (bot-found)
- Peripheral OMAP fbdev DSI, not a core path
- Vanilla patch needs a one-line context adjust (`del_timer`)
- DISPC has a similar leftover pattern (out of scope for this commit)

UNRESOLVED:
- Could not query lore.kernel.org/stable (HTTP 403)
- No runtime reproduction in this session (analysis is from code +
  thread)

**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — lock documents the intent;
   maintainer applied; DRM twin already on 6.12.y
2. Fixes a real bug that affects users? **PASS** — UAF on DSI IRQ vs
   stack completions
3. Important issue? **PASS** — UAF / oops / memory corruption (HIGH)
4. Small and contained? **PASS** — 9 lines, one file
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — functional diff matches;
   trivial `del_timer` context

**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not an ID/quirk/DT/build/docs exception. It is a normal memory-
safety bugfix.

**Step 9.4: MAKE YOUR DECISION**
This should be backported to **this** 6.12.y tree. The buggy
`isr_tables_copy` path is here; the fix is the same one already taken
for omapdrm; omapfb is a separate config and still needs it.

Verification:
- [Phase 1] Tags: Suggested-by sashiko-bot; SOB Kemnade + Deller; no
  Fixes/Cc stable/Reported-by
- [Phase 2] Diff: remove `isr_tables_copy`; call
  `dsi_handle_isrs(&dsi->isr_tables)` before `spin_unlock`
- [Phase 2] Confirmed stack completions at dsi.c:2298, 2347, 2617, 3115
- [Phase 2] Confirmed ISR callbacks only complete/read MMIO; no
  unregister
- [Phase 3] Blame: f76ee892a99e6 (2015); ancestor check of 31875c51c31ed
  vs HEAD: not present
- [Phase 3] DRM twin 2c27a84ba8d1b is in this tree (`git merge-base
  --is-ancestor` exit 0; `v6.12.110~477`)
- [Phase 4] b4 dig: lore msgid 20260702-fbomap-uaf-
  fix-v1-1-6417edf9d3e7@kemnade.info; v1 only; Deller “applied.”
- [Phase 4] UNVERIFIED: stable@ lore search (HTTP 403)
- [Phase 5] Handler installed via `devm_request_irq` + `IRQF_SHARED`;
  sync/BTA/ULPS are the users
- [Phase 6] Local tree 6.12.111; buggy memcpy still at dsi.c:841-848;
  apply --check fails only on `timer_delete` vs `del_timer`
- [Phase 7] Peripheral fbdev OMAP DSI; mutually exclusive with DRM_OMAP
- [Phase 8] Failure mode: hardirq UAF of stack object, severity HIGH

The patch is a small UAF fix that already exists for the DRM clone in
this tree, applies with a trivial context tweak, and matches stable
rules.

**YES**

 drivers/video/fbdev/omap2/omapfb/dss/dsi.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
index 370e8623754ec..b1df2c014b00c 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
@@ -326,8 +326,6 @@ struct dsi_data {
 
 	spinlock_t irq_lock;
 	struct dsi_isr_tables isr_tables;
-	/* space for a copy used by the interrupt handler */
-	struct dsi_isr_tables isr_tables_copy;
 
 	int update_channel;
 #ifdef DSI_PERF_MEASURE
@@ -838,15 +836,10 @@ static irqreturn_t omap_dsi_irq_handler(int irq, void *arg)
 		timer_delete(&dsi->te_timer);
 #endif
 
-	/* make a copy and unlock, so that isrs can unregister
-	 * themselves */
-	memcpy(&dsi->isr_tables_copy, &dsi->isr_tables,
-		sizeof(dsi->isr_tables));
 
+	dsi_handle_isrs(&dsi->isr_tables, irqstatus, vcstatus, ciostatus);
 	spin_unlock(&dsi->irq_lock);
 
-	dsi_handle_isrs(&dsi->isr_tables_copy, irqstatus, vcstatus, ciostatus);
-
 	dsi_handle_irq_errors(dsidev, irqstatus, vcstatus, ciostatus);
 
 	dsi_collect_irq_stats(dsidev, irqstatus, vcstatus, ciostatus);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 6.18-5.10] fbcon: Fix KASAN slab-out-of-bounds Read in fbcon_prepare_logo
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
                   ` (2 preceding siblings ...)
  2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] fbdev: omap2: dsi: do not copy isr table Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
  2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] fbdev: platinumfb: add error checking for ioremap calls Sasha Levin
  4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
  To: patches, stable
  Cc: Helge Deller, syzbot+0c815b25cdb3678e7083, Sasha Levin,
	tzimmermann, simona, dri-devel, linux-fbdev, linux-kernel

From: Helge Deller <deller@gmx.de>

[ Upstream commit 94e6a058b16820e02f25e1221a4c4e713ba23550 ]

Ensure the logo will not exceed the screen size, which then should
fix a reported KASAN: slab-out-of-bounds Read in fbcon_prepare_logo.

Reported-by: syzbot+0c815b25cdb3678e7083@syzkaller.appspotmail.com
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

The local tree is **6.12.111** (`pending-6.12`). The buggy `logo_lines`
walk is present here, and the candidate is a 7-line KASAN bounds check
that applies cleanly.

===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================

**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[fbcon] [Fix] [KASAN slab-out-of-bounds read in
fbcon_prepare_logo]**

**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- Reported-by: syzbot+0c815b25cdb3678e7083@syzkaller.appspotmail.com
- Signed-off-by: Helge Deller \<deller@gmx.de\> (FRAMEBUFFER LAYER
  maintainer; applied the patch himself)
- No Fixes:, no Cc: stable, no Reviewed-by/Acked-by/Tested-by, no Link:
  in the committed message (the syzbot-forwarded submission did include
  `Link: https://syzkaller.appspot.com/bug?extid=0c815b25cdb3678e7083`)
- Notable: syzbot + maintainer SOB; absence of Fixes/Cc:stable is
  expected for this review pipeline

**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- Bug: calculated logo height can exceed the console buffer, then
  `fbcon_prepare_logo()` reads past it.
- Symptom: KASAN slab-out-of-bounds Read (also KFENCE OOB and UAF-class
  reports on the same syzbot bug).
- Version info in the message: none. Syzbot shows crashes from Feb 2025
  through 2026 on upstream, and on linux-5.15.y / 6.1.y / 6.6.y.
- Root cause as stated: logo not clipped to screen size before the
  buffer walk.

**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a hidden fix. Explicit KASAN OOB fix. The later `if
(logo_lines > vc->vc_bottom)` path already tried to disable an oversized
logo, but only **after** the out-of-bounds walk.

===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================

**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/video/fbdev/core/fbcon.c` (+7 / −0)
- Function: `fbcon_prepare_logo()`
- Scope: single-file surgical fix

**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record: After `logo_lines = DIV_ROUND_UP(logo_height,
vc->vc_font.height)` the code immediately walked `q - logo_lines *
cols`. Now it clamps `logo_lines` to `rows` and `new_rows - 1`, and if
that leaves nothing, sets `logo_shown = FBCON_LOGO_DONTSHOW` and
returns. Normal path with a logo that already fits is unchanged.

**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **Memory safety / out-of-bounds.** `fb_prepare_logo()` returns a
pixel height `<= yres`, but `DIV_ROUND_UP(height, font_height)` can be
one larger than `yres / font_height` (`new_rows`). Example: yres=100,
font=16 → logo_lines=7, new_rows=6. Then `q - logo_lines * cols` is one
row before `vc_origin`, and `scr_readw` / `scr_memcpyw` go OOB. The
existing `logo_lines > vc->vc_bottom` check runs too late.

**Step 2.4: ASSESS THE FIX QUALITY**
Record: Obviously correct bounds clamp; 7 lines; no API change.
`new_rows == 0` yields `new_rows - 1 == -1`, then `logo_lines <= 0`
bails out. Regression risk is low: at worst the penguin is skipped on
tiny/odd geometries instead of walking off the buffer. A 2025 RFC (Wang
ShaoBo) described a related cols-mismatch after `vc_resize()`; this
clamp does not fully cover that variant, but it does not worsen it, and
syzbot named this commit as the fix for the 1873-crash report.

===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================

**Step 3.1: BLAME THE CHANGED LINES**
Record: The OOB walk (`q - logo_lines * cols` / `scr_readw` loop) is
from **^1da177e4c3f41** (v2.6.12 era). `logo_lines = DIV_ROUND_UP(...)`
is from **416e74ea781359** (Julia Lawall, 2008). This is long-standing
code, not a recent 6.12-only bug.

**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No Fixes: tag. The buggy pattern has been in the function for
~20 years. `6104c37094e7` ("fbcon: Make fbcon a built-time dependency
for fbdev", 2017) is in this tree (`merge-base --is-ancestor` exit 0);
Wang’s RFC cited it, but that is a different proposed patch.

**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Related but distinct history in this tree: `a6a00d7e8ffd7` /
`5c38052680f56` (kzalloc KMSAN false-positive, reverted);
`250038f5a7207` (2007 vertical overflow when making space for the logo —
different hunk). No prior clamp of `logo_lines` to `rows`/`new_rows`.
Standalone, not patch N/M.

**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Helge Deller is FRAMEBUFFER LAYER maintainer (`MAINTAINERS`). He
authors/applies fbdev fixes regularly in this tree.

**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Uses existing `logo_lines`, `rows`, `new_rows`, `logo_shown`,
`FBCON_LOGO_DONTSHOW`. No new helpers. `git apply --check` succeeded
(hunk at line 596, offset −64). Standalone.

===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================

**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record: `b4 dig -c 94e6a058b168` →
https://patch.msgid.link/6a897c8e.dbb3a75c.13dd47.002f.GAE@google.com
(syzbot-forwarded `#syz test` from deller@kernel.org, 2026-08-22). `b4
dig -a`: v1 only. lore.kernel.org returned 403; openwall mirror confirms
the same single-hunk patch. No NAK found. Applied two days later by
Deller (commit date 2026-08-24).

**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: `b4 dig -w`: recipients were syzbot and linux-
kernel@vger.kernel.org only. No public Reviewed-by. Weight comes from
the maintainer applying it, not from a review thread.

**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: https://syzkaller.appspot.com/bug?extid=0c815b25cdb3678e7083
- Status: upstream, C repro, **prio:high**, **1873 crashes**, first
  2025-02-25
- Fix commit listed: **94e6a058b168**
- Patched on 20/29 syzbot instances
- Stack: `store_bind` → `vt_bind` → `do_bind_con_driver` → `visual_init`
  → `fbcon_init` → `fbcon_prepare_logo` → `scr_memcpyw`
- Same title also on linux-5.15.y (C repro), linux-6.1.y, linux-6.6.y
- Syzbot security assessment: DenialOfService yes, Exploitable yes,
  Unprivileged no (sysfs `bind` is `S_IWUSR`)

**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Not a series. Related RFC 2025-09-25 from Wang ShaoBo added a
pointer-vs-`vc_origin` guard in the scroll loop; it was not applied in
this tree or in the candidate’s parent. Older kzalloc change is
unrelated (reverted KMSAN false positive).

**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable 403. No evidence this 2026-08-24 commit
was already discussed for 6.12.y. The same KASAN title is already
reproduced on older stable CIs, which supports that 6.12 is affected.

===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================

**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `fbcon_prepare_logo()` only. Semcode MCP not available.

**Step 5.2: TRACE CALLERS**
Record: Two call sites in this tree:
1. `con2fb_init_display()` (line 826) — console mapped to a framebuffer,
   `show_logo`.
2. `fbcon_init()` (line 1155) — after setting/resizing the VC, if `logo`
   is set.

`fbcon_init` is `con_init` for fbcon, called from `visual_init()` during
bind/takeover. `store_bind` is the syzbot trigger (root sysfs write).
Also runs on normal fbcon takeover at boot when the logo is enabled
(`!MODULE`).

**Step 5.3: TRACE CALLEES**
Record: `fb_prepare_logo()`, `scr_readw`/`scr_memcpyw`/`scr_memsetw`,
`kmalloc`/`kfree`, `update_screen`. The new code only uses `min()` and
existing `logo_shown`.

**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace (root) → `sysfs` `vtconN/bind` → `fbcon_init` →
`fbcon_prepare_logo`. Also boot-time logo on `CONFIG_FRAMEBUFFER_CONSOLE
&& !MODULE`. Path is real and reproduced.

**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: The too-late `logo_lines > vc->vc_bottom` check is the same bug
class, just after the OOB. No sibling drivers copy this logo-scroll
logic.

===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================

**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: Yes. `drivers/video/fbdev/core/fbcon.c` lines 597–604 compute
`logo_lines` and walk `q - logo_lines * cols` with no clamp. `git merge-
base --is-ancestor 94e6a058b168 HEAD` is **false** — the fix is not in
6.12.111. Bug introduced long before 6.12.

**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: `git apply --check` **clean** (offset −64 from surrounding-line
drift / `fbcon_ops`→`fbcon_par` rename already in this tree). Expected
difficulty: **clean apply**.

**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No `logo_lines = min(...)` in this file. The 2007 Geert overflow
tweak and the kzalloc revert do not clamp logo lines.

===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================

**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **fbcon** (`drivers/video/fbdev/core/fbcon.c`), the framebuffer
console used with `CONFIG_FRAMEBUFFER_CONSOLE` (default follows
`DRM_FBDEV_EMULATION`). Criticality: **IMPORTANT** — not mm/core, but
default console path on many DRM/fbdev systems.

**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature; still receiving OOB/NULL/font fixes in this tree
(`51b8258efe254`, `b5bb2c696e140`, `ac970358c5ca0`). Not staging.

===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================

**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Users with framebuffer console (typical desktop/embedded DRM
fbdev emulation, vkms/syzbot, classic fbdev). Config-specific
(`FRAMEBUFFER_CONSOLE`), not universal.

**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: (1) Console bind/takeover with a logo taller than the text grid
after integer rounding; (2) syzbot C repro via `vtcon` bind. Common on
fuzzing and on small/rotated/odd modes. Unprivileged users cannot write
`bind` (`S_IWUSR`); boot-time logo does not need that.

**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: KASAN slab-out-of-bounds **read** (and related UAF/KFENCE
reports). Can oops under KASAN/KFENCE; without sanitizers it is still an
invalid buffer walk. Severity: **HIGH** (memory-safety crash; syzbot
marked exploitable but not unprivileged).

**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record: Benefit **high** (stops a reproducible KASAN OOB in a common
console path). Risk **very low** (7-line clamp, early return already
used elsewhere in the same function). Ratio strongly favors backport.

===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================

**Step 9.1: COMPILE THE EVIDENCE**

FOR:
- Real, syzbot-reproduced KASAN OOB with C repro, 1873 crashes,
  prio:high
- Same bug title on 5.15.y / 6.1.y / 6.6.y — this 6.12 tree still has
  the unclamped walk
- Syzbot lists 94e6a058b168 as the fix commit
- Maintainer-authored, 7 lines, applies cleanly here
- Fixes a rounding hole (`DIV_ROUND_UP` vs `yres / font_height`) plus
  any logo taller than the VC
- No new API/feature; existing `FBCON_LOGO_DONTSHOW` path

AGAINST:
- Short commit message, no public review, no Fixes:
- Does not fully address the Wang RFC cols-mismatch-after-resize case
- Trigger via sysfs bind is root-only (boot logo path still matters)

UNRESOLVED:
- Could not read lore.kernel.org (403); used b4 + openwall instead
- Could not confirm whether post-fix syzbot “last: 2d” crashes are only
  on the 9 unpatched instances (likely, given 20/29 patched)

**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — clamp is local and readable;
   syzbot C repro; maintainer applied it
2. Fixes a real bug that affects users? **PASS** — KASAN OOB, thousands
   of syzbot hits, stable-tree crashes
3. Important issue (crash, security, corruption, deadlock)? **PASS** —
   HIGH memory-safety OOB
4. Small and contained? **PASS** — +7 lines, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — buggy code present; `git
   apply --check` clean

**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/docs exception. It is a normal memory-
safety bug fix.

**Step 9.4: MAKE YOUR DECISION**
This is a small, correct bounds check for a long-standing fbcon OOB that
syzbot can hit on this class of kernels. It belongs in 6.12.y.

Verification:
- [Phase 1] Parsed tags: syzbot Reported-by; Helge Deller SOB; no
  Fixes/Cc:stable/review tags in the committed message
- [Phase 2] Diff: +7 lines in `fbcon_prepare_logo()` after
  `DIV_ROUND_UP`; clamp `logo_lines` then early return
- [Phase 3] `git describe HEAD` / Makefile: **6.12.111**, branch
  pending-6.12
- [Phase 3] `git blame`: OOB loop from ^1da177e4c3f41; DIV_ROUND_UP from
  416e74ea781359 (2008)
- [Phase 3] `git merge-base --is-ancestor 94e6a058b168 HEAD` → not an
  ancestor (fix absent here)
- [Phase 3] `git apply --check` of 94e6a058b168 → clean, offset −64
- [Phase 3] Confirmed no `logo_lines = min` already in this file
- [Phase 4] `b4 dig -c 94e6a058b168` → msgid
  6a897c8e.dbb3a75c.13dd47.002f.GAE@google.com; v1 only
- [Phase 4] Syzbot extid 0c815b25cdb3678e7083: C repro, prio:high, 1873
  crashes, Fix commit 94e6a058b168
- [Phase 4] Same KASAN title on linux-5.15.y / 6.1.y / 6.6.y
- [Phase 5] Callers: `fbcon_init` (line 1155), `con2fb_init_display`
  (line 826); bind path `store_bind` is `S_IWUSR`
- [Phase 6] Local function still matches the pre-patch walk at lines
  597–604
- [Phase 7] Helge Deller listed as FRAMEBUFFER LAYER maintainer
- [Phase 8] Failure mode: slab-out-of-bounds read in
  `scr_memcpyw`/`scr_readw`; severity HIGH
- UNVERIFIED: full lore.kernel.org thread (HTTP 403); whether every
  post-Aug-24 syzbot crash is on unpatched instances only

The patch is a 7-line clamp in `fbcon_prepare_logo()` that stops a
syzbot-reproduced KASAN out-of-bounds read. The buggy walk is in this
6.12 tree, the fix applies cleanly, and the risk is limited to skipping
the boot logo when it cannot fit.

**YES**

 drivers/video/fbdev/core/fbcon.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 532522c2aed05..903823c9ce567 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -600,6 +600,13 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
 		erase &= ~0x400;
 	logo_height = fb_prepare_logo(info, par->rotate);
 	logo_lines = DIV_ROUND_UP(logo_height, vc->vc_font.height);
+	logo_lines = min(logo_lines, rows);
+	logo_lines = min(logo_lines, new_rows - 1);
+	if (logo_lines <= 0) {
+		logo_lines = 0;
+		logo_shown = FBCON_LOGO_DONTSHOW;
+		return;
+	}
 	q = (unsigned short *) (vc->vc_origin +
 				vc->vc_size_row * rows);
 	step = logo_lines * cols;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 6.18-5.10] fbdev: platinumfb: add error checking for ioremap calls
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
                   ` (3 preceding siblings ...)
  2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] fbcon: Fix KASAN slab-out-of-bounds Read in fbcon_prepare_logo Sasha Levin
@ 2026-10-19 13:53 ` Sasha Levin
  4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-10-19 13:53 UTC (permalink / raw)
  To: patches, stable
  Cc: BingKun Yue, Helge Deller, Sasha Levin, linux-fbdev, dri-devel,
	linux-kernel

From: BingKun Yue <yuebingkun@kylinos.cn>

[ Upstream commit ba13226fc966004713aed47931db8922b84d1960 ]

The ioremap() and ioremap_wt() calls in platinumfb_probe() were not
checked for failure. If any of these mappings fail, the driver would
dereference NULL pointers, leading to a kernel panic.

Add proper error checking and use goto-based cleanup to avoid code
duplication across the error paths.

Signed-off-by: BingKun Yue <yuebingkun@kylinos.cn>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit message forensics

**Step 1.1 — Subject**
- Record: **[fbdev/platinumfb] [add] error checking for ioremap calls**
- Subsystem: `fbdev: platinumfb`. Action verb: **add** (defensive
  checks). Intent: stop using NULL mappings after failed `ioremap()` /
  `ioremap_wt()`.

**Step 1.2 — Tags**
- Record (committed message):
  - `Signed-off-by: BingKun Yue <yuebingkun@kylinos.cn>` (author)
  - `Signed-off-by: Helge Deller <deller@gmx.de>` (fbdev maintainer)
- Ignored as pipeline noise: any `[Upstream commit ...]` / `Signed-off-
  by: Sasha Levin`.
- Not in the committed message, but present on the mailing-list v1/v2:
  `Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")`. Helge Deller dropped that
  tag as having no value (it points at the git epoch, not a real
  introducing change).
- No `Reported-by`, `Tested-by`, `Reviewed-by`, `Acked-by`, `Link:`, or
  `Cc: stable` in the committed message.
- Absence of `Cc: stable` / `Fixes:` is expected for this review and is
  not a negative signal.

**Step 1.3 — Body**
- Record:
  - **Bug:** `ioremap()` / `ioremap_wt()` in `platinumfb_probe()` are
    unchecked.
  - **Symptom:** NULL dereference → kernel panic.
  - **Root cause:** probe maps framebuffer, platinum registers, and cmap
    registers, then immediately writes through those pointers
    (`out_be32`, `fbuffer[0x100000] = ...`).
  - **Fix approach:** check each mapping, return `-ENOMEM`, goto-based
    cleanup to avoid duplicated teardown.
  - **Version info:** none in the body. Hardware is Apple “platinum” on
    PPC32 Power Mac.

**Step 1.4 — Hidden bug fix?**
- Record: **Yes, a real bug fix**, not style cleanup. The “goto cleanup”
  wording also **closes a pre-existing resource leak**: on
  `platinum_init_fb()` failure the old path `iounmap()`’d three mappings
  and called `framebuffer_release()` but never `release_mem_region()`
  for the framebuffer or cmap regions. `platinumfb_remove()` does
  release those regions, but `remove()` is not run after a failed probe.

---

## Phase 2: Diff analysis

**Step 2.1 — Inventory**
- Record:
  - File: `drivers/video/fbdev/platinumfb.c` (~+23 / −8 in
    `platinumfb_probe()`; ~35 lines net as posted).
  - Function modified: **`platinumfb_probe()` only**.
  - Scope: **single-file surgical fix**.

**Step 2.2 — Code flow**
- Record:
  - **Hunk 1 (framebuffer):** `ioremap_wt(..., 0x400000)` was used
    unconditionally. After: if NULL, `rc = -ENOMEM`, `goto
    err_release_fb`.
  - **Hunk 2 (platinum regs):** `ioremap(..., 0x1000)` was used
    unconditionally. After: if NULL, `goto err_unmap_fb`.
  - **Hunk 3 (cmap regs):** `ioremap(..., 0x1000)` was used
    unconditionally. After: if NULL, `goto err_release_cmap`.
  - **Hunk 4 (init failure / cleanup):** old `platinum_init_fb()`
    failure only unmapped and released `fb_info`. After: shared labels
    unmap cmap → release cmap region → unmap platinum regs → unmap
    framebuffer → release framebuffer region → `framebuffer_release()`.
    Success path now `return 0` explicitly.

**Step 2.3 — Bug mechanism**
- Record:
  - **Category:** memory safety (NULL deref) + error-path resource leak.
  - **Mechanism:** failed `ioremap*` returns NULL; next statements
    dereference it (`out_be32(&pinfo->platinum_regs->...)`,
    `fbuffer[0x100000] = 0x34`, `out_8(&pinfo->cmap_regs->addr, ...)`).
  - **Cleanup:** goto chain matches acquisition order. Nathan Chancellor
    later had an unused `err_unmap_regs` label removed; the candidate
    diff is that folded version (no unused label).

**Step 2.4 — Fix quality**
- Record:
  - Obviously correct: each mapping is checked before use; unwind order
    is right.
  - Minimal: one probe function, no API/struct changes.
  - Regression risk: **low**. Same teardown operations already exist on
    the `platinum_init_fb()` error path and in `platinumfb_remove()`.
    Extra `release_mem_region()` on probe failure is the missing
    counterpart of `request_mem_region()`.
  - Pre-existing (unchanged): `request_mem_region()` for cmap is still
    unchecked; `dev_set_drvdata()` still happens before
    `platinum_init_fb()`. Not introduced by this patch.
  - `rc` is already declared in this tree and is set on every new goto
    path (`-ENOMEM`) or by `platinum_init_fb()`.

---

## Phase 3: Git history

**Step 3.1 — Blame**
- Record:
  - Unchecked `ioremap()` of platinum/cmap regs: present since
    `1da177e4c3f41` (Linux-2.6.12-rc2, 2005).
  - Framebuffer mapping: `cc5d0189b9ba95` (2005) then `e04e39507c3c3d`
    (2018, `__ioremap()` → `ioremap_wt()`), still unchecked.
  - Partial teardown on `platinum_init_fb()` failure: `2b7574da255fef`
    (2006, “ioremap balanced with iounmap”).
- The hole has been in this driver for the entire git history, including
  this 6.12 tree.

**Step 3.2 — Fixes: tag**
- Record: mailing-list `Fixes: 1da177e4c3f4` is the epoch commit, not a
  real regression. Helge dropped it. `1da177e4c3f41` **is** an ancestor
  of this tree (`git merge-base --is-ancestor` succeeded). The buggy
  code is here regardless of the tag.

**Step 3.3 — File history**
- Record: last change touching this file in-tree is `01ecc142ef7c8`
  (“fbdev: Switch back to struct platform_driver::remove()”). Recent
  platinumfb work is platform-remove/fb_ops/whitespace, not probe
  mapping. **Standalone**, not patch N/M.

**Step 3.4 — Author**
- Record: BingKun Yue has **no** other commits in this tree under
  `drivers/video/fbdev/`. Helge Deller is the fbdev maintainer and
  signed off. Nathan Chancellor (Clang/build) reviewed the applied
  version.

**Step 3.5 — Dependencies**
- Record: uses `ioremap_wt()`, `dev_err()`, `request_mem_region()`,
  `framebuffer_release()` — all present. **Can apply standalone.**
  Context in this checkout matches the “before” side of the diff
  exactly.

---

## Phase 4: Mailing list / external research

`b4 dig -c` was not used: the candidate is **not in this checkout**, so
there is no local SHA to feed it. Thread was recovered from
lore/openwall/patchew.

**Step 4.1 — Original discussion**
- Record:
  - v1 (duplicated cleanup, no gotos):
    `20260817055625.276500-1-yuebingkun@kylinos.cn`
  - v2 (goto cleanup, applied): `https://lists.openwall.net/linux-
    kernel/2026/08/17/346` — Message-ID
    `20260817063637.306865-1-yuebingkun@kylinos.cn`
  - Helge Deller: applied v2, dropped the epoch `Fixes:` line, asked for
    a real name on SOB.
  - Nathan Chancellor: unused label `err_unmap_regs` warning; suggested
    deleting it. Helge folded that in. Candidate diff matches the folded
    result (`err_release_cmap` falls through into
    `iounmap(platinum_regs)`).
  - A later “v3” wrongly changed `strscpy` → `strcpy` and reused this
    subject; reviewers called out the mismatch. **That v3 is not this
    candidate.**
  - Merged in Helge’s fbdev pull for **v7.3-rc1**.
- No NAK. No reviewer `Cc: stable`. No claim this was a production crash
  report.

**Step 4.2 — Recipients**
- Record: To Helge Deller; Cc `linux-fbdev`, `dri-devel`, `linux-
  kernel`. Right maintainer and list.

**Step 4.3 — Bug report**
- Record: **No user/syzbot report.** Static/code-review finding. Failure
  mode is still a panic if `ioremap*` fails.

**Step 4.4 — Series**
- Record: Standalone driver fix. Same family of Mac fbdev drivers:
  `controlfb.c` and `valkyriefb.c` **already** check ioremap failure in
  this tree. platinumfb is the outlier.

**Step 4.5 — Stable list**
- Record: lore.kernel.org/stable search returned **403**. **UNVERIFIED**
  whether this SHA was discussed on stable@. No evidence it was
  rejected.

---

## Phase 5: Semantic / call-chain

**Step 5.1 — Functions**
- Record: `platinumfb_probe()` only.

**Step 5.2 — Callers**
- Record: `.probe = platinumfb_probe` in `platinum_driver`. Called from
  platform/OF probe when the node name is `"platinum"`. Boot/device-bind
  path, not a syscall.

**Step 5.3 — Callees**
- Record: `ioremap_wt` / `ioremap`, `request_mem_region` /
  `release_mem_region`, `iounmap`, `platinum_init_fb()` (VRAM probe +
  `register_framebuffer()`), `framebuffer_release()`. After a successful
  map, probe immediately MMIO-writes the mapped addresses.

**Step 5.4 — Reachability**
- Record: Reachable only if `CONFIG_FB_PLATINUM=y` (`bool`, not
  tristate) on **PPC_PMAC && PPC32** with a `platinum` OF node. Not
  userspace-triggerable on other machines. If probe runs and `ioremap*`
  fails, panic is certain.

**Step 5.5 — Similar patterns**
- Record: `controlfb.c` already does `if (!p->cmap_regs ||
  !p->control_regs || !p->frame_buffer) goto error_out;`. `valkyriefb.c`
  already does a NULL check after the three ioremaps. Historical
  analogue: `13b7c0390a5d3` “video: imsttfb: check for ioremap()
  failures”.

---

## Phase 6: This tree (pending-6.12 / 6.12.111)

**Step 6.1 — Does the bug exist here?**
- Record: **Yes.** `drivers/video/fbdev/platinumfb.c` lines 569, 574,
  578: three unchecked maps, then MMIO at 581–608. `make kernelversion`
  → **6.12.111**. `git describe HEAD` → `p-6.12-3-gc504cf0baa3d8`.
  Subject grep on this file history: **commit not present**.

**Step 6.2 — Backport difficulty**
- Record: **Clean apply.** All four “before” contexts match this file
  byte-for-byte.

**Step 6.3 — Related fixes already here?**
- Record: **No** platinumfb ioremap-check. Sibling Mac drivers already
  have the check; this driver does not.

---

## Phase 7: Subsystem context

**Step 7.1**
- Record: **fbdev / Apple Platinum**. Criticality: **PERIPHERAL**
  (legacy PPC32 Power Mac only). Still a first-party in-tree driver, not
  staging.

**Step 7.2**
- Record: File is mature/quiet. Last platinumfb-specific functional
  change in this tree is older fbdev-ops/platform-remove work. Bug is
  long-standing, not a new-subsystem leftover.

---

## Phase 8: Impact and risk

**Step 8.1 — Who is affected**
- Record: **config- and platform-specific**: 32-bit Power Macintosh with
  platinum graphics (`CONFIG_FB_PLATINUM`). Tiny population, but those
  users take this path on every boot.

**Step 8.2 — Trigger**
- Record: Probe-time `ioremap*` failure (vmalloc/ioremap space
  exhaustion or mapping rejection). Unprivileged users cannot trigger it
  except by booting that machine. Uncommon, not theoretical in the “dead
  code” sense: the next instructions are unconditional dereferences.

**Step 8.3 — Failure mode**
- Record: **NULL deref → oops/panic during probe**. Severity: **HIGH**
  (crash), not a security hole. Secondary: mem-region leak if
  `platinum_init_fb()` fails after regions were requested.

**Step 8.4 — Risk/benefit**
- Record:
  - Benefit: avoid panic and leak on a real (if rare) probe failure;
    same check sibling drivers already have.
  - Risk: **very low** (~20 lines, error path only, maintainer-applied,
    unused-label already folded).
  - Ratio: favorable. Conservative stable policy still accepts small
    NULL-deref probe fixes in in-tree drivers.

---

## Phase 9: Synthesis

**Step 9.1 — Evidence**

For:
- Unchecked `ioremap*` then immediate dereference is a real panic.
- Bug is in **this** 6.12 tree and dates to 2.6.12-era code.
- Small, obviously correct, applies cleanly.
- Also releases mem regions on `platinum_init_fb()` failure (old path
  leaked them).
- Maintainer SOB; Nathan’s warning fix is already in the candidate.
- Sibling `controlfb` / `valkyriefb` already check this.

Against:
- No crash report, no syzbot, no `Cc: stable`.
- Hardware is ancient PPC32 Power Mac; ioremap failure is uncommon.
- Helge treated the epoch `Fixes:` as worthless (not a tracked
  regression).
- Mixed into a 7.3-rc1 “fixes and updates” pull with many cleanups.

Unresolved:
- Exact mainline SHA (not in this tree).
- Whether stable@ already discussed it (lore/stable 403).
- `b4 dig` not run (no local SHA).

**Step 9.2 — Stable rules**
1. Obviously correct and tested? **PASS** — logic is straightforward;
   maintainer applied it; unused-label fixed.
2. Real bug affecting users? **PASS** — NULL deref on failed map; probe
   is the only path this driver has.
3. Important (crash/security/corruption/deadlock)? **PASS** — kernel
   panic. User base is small; crash class still qualifies.
4. Small and contained? **PASS** — one function, one file.
5. No new features/APIs? **PASS**.
6. Applies to this tree? **PASS** — context matches; no missing prereqs.

**Step 9.3 — Exceptions**
- Record: none (not a device ID, quirk, DT, build, or docs patch).
  Ordinary crash-prevention fix.

**Step 9.4 — Decision**
This is a surgical NULL-deref/leak fix in code this tree actually runs
on PPC32 Power Mac. Sibling drivers already do the check. Benefit is a
avoided panic; risk is negligible. Meets stable rules for **this** 6.12
checkout.

---

### Problem for stable users
On Power Macs with Apple platinum graphics, `platinumfb_probe()` maps
the framebuffer and two register windows and then writes through those
pointers with no NULL check. A failed `ioremap*` panics instead of
failing probe. The new labels also `release_mem_region()` on
`platinum_init_fb()` failure, which the old error path skipped.

### Rules / risk
Small, maintainer-acked, applies cleanly to 6.12.111, no new API, no
extra dependencies. Regression risk is limited to probe error paths.

### Concerns
Rare hardware and no field report; Helge dropped the epoch `Fixes:` tag.
Those do not remove the panic if mapping fails, and they do not block
backport to this tree.

Verification:
- [Phase 1] Parsed committed tags: author + Helge Deller SOB only; no
  Reported-by/syzbot.
- [Phase 1] Openwall v2: original `Fixes: 1da177e4c3f4`; Helge dropped
  it as valueless.
- [Phase 2] Diff: three NULL checks + goto teardown in
  `platinumfb_probe()`; also `release_mem_region` on init failure.
- [Phase 2] Current probe (lines 569–633) dereferences maps with no
  checks; cleanup omits mem-region release.
- [Phase 3] `git blame`: unchecked ioremap since `1da177e4c3f41` /
  `cc5d0189b9ba95`; `ioremap_wt` from `e04e39507c3c3d`.
- [Phase 3] `git merge-base --is-ancestor 1da177e4c3f41 HEAD`: yes.
- [Phase 3] `git log --oneline --grep='add error checking for ioremap'
  -- drivers/video/fbdev/`: not in this tree.
- [Phase 3] Author search in fbdev: no other BingKun Yue commits here.
- [Phase 4] Thread: v2 applied by Helge; Nathan unused-label fold
  matches candidate; bogus v3 strcpy not this patch.
- [Phase 4] lore.kernel.org/stable: 403, UNVERIFIED.
- [Phase 4] `b4 dig`: UNVERIFIED (no local candidate SHA).
- [Phase 5] Only caller: `platinum_driver.probe`; Kconfig `FB_PLATINUM`
  depends on `PPC_PMAC && PPC32`, `bool`.
- [Phase 5] `controlfb.c` / `valkyriefb.c` already check ioremap NULL in
  this tree.
- [Phase 6] `git describe HEAD` = `p-6.12-3-gc504cf0baa3d8`; `make
  kernelversion` = 6.12.111.
- [Phase 6] Before-context of all four hunks matches this file.
- [Phase 8] Failure mode: NULL deref oops on probe, severity HIGH;
  trigger rare but on the only probe path.
- UNVERIFIED: mainline SHA; stable@ discussion.

**YES**

 drivers/video/fbdev/platinumfb.c | 35 ++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/video/fbdev/platinumfb.c b/drivers/video/fbdev/platinumfb.c
index a08d955d9b439..81fdaa3541db9 100644
--- a/drivers/video/fbdev/platinumfb.c
+++ b/drivers/video/fbdev/platinumfb.c
@@ -567,15 +567,30 @@ static int platinumfb_probe(struct platform_device* odev)
 	/* frame buffer - map only 4MB */
 	pinfo->frame_buffer_phys = pinfo->rsrc_fb.start;
 	pinfo->frame_buffer = ioremap_wt(pinfo->rsrc_fb.start, 0x400000);
+	if (!pinfo->frame_buffer) {
+		dev_err(&odev->dev, "failed to ioremap frame buffer\n");
+		rc = -ENOMEM;
+		goto err_release_fb;
+	}
 	pinfo->base_frame_buffer = pinfo->frame_buffer;
 
 	/* registers */
 	pinfo->platinum_regs_phys = pinfo->rsrc_reg.start;
 	pinfo->platinum_regs = ioremap(pinfo->rsrc_reg.start, 0x1000);
+	if (!pinfo->platinum_regs) {
+		dev_err(&odev->dev, "failed to ioremap registers\n");
+		rc = -ENOMEM;
+		goto err_unmap_fb;
+	}
 
 	pinfo->cmap_regs_phys = 0xf301b000;	/* XXX not in prom? */
 	request_mem_region(pinfo->cmap_regs_phys, 0x1000, "platinumfb cmap");
 	pinfo->cmap_regs = ioremap(pinfo->cmap_regs_phys, 0x1000);
+	if (!pinfo->cmap_regs) {
+		dev_err(&odev->dev, "failed to ioremap cmap registers\n");
+		rc = -ENOMEM;
+		goto err_release_cmap;
+	}
 
 	/* Grok total video ram */
 	out_be32(&pinfo->platinum_regs->reg[16].r, (unsigned)pinfo->frame_buffer_phys);
@@ -623,13 +638,21 @@ static int platinumfb_probe(struct platform_device* odev)
 	dev_set_drvdata(&odev->dev, info);
 
 	rc = platinum_init_fb(info);
-	if (rc != 0) {
-		iounmap(pinfo->frame_buffer);
-		iounmap(pinfo->platinum_regs);
-		iounmap(pinfo->cmap_regs);
-		framebuffer_release(info);
-	}
+	if (rc != 0)
+		goto err_unmap_cmap;
+
+	return 0;
 
+err_unmap_cmap:
+	iounmap(pinfo->cmap_regs);
+err_release_cmap:
+	release_mem_region(pinfo->cmap_regs_phys, 0x1000);
+	iounmap(pinfo->platinum_regs);
+err_unmap_fb:
+	iounmap(pinfo->frame_buffer);
+err_release_fb:
+	release_mem_region(pinfo->rsrc_fb.start, resource_size(&pinfo->rsrc_fb));
+	framebuffer_release(info);
 	return rc;
 }
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-19 14:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] fbdev: udlfb: validate vendor descriptor items Sasha Levin
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] fbdev: maxine: fix 64-bit build error Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] fbdev: omap2: dsi: do not copy isr table Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] fbcon: Fix KASAN slab-out-of-bounds Read in fbcon_prepare_logo Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] fbdev: platinumfb: add error checking for ioremap calls Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox