* [PATCH AUTOSEL 6.18-5.10] tty: serial: pch_uart: add check for pci_get_slot()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:45 ` Sasha Levin
2026-10-19 14:06 ` sashiko-bot
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-6.12] serial: 8250: force synchronous probe for the ISA and PNP drivers Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-6.6] serial: qcom_geni: Disable closing_wait for console to prevent shutdown timeout Sasha Levin
2 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:45 UTC (permalink / raw)
To: patches, stable
Cc: Haoxiang Li, Andy Shevchenko, Greg Kroah-Hartman, Sasha Levin,
jirislaby, linux-kernel, linux-serial
From: Haoxiang Li <haoxiang_li2024@163.com>
[ Upstream commit 970ede67de0e3f6a1e707ec99ae49d80bf3f2966 ]
Add check for pci_get_slot() to prevent a potetial
null pointer dereference in pch_request_dma().
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/20260623140539.2272473-1-haoxiang_li2024@163.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
Record: `[tty: serial: pch_uart] [add] [NULL check on pci_get_slot() in
pch_request_dma()]`
**Step 1.2: Tags** (from the candidate message; original mail had extra
tags — see Phase 4)
- Reviewed-by: Andy Shevchenko `<andriy.shevchenko@linux.intel.com>`
(serial/PCI reviewer; author of the `pci_get_slot()` call)
- Link: `https://patch.msgid.link/20260623140539.2272473-1-
haoxiang_li2024@163.com`
- Signed-off-by: Haoxiang Li (author); Greg Kroah-Hartman (tty
maintainer)
- No Reported-by, Tested-by, Acked-by in the committed message
- No Fixes:/Cc: stable in the committed message (expected for this
review queue)
**Step 1.3: Body**
Record:
- Bug: `pci_get_slot()` can return NULL; the caller then does
`param->dma_dev = &dma_dev->dev`.
- Symptom: potential NULL pointer dereference in `pch_request_dma()`.
- No stack trace, reproducer, or version range. Typo “potetial”.
- Root cause as stated: missing return-value check.
**Step 1.4: Hidden bug fix?**
Record: Not hidden. Explicit defensive NULL check. Same pattern as other
static-analysis “add check for X()” patches from this author.
---
## Phase 2: Diff analysis
**Step 2.1: Inventory**
Record:
- File: `drivers/tty/serial/pch_uart.c` (+5 / −0)
- Function: `pch_request_dma()`
- Scope: single-file surgical fix
**Step 2.2: Flow**
Record: After `pci_get_slot(priv->pdev->bus,
PCI_DEVFN(PCI_SLOT(priv->pdev->devfn), 0))`, the code used `dma_dev`
unconditionally. After the patch, NULL returns with `dev_err` and no
`pci_dev_put` (correct: no reference was taken). Later failures still
use `goto err_pci_get` → `pci_dev_put(dma_dev)`.
**Step 2.3: Bug mechanism**
Record: **NULL pointer dereference.** `pci_get_slot()` is documented to
return NULL when the slot/function is absent (`drivers/pci/search.c`).
The next statement dereferences `dma_dev`.
**Step 2.4: Fix quality**
Record: Obviously correct, minimal, no API/lock/structure changes.
Regression risk is very low. Early return leaves DMA unset;
`pch_uart_verify_port()` only sets `use_dma` if `priv->chan_rx` is set,
so PIO fallback still works on that path.
---
## Phase 3: Git history
**Step 3.1: Blame**
Record:
- Driver added in `3c6a483275f47` (2010, “Serial: EG20T: add PCH_UART
driver”) using hardcoded `pci_get_bus_and_slot()`.
- Current lookup introduced in `8368d6a2b73900` (Andy Shevchenko,
2014-07-30, v3.17-rc1).
- NULL check added by this commit `970ede67de0e3` (2026-06-23).
- Bug has been in tree for ~12 years.
**Step 3.2: Fixes: tag**
Committed message has none. Original mail (Phase 4) had
`Fixes: 8368d6a2b739 ("pch_uart: don't hardcode PCI slot to get DMA
device")`.
Record: `8368d6a2b739` **is** an ancestor of this checkout (`git merge-
base --is-ancestor` succeeded; contained in v3.17-rc1).
**Step 3.3: Related file history**
Record:
- `8be3a7bf77370` (2022): missing `pci_dev_put()` after `pci_get_slot()`
— confirms the API contract (refcount on success, NULL on failure).
- `6fe472c1bbbe2` (2026): NULL check for `dma_alloc_coherent()` in the
same function, with `Fixes:` + `Cc: stable`.
- `e41c0981213f3` (2013): real NULL deref in this driver when DMA setup
failed.
Standalone 5-line patch, not part of a numbered series.
**Step 3.4: Author**
Record: Haoxiang Li is a regular static-analysis contributor, not the
tty maintainer. One pch_uart commit. Reviewed by Andy Shevchenko;
applied by Greg Kroah-Hartman.
**Step 3.5: Dependencies**
Record: None. Uses existing `pci_get_slot()`, `dev_err`, and
`pch_request_dma()`. Applies standalone.
---
## Phase 4: Mailing list / external
**Step 4.1: Original discussion**
Record:
- `b4 dig -c 970ede67de0e3` → https://patch.msgid.link/20260623140539.22
72473-1-haoxiang_li2024@163.com
- `b4 dig -a`: v1 only; that version was applied.
- Original body **did** include `Fixes: 8368d6a2b739` and `Cc:
stable@vger.kernel.org` (stripped in the git commit; that is why this
is in the manual queue, not a reason to reject).
- Andy Shevchenko: *“Not that it's quite possible, but probably you
found this due to AI review which might have considered some cases of
manual binding a PCI device to any PCI driver. Given that is the case,
I'm fine with the change.”*
- No NAK.
**Step 4.2: Recipients (`b4 dig -w`)**
Record: gregkh, jirislaby, andriy.shevchenko, linux-serial, linux-
kernel, **stable@vger.kernel.org**, kees@kernel.org. Right maintainers
were on CC. Andy reviewed; Greg applied.
**Step 4.3: Bug report**
Record: No syzbot, bugzilla, or user report. Static/AI analysis of a
documented NULL return.
**Step 4.4: Series**
Record: Standalone v1. Related in-function fix `6fe472c1bbbe2` is
independent.
**Step 4.5: Stable list**
Record: lore stable search shows the original patch (because it was CCed
there) and Andy’s reply. No follow-up rejecting it for stable. WebFetch
to lore returned 403; thread recovered via `b4 dig -m` and curl.
---
## Phase 5: Semantic / call graph
**Step 5.1: Functions**
Record: `pch_request_dma()` only.
**Step 5.2: Callers**
Record (from this tree):
1. `pch_uart_startup()` — if `priv->use_dma`
2. `pch_uart_verify_port()` — TIOCSSERIAL / setserial path, which this
driver historically runs during install (`af6d17cdc8c89`)
**Step 5.3: Callees**
Record: `pci_get_slot`, `dma_request_channel`, `dma_alloc_coherent`,
`pci_dev_put`.
**Step 5.4: Reachability**
Record: Yes, from userspace serial setup (TIOCSSERIAL) and from UART
startup when DMA was already enabled. Needs `CONFIG_SERIAL_PCH_UART` and
typically `CONFIG_PCH_DMA`. Not a syscall fuzz target for unprivileged
users; admin/boot path on EG20T/ML72xx hardware.
**Step 5.5: Similar patterns**
Record: Same function already checks `dma_request_channel` and
`dma_alloc_coherent`. DMA PCI ID 0x8810 is function-0 sibling of UART
IDs 0x8811–0x8814; lookup is “same slot, function 0”.
---
## Phase 6: This checkout
**Step 6.1: Buggy code in this tree?**
Record: Local tree is **v7.3-rc4-27** (`Makefile` 7.3.0-rc4). Driver and
`pci_get_slot()` lookup are present. Introducing commit `8368d6a2b739`
is an ancestor. Current `pch_request_dma()` already contains the NULL
check from `970ede67de0e3` (first appeared `v7.2-rc1-21`). The bug class
exists in this lineage; the file is not missing.
**Step 6.2: Backport difficulty**
Record: Clean apply. Hunk is independent of the later `goto` ladder.
**Step 6.3: Other fixes already here?**
Record: Refcount leak fix and `dma_alloc_coherent` check are present. No
alternate fix for this NULL case besides this commit.
---
## Phase 7: Subsystem context
**Step 7.1:**
Record: `drivers/tty/serial/pch_uart.c` — Intel EG20T PCH / LAPIS
ML7213/ML7223/ML7831 UART (Atom E6xx-era embedded/IVI). **PERIPHERAL**,
but a real in-tree production driver.
**Step 7.2:**
Record: Low churn; occasional correctness fixes. Mature, not staging.
---
## Phase 8: Impact and risk
**Step 8.1: Who is affected**
Record: Driver/config-specific: `CONFIG_SERIAL_PCH_UART` on 32-bit x86
or MIPS (or `COMPILE_TEST`). Not universal.
**Step 8.2: Trigger**
Record: `pch_request_dma()` when function 0 of the UART’s PCI slot is
not enumerated (firmware hid DMA, sparse multifunction device, or sysfs
bind of this driver to a non-fn0 device with no fn0 sibling). On stock
EG20T, fn0 DMA (0x8810) is normally present, so `pci_get_slot()`
succeeds. Unprivileged users cannot bind PCI drivers. `verify_port` is a
real install/setserial path.
**Step 8.3: Failure mode**
Record: Kernel oops (NULL deref) instead of PIO fallback. Severity:
**HIGH** if it fires, **low likelihood** on intended hardware. Andy
called the trigger unlikely but accepted the case.
**Step 8.4: Risk/benefit**
Record: Benefit: prevent oops, match existing error handling. Risk: very
low (5 lines, error path only). Ratio: favorable.
---
## Phase 9: Synthesis
**Step 9.1: Evidence**
For:
- Real, documented NULL return from `pci_get_slot()`, immediately
dereferenced.
- 5-line, obviously correct check; UART continues without DMA.
- Reviewed by Andy Shevchenko; applied by Greg KH.
- Original submission asked for stable and cited `Fixes: 8368d6a2b739`
(in tree since v3.17).
- Same function already treats DMA setup failure as a crash-worthy bug
(`e41c0981213f3`, `6fe472c1bbbe2`).
- Applies cleanly; no new API; no prerequisites.
Against:
- Niche ~2010 embedded UART.
- No user/syzbot report; likely AI/static analysis.
- Reviewer: trigger is “not quite possible” except odd PCI bind.
- Committed message dropped `Cc: stable` / `Fixes:` (auto-select would
have needed those tags).
Unresolved: none that change the technical call. Lore WebFetch 403 was
covered by `b4` + curl.
**Step 9.2: Stable rules**
1. Obviously correct and tested? **PASS** — reviewed; Greg merged; logic
is local and clear.
2. Real bug affecting users? **PASS** — API can return NULL; oops if DMA
setup runs without fn0. Rare, but not fictional.
3. Important (crash/security/corruption/deadlock)? **PASS** — NULL deref
→ oops; small blast radius.
4. Small and contained? **PASS** — +5 lines, one function.
5. No new features/APIs? **PASS**.
6. Can apply to this tree? **PASS** — driver and call site exist; hunk
is clean.
**Step 9.3: Exception category**
Record: None (not an ID/quirk/DT/build/docs exception). Ordinary memory-
safety fix.
**Step 9.4: Decision**
This tree is 7.3-rc4 and already contains the driver. The bug is a NULL
deref on a documented failure path, the patch is a five-line guard with
PIO fallback, and it needs no other commits. Niche hardware and a rare
trigger do not outweigh an oops fix this small. It belongs in this tree.
---
### Problem and why it matters here
`pch_request_dma()` looks up the EG20T/ML72xx DMA function with
`pci_get_slot(..., function 0)` and then uses `dma_dev->dev`. If that
function is missing, the kernel oopses while opening or reconfiguring
the UART. The patch logs and returns so the port can keep running in PIO
mode.
### Stable rules / risk
Meets the rules: real crash, tiny, no features, clean apply on this
checkout. Benefit (avoid oops) dwarfs regression risk.
### Concerns
Trigger is uncommon on real EG20T boards; this is defensive. That is not
a reason to keep a NULL deref. No extra backport work.
---
Verification:
- [Phase 1] Parsed committed tags: Reviewed-by Andy Shevchenko, Link to
msgid, SOBs Haoxiang Li + Greg KH; no Reported-by/syzbot.
- [Phase 2] `git show 970ede67de0e3`: +5 lines in `pch_request_dma()`
after `pci_get_slot()`.
- [Phase 2] Read `pch_request_dma()` in this tree: check +
`err_pci_get`/`pci_dev_put` paths confirmed.
- [Phase 3] `git blame` / `git log -L`: lookup from `8368d6a2b739`
(v3.17-rc1); driver from `3c6a483275f47` (2010).
- [Phase 3] `git merge-base --is-ancestor 8368d6a2b739 HEAD` → yes.
- [Phase 3] Related: `8be3a7bf77370` refcount, `6fe472c1bbbe2` dma_alloc
check, `e41c0981213f3` historical DMA NULL oops.
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1 thread; original had
Fixes:+Cc:stable; Andy Reviewed-by; no NAK.
- [Phase 4] WebFetch lore 403; curl on lore stable showed patch + Andy
reply only.
- [Phase 5] Callers: `pch_uart_startup` (line 1267),
`pch_uart_verify_port` (line 1427).
- [Phase 5] `pci_get_slot()` in `drivers/pci/search.c` returns NULL if
not found.
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; `make
kernelversion` = 7.3.0-rc4.
- [Phase 6] `git merge-base --is-ancestor 970ede67de0e3 HEAD` → yes
(first at v7.2-rc1-21).
- [Phase 7] Kconfig `SERIAL_PCH_UART`: EG20T/ML7213/ML7223/ML7831.
- [Phase 8] DMA ID 0x8810 vs UART 0x8811–0x8814 in `pch_dma.c` /
`pch_uart.c`.
- UNVERIFIED: live crash on real EG20T hardware (no user report).
**YES**
drivers/tty/serial/pch_uart.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index c3f32e0590b7f..9104361ccb14f 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -678,6 +678,11 @@ static void pch_request_dma(struct uart_port *port)
/* Get DMA's dev information */
dma_dev = pci_get_slot(priv->pdev->bus,
PCI_DEVFN(PCI_SLOT(priv->pdev->devfn), 0));
+ if (!dma_dev) {
+ dev_err(priv->port.dev, "%s: failed to get DMA device\n",
+ __func__);
+ return;
+ }
/* Set Tx DMA */
param = &priv->param_tx;
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 6.18-6.12] serial: 8250: force synchronous probe for the ISA and PNP drivers
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-5.10] tty: serial: pch_uart: add check for pci_get_slot() Sasha Levin
@ 2026-10-19 13:45 ` Sasha Levin
2026-10-19 14:10 ` sashiko-bot
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-6.6] serial: qcom_geni: Disable closing_wait for console to prevent shutdown timeout Sasha Levin
2 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:45 UTC (permalink / raw)
To: patches, stable
Cc: Rahul Bukte, Greg Kroah-Hartman, Sasha Levin, jirislaby,
linux-kernel, linux-serial
From: Rahul Bukte <rahul.bukte@sony.com>
[ Upstream commit 31e11af34e2c7cc61e25533ab877b1f9cc3eb528 ]
On x86_64 defconfig, booting with driver_async_probe=serial hangs in early
init. The 8250 PNP driver is put onto the async probe pool.
serial8250_register_8250_port() runs in a kworker concurrently with the
ISA registration done from the serial8250_init() initcall resulting in a
deadlock or NULL dereference.
- Deadlock: serial_core_register_port() holds port_mutex across
serial_core_add_one_port()
uart_configure_port()
autoconfig_irq()
probe_irq_on()
async_synchronize_full(),
which waits for the async probe pool to drain.
The async PNP worker reaches the "port already in use" check and
tries to unregister it.
serial8250_register_8250_port()
uart_remove_one_port()
serial_core_unregister_port()
This blocks on port_mutex.
The init thread waits for the worker and the worker waits for the
init thread.
- NULL deref: when the worker instead observes a slot whose port.dev
is set but whose port_dev has not yet been populated, it hits the
null pointer on the call to serial_core_get_ctrl_dev() in
serial_core_unregister_port().
Signed-off-by: Rahul Bukte <rahul.bukte@sony.com>
Link: https://patch.msgid.link/20260610054130.2825182-1-rahul.bukte@sony.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **v7.3-rc4**. The two-line `PROBE_FORCE_SYNCHRONOUS`
change is a real deadlock/NULL-deref fix on the 8250 ISA vs PNP probe
path, and the affected code is present here.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[serial/8250] [force] force synchronous probe for the ISA and
PNP drivers`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Signed-off-by: Rahul Bukte <rahul.bukte@sony.com>` (author)
- `Link: https://patch.msgid.link/20260610054130.2825182-1-
rahul.bukte@sony.com`
- `Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>` (tty
maintainer applied it)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
by:`, or `Cc: stable` (absence of Fixes/Cc-stable is expected for this
review)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** On x86_64 defconfig, `driver_async_probe=serial` hangs in
early init. The 8250 PNP driver (`name = "serial"`) is queued for
async probe while `serial8250_init()` still registers ISA ports.
- **Symptom/failure mode:**
1. **Deadlock:** ISA `serial_core_register_port()` holds `port_mutex`
through `autoconfig_irq()` → `probe_irq_on()` →
`async_synchronize_full()`. The async PNP worker hits the in-use
port path and blocks in `serial_core_unregister_port()` on the same
mutex.
2. **NULL deref:** The worker sees `port.dev` set but `port_dev` still
NULL, then `serial_core_get_ctrl_dev(port_dev)` oopses.
- **Version information:** None beyond “x86_64 defconfig”.
- **Root cause:** Concurrent ISA initcall registration vs async PNP
probe of the same 8250 port slots.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. It is an explicit deadlock/oops fix.
The in-thread note “Found while experimenting with
`driver_async_probe=*` to improve boot time” only explains how it was
found, not that it is theoretical.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- `drivers/tty/serial/8250/8250_platform.c`: +1
(`serial8250_isa_driver.driver.probe_type`)
- `drivers/tty/serial/8250/8250_pnp.c`: +1
(`serial_pnp_driver.driver.probe_type`)
- Two insertions total. Surgical, two-file, same subsystem.
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** Both drivers use default `PROBE_DEFAULT_STRATEGY`.
`driver_allows_async_probing()` then honors `driver_async_probe=` /
module async-probe, so PNP probe can run in a kworker while
`serial8250_init()` is still in the initcall.
- **After:** `PROBE_FORCE_SYNCHRONOUS` makes
`driver_allows_async_probing()` return false, so probe stays on the
registration thread and cannot overlap ISA `uart_add_one_port()`.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** Race → deadlock and NULL dereference.
- **Mechanism verified in this tree:**
- `serial8250_init()` calls `serial8250_pnp_init()` then later
`serial8250_register_ports()` which sets `up->port.dev` and calls
`uart_add_one_port()`.
- `serial_core_register_port()` holds `port_mutex` across
`serial_core_add_one_port()` → `uart_configure_port()` →
`config_port()`. With `UPF_AUTO_IRQ` (x86 `STD_COM*_FLAGS` when
`CONFIG_SERIAL_8250_DETECT_IRQ`), that reaches `autoconfig_irq()` →
`probe_irq_on()` → `async_synchronize_full()`.
- Concurrent PNP: `serial_pnp_probe()` →
`serial8250_register_8250_port()`. If `uart->port.dev` is already
set: `uart_remove_one_port()` → `serial_core_unregister_port()`.
- `serial_core_unregister_port()` reads `port->port_dev` and calls
`serial_core_get_ctrl_dev(port_dev)` **before** taking `port_mutex`.
NULL `port_dev` is an oops. Non-NULL `port_dev` waits on
`port_mutex` while the init thread waits for async probe → deadlock.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: `PROBE_FORCE_SYNCHRONOUS` exists specifically so
drivers that cannot tolerate async probe stay synchronous, including
against `driver_async_probe=`. Same pattern as i8042 PNP.
- Minimal: two field initializers, no API/behavior change for the
default (already synchronous) boot path.
- Regression risk is very low. The only cost is that these two drivers
cannot be async-probed, which is the intended workaround.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- ISA `platform_driver` struct: `ffd8e8bd26e94` (“serial: 8250: Extract
platform driver”, 2024).
- PNP `pnp_driver` struct: original tree (`1da177e4c3f41`, 2005).
- The race is not in newly added probe_type lines; those are the fix.
The overlapping register/unregister design is long-standing. The NULL
deref specifically needs `port_dev` / `serial_core_get_ctrl_dev()`,
introduced by `84a9582fd2030` (“serial: core: Start managing serial
controllers to enable runtime PM”, in v6.5-rc1).
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Not applicable. The deadlock ingredients
(`probe_irq_on()` → `async_synchronize_full()` in `22a9d645677fe`;
`driver_async_probe=` in `1ea61b68d0f86`) and the serial-core `port_dev`
path are all in this tree.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Recent 8250_platform/pnp work is refactoring, not a hidden
prerequisite series.
- This is standalone (not `patch X/Y`).
- Follow-up on these files after this commit is only `4f643bef0984b`
(named initializers in the PNP ID table), unrelated.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Rahul Bukte is not a serial maintainer. This is their only
serial commit here; other listed commits are i915 NULL checks. Weight
comes from Greg Kroah-Hartman applying it to tty, not from author
tenure.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Standalone. It only needs `struct device_driver.probe_type` and
`PROBE_FORCE_SYNCHRONOUS`, both present in
`include/linux/device/driver.h` and `drivers/base/dd.c` in this tree.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 31e11af34e2c7`:
https://patch.msgid.link/20260610054130.2825182-1-rahul.bukte@sony.com
- `b4 dig -a`: **v1 only**, no later revisions.
- Saved mbox (`b4 dig -m`) is the original patch only; **no review
replies, NAKs, or Cc: stable comments**.
- Extra note in the mail (not in the committed message): “Found while
experimenting with `driver_async_probe=*` … Tested on x86_64 defconfig
under QEMU + KVM with `driver_async_probe=serial`.”
- lore.kernel.org HTTP fetch returned 403; LKML mirror matches the same
v1 text.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc included Greg Kroah-Hartman, Jiri Slaby,
Ilpo Järvinen, linux-serial, linux-kernel. No `Reviewed-by` landed. Greg
applied it (tty maintainer SOB).
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla. Author reproduced a boot hang on x86_64
defconfig + QEMU. That is a concrete trigger, not a static guess.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone. Historical analogue: i8042 PNP
`PROBE_FORCE_SYNCHRONOUS` (same driver-core annotation for probes that
cannot run async). A 2018 attempt to mark 8250
`PROBE_PREFER_ASYNCHRONOUS` shows async 8250 probe has been known to be
delicate.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: `https://lore.kernel.org/stable/?q=...` returned 403. Web search
found no stable-thread discussion. **UNVERIFIED:** whether anyone
already asked for or rejected this on stable@.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: No function bodies changed. Annotations on
`serial8250_isa_driver` and `serial_pnp_driver`. Runtime effect is
through `driver_allows_async_probing()` → probe of `serial8250_probe` /
`serial_pnp_probe`.
**Step 5.2: TRACE CALLERS**
Record:
- `serial_pnp_probe`: PNP match during `pnp_register_driver()` from
`serial8250_pnp_init()` inside `serial8250_init()` (module_init /
boot).
- ISA path: `serial8250_register_ports()` from the same initcall;
platform `serial8250_probe` from
`platform_driver_register(&serial8250_isa_driver)`.
- Context: boot initcall, not a syscall hot path. Reachable on any x86
box with 8250 + PNP (ACPI selects `PNP`; `SERIAL_8250_PNP` defaults to
y).
**Step 5.3: TRACE CALLEES**
Record: `serial_pnp_probe` → `serial8250_register_8250_port` → possible
`uart_remove_one_port` → `serial_core_unregister_port`. ISA:
`uart_add_one_port` → `serial_core_register_port` →
`uart_configure_port` → `autoconfig_irq` → `probe_irq_on` →
`async_synchronize_full`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Triggered by the documented kernel parameter
`driver_async_probe=serial` or `=*`, not by an unprivileged syscall.
Still a real boot-time path: x86_64 defconfig sets
`CONFIG_SERIAL_8250=y`, `CONFIG_SERIAL_8250_CONSOLE=y`,
`CONFIG_SERIAL_8250_DETECT_IRQ=y`. Userspace cannot hit it after boot
unless the driver is re-probed.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Many drivers already set `PROBE_FORCE_SYNCHRONOUS` (i8042 PNP,
USB gadget UDC, PHY, Hyper-V, CXL, regulators, etc.). This is the
established way to opt a driver out of async probe.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- `git describe HEAD`: `v7.3-rc4-27-gf0100363d8c37`
- `make kernelversion` / Makefile: **7.3.0-rc4**
- Both files and both driver structs exist.
`serial_core_unregister_port()` still dereferences `port_dev` before
`port_mutex`. `serial8250_register_ports()` still sets `port.dev` then
calls `uart_add_one_port()`. `probe_irq_on()` still calls
`async_synchronize_full()`. `x86_64_defconfig` still enables
`SERIAL_8250_DETECT_IRQ`.
- NULL-deref window exists since serial-core `port_dev` (v6.5). Deadlock
window with `driver_async_probe` exists since that cmdline was added
(2019) on kernels with IRQ autodetect.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Clean apply in this tree. `8250_platform.c` exists here (split
in `ffd8e8bd26e94`). `probe_type` field exists. No conflicting local
changes.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No earlier alternate fix for this race. `git log
--grep='synchronous probe' -- drivers/tty/serial/` only shows this
commit.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: `drivers/tty/serial/8250` — **IMPORTANT**. 8250 is the standard
PC UART/console driver, not staging/peripheral.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively maintained (regular tty/serial 8250 cleanups). The bug
is in long-lived register/unregister interaction plus async probe, not
in brand-new 8250 code.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Config/cmdline specific: `CONFIG_SERIAL_8250` + PNP 8250, and
`driver_async_probe=serial` or `=*`. Default boot (no that parameter)
stays synchronous and does not hit it. Population is everyone using that
documented parameter on x86 8250, including distro/OEM boot-time
experiments.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Trigger: boot with `driver_async_probe=serial` (PNP name) or `=*`.
- Not default, but documented in `kernel-parameters.txt` for boot-time
optimization.
- Timing-dependent: deadlock vs NULL deref depends on whether `port_dev`
is populated yet.
- Unprivileged users cannot set the cmdline after boot.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- Deadlock during early init → **CRITICAL** (boot hang).
- NULL deref in `serial_core_get_ctrl_dev()` → **CRITICAL** (oops).
- Author reproduced the hang on x86_64 defconfig + QEMU.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** High for anyone using `driver_async_probe` on 8250;
prevents boot hang/oops. Default path is unchanged.
- **Risk:** Very low (2 lines, existing driver-core flag, no locking/API
change).
- **Ratio:** Strongly in favor of taking it.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR backporting:
- Real, reproduced boot hang and a NULL deref on a common x86_64
defconfig.
- Failure modes are deadlock and oops (stable-class bugs).
- Two-line, obviously correct use of `PROBE_FORCE_SYNCHRONOUS`.
- Call paths verified in this 7.3-rc4 tree.
- Applied by the tty maintainer.
- Same annotation pattern as i8042 PNP.
- Default probe behavior is already synchronous; this only blocks a
dangerous async override.
AGAINST:
- Trigger is a non-default cmdline, not everyday production boots.
- No `Reviewed-by`; mailing list had no discussion.
- Author is not a serial regular.
- No syzbot or multi-user reports.
UNRESOLVED:
- Stable@ discussion could not be fetched (HTTP 403).
- Whether a more invasive serial-core locking fix would be “more
correct” long-term (not required for this self-contained workaround).
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — two field sets; tested on
QEMU x86_64 defconfig; Greg applied it.
2. Fixes a real bug that affects users? **PASS** — hang/oops with a
documented kernel parameter.
3. Important issue? **PASS** — deadlock and NULL deref, severity
CRITICAL when triggered.
4. Small and contained? **PASS** — +2 lines, two 8250 driver structs.
5. No new features or APIs? **PASS** — uses existing `probe_type`.
6. Can apply to the local tree? **PASS** — files, APIs, and buggy paths
all exist on v7.3-rc4.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/DT/docs/build fix. Closest analogue is a
hardware/driver quirk: annotate a driver that cannot safely probe
asynchronously.
**Step 9.4: DECISION**
This belongs in this tree. The 8250 ISA/PNP overlap plus
`probe_irq_on()`’s `async_synchronize_full()` is a genuine boot deadlock
and oops once async probe is forced. The workaround is the smallest
correct change and does not alter the default synchronous boot path.
===============================================================
Verification
===============================================================
- [Phase 1] Parsed tags: author SOB, Greg KH SOB, lore msgid link; no
Fixes/Reported-by/Reviewed-by/Cc-stable.
- [Phase 2] Diff: +1 line each in `8250_platform.c` and `8250_pnp.c`;
`probe_type = PROBE_FORCE_SYNCHRONOUS`.
- [Phase 2] `driver_allows_async_probing()`: `PROBE_FORCE_SYNCHRONOUS`
returns false even when cmdline requests async.
- [Phase 2] `serial8250_init()` order: `serial8250_pnp_init()` then
`serial8250_register_ports()` then `platform_driver_register()`.
- [Phase 2] `serial8250_register_8250_port()`: `if (uart->port.dev)
uart_remove_one_port()`.
- [Phase 2] `serial_core_unregister_port()` calls
`serial_core_get_ctrl_dev(port_dev)` before `port_mutex`.
- [Phase 2] `probe_irq_on()` calls `async_synchronize_full()`
(`kernel/irq/autoprobe.c:39`).
- [Phase 2] x86 `STD_COM*_FLAGS` include `UPF_AUTO_IRQ` when
`CONFIG_SERIAL_8250_DETECT_IRQ`; x86_64_defconfig sets that.
- [Phase 3] `git blame`: probe_type lines are this commit; surrounding
structs are older.
- [Phase 3] `84a9582fd2030` introduced `serial_core_unregister_port`
(contained in v6.5-rc1).
- [Phase 3] Author has no other 8250 commits; no prerequisite series.
- [Phase 4] `b4 dig -c 31e11af34e2c7`: original submission found; `-a`
shows v1 only; `-w` lists Greg/Jiri/Ilpo/linux-serial; `-m` mbox has
no replies.
- [Phase 4] lore.kernel.org WebFetch/curl: 403. LKML mirror confirms
same v1 text.
- [Phase 4] UNVERIFIED: stable@ archive contents.
- [Phase 5] PNP driver `.name = "serial"` matches
`driver_async_probe=serial`. ISA name is `serial8250`.
- [Phase 5] ACPI Kconfig `select PNP`; `SERIAL_8250_PNP` default y.
- [Phase 6] `git describe HEAD`: v7.3-rc4-27-gf0100363d8c37; Makefile
7.3.0-rc4.
- [Phase 6] `git merge-base --is-ancestor 31e11af34e2c7 HEAD`: ancestor
(used only to confirm the code under review is this change).
- [Phase 8] Failure mode: early-init deadlock or NULL deref; severity
CRITICAL when `driver_async_probe` is used.
The patch is small, correct, and prevents a boot hang/oops on a path
that exists in this 7.3-rc4 tree.
**YES**
drivers/tty/serial/8250/8250_platform.c | 1 +
drivers/tty/serial/8250/8250_pnp.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_platform.c b/drivers/tty/serial/8250/8250_platform.c
index fe7ec440ffa58..69d8720fb65bf 100644
--- a/drivers/tty/serial/8250/8250_platform.c
+++ b/drivers/tty/serial/8250/8250_platform.c
@@ -287,6 +287,7 @@ static struct platform_driver serial8250_isa_driver = {
.driver = {
.name = "serial8250",
.acpi_match_table = acpi_platform_serial_table,
+ .probe_type = PROBE_FORCE_SYNCHRONOUS,
},
};
diff --git a/drivers/tty/serial/8250/8250_pnp.c b/drivers/tty/serial/8250/8250_pnp.c
index 7a837fdf9df13..3f41a9d6cb273 100644
--- a/drivers/tty/serial/8250/8250_pnp.c
+++ b/drivers/tty/serial/8250/8250_pnp.c
@@ -521,6 +521,7 @@ static struct pnp_driver serial_pnp_driver = {
.remove = serial_pnp_remove,
.driver = {
.pm = pm_sleep_ptr(&serial_pnp_pm_ops),
+ .probe_type = PROBE_FORCE_SYNCHRONOUS,
},
.id_table = pnp_dev_table,
};
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 6.18-6.6] serial: qcom_geni: Disable closing_wait for console to prevent shutdown timeout
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-5.10] tty: serial: pch_uart: add check for pci_get_slot() Sasha Levin
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-6.12] serial: 8250: force synchronous probe for the ISA and PNP drivers Sasha Levin
@ 2026-10-19 13:53 ` Sasha Levin
2026-10-19 15:51 ` sashiko-bot
2 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:53 UTC (permalink / raw)
To: patches, stable
Cc: Aniket Randive, Greg Kroah-Hartman, Sasha Levin, jirislaby,
linux-arm-msm, linux-kernel, linux-serial
From: Aniket Randive <aniket.randive@oss.qualcomm.com>
[ Upstream commit acba38e675dd66f52925cb9edf9ed8db16335875 ]
During system power-off, systemd closes the console UART and blocks in
tty_wait_until_sent() waiting for pending TX completion before
uart_shutdown() can cancel pending TX operations. With the default
closing_wait of 30s, this causes a watchdog reset when systemd is
responsible for watchdog feeding.
Set closing_wait to ASYNC_CLOSING_WAIT_NONE in startup() for console
ports to bypass tty_wait_until_sent() on close, allowing uart_shutdown()
to cancel TX commands cleanly. This change doesn't impact regular ports
functionality.
Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
Link: https://patch.msgid.link/20260706093749.836121-1-aniket.randive@oss.qualcomm.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This checkout is **v7.3-rc4** (`7.3.0-rc4`). The candidate is a small
Qualcomm GENI UART console workaround for a 30s close-time TX drain that
can watchdog-reset the box on power-off. The driver, `startup()`, and
`closing_wait` path are all present here.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[serial/qcom_geni] [Disable] [skip console `closing_wait` so
UART close does not stall shutdown]**
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected for this review pipeline)
- **Reported-by / Tested-by / Reviewed-by / Acked-by:** none
- **Cc: stable:** none (expected)
- **Link:** https://patch.msgid.link/20260706093749.836121-1-
aniket.randive@oss.qualcomm.com
- **Signed-off-by:** Aniket Randive (author); Greg Kroah-Hartman (TTY
maintainer). Ignore any Sasha Levin SOB — not on this commit.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** On power-off, systemd closes the console UART. Close blocks
in `tty_wait_until_sent()` for pending TX before the UART can cancel
that TX.
- **Symptom:** Default `closing_wait` is 30s (`tty_port_init`: `(3000 *
HZ) / 100`). If systemd is the watchdog feeder, that stall causes a
**watchdog reset** instead of a clean power-off.
- **Root cause (author):** GENI TX is command-based; drain waits for
completion, but cancel happens only later in `uart_shutdown()` /
driver shutdown. Chicken-and-egg.
- **Fix:** In `qcom_geni_serial_startup()`, if `uart_console(uport)`,
set `tport->closing_wait = ASYNC_CLOSING_WAIT_NONE`. Regular UARTs
unchanged.
- **Versions:** none named.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. It is an explicit hang/watchdog fix
using an existing tty_port flag.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/tty/serial/qcom_geni_serial.c` — **+8 / −0**
- Function: `qcom_geni_serial_startup()`
- Scope: single-file, surgical, console-only
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** Console ports keep default 30s `closing_wait`. Last close
→ `tty_port_close_start()` → `tty_wait_until_sent(tty, 30s)` if the
software TX buffer or UART FIFO is not empty.
- **After:** Console ports skip that drain. Close proceeds immediately
so shutdown can continue.
- **Path:** close/power-off of the console tty, not the hot TX path, not
non-console ports.
Verified close chain:
1. `tty_release()` → `uart_close()` → `tty_port_close()`
2. `tty_port_close_start()` (tty_port.c:638-643) waits if `closing_wait
!= ASYNC_CLOSING_WAIT_NONE`
3. `tty_wait_until_sent()` waits on `!tty_chars_in_buffer(tty)`, then
`uart_wait_until_sent()` polls `ops->tx_empty`
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Logic / hardware workaround]** GENI TX uses a sequencer
command (`UART_START_TX`). If that command is still active, `tx_empty`
(`SE_GENI_TX_FIFO_STATUS`) and the xmit fifo stay non-empty, so close
waits the full 30s. Setting `ASYNC_CLOSING_WAIT_NONE` is the documented
way to skip that wait (`tty_port.c:642-643`).
Nuance (verified, not assumed): `tty_port_shutdown()` returns
immediately when `port->console` is set, so **console close does not
call driver `shutdown()`**. Hangup (`uart_hangup()` → `uart_shutdown()`)
*does* shut the port down and does *not* use `closing_wait`. The 30s
stall is the close-time wait itself. Skipping it still fixes the
watchdog; the “so shutdown can cancel TX” wording is only partly
accurate for the close path.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Quality: **obviously correct** for the stated goal.
`ASYNC_CLOSING_WAIT_NONE` is the uapi sentinel (`65535`).
`uport->state` is valid in `startup()` because `uart_port_startup()`
calls `ops->startup()` only after the port is bound.
- Risk: last few console bytes on last close may not drain. That is
acceptable at power-off versus a watchdog reboot. Non-console ports
unchanged. No API/ABI change. Userspace can still override via
`TIOCSSERIAL`.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- `qcom_geni_serial_startup()` introduced in **c4f528795d1ad**
(2018-03-20, “Add serial driver support for GENI based QUP”).
- Default 30s `closing_wait` is in `tty_port_init()` and is old.
- Bug is long-standing in this driver, not a last-month regression.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Not applicable.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Standalone, not “patch X/Y”. Nearby later work (`dbe2afb952bdb`
reboot quiesce, panic notifier) is a different problem (DMA/SMMU on
reboot/panic), not a prerequisite. `a699213d4e6ef` “Fix blocked task” is
an unrelated PM-runtime hang revert.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Aniket Randive has a few qcom-geni commits (this one; debug-
console probe). Qualcomm platform engineer, not the TTY maintainer. Greg
KH applied it.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: **None.** Uses `uart_console()`, `tty_port.closing_wait`, and
`ASYNC_CLOSING_WAIT_NONE`, all older than this driver. Applies
standalone. In this tree, later PM-runtime lines sit around the same
function; the closing_wait hunk does not depend on them.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c acba38e675dd`: **[PATCH V1]** https://patch.msgid.link/2026
0706093749.836121-1-aniket.randive@oss.qualcomm.com
- `b4 dig -a`: **v1 only**; that is what was applied (2026-07-06 submit,
2026-07-10 by Greg).
- Saved mbox: **patch only, no replies, no NAK, no Cc: stable from
reviewers.**
- lore.kernel.org fetches returned 403; thread confirmed via b4 mbox +
Openwall/LKML mirrors.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To Greg Kroah-Hartman, Jiri Slaby (TTY
maintainers). Cc: linux-serial, linux-arm-msm, Krzysztof Kozlowski,
Qualcomm/Linaro GENI people. No extra Reviewed-by; maintainer SOB is the
review signal.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot, bugzilla, or public user report. Description matches
an internal Qualcomm power-off/watchdog failure. No independent repro in
the thread.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Single patch, not a series. Later “quiesce on reboot/panic”
series is independent.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search returned 403. No stable discussion
found via web search. **UNVERIFIED** whether anyone already requested or
rejected this for -stable.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
semcode MCP is not available (only the `cursor` namespace).
Callers/callees traced with grep/read.
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `qcom_geni_serial_startup` (modified). Related:
`tty_port_close_start`, `tty_wait_until_sent`, `uart_wait_until_sent`,
`qcom_geni_serial_tx_empty`, `qcom_geni_serial_shutdown`.
**Step 5.2: TRACE CALLERS**
Record: `.startup = qcom_geni_serial_startup` is in both FIFO and DMA
`uart_ops`. Called from `uart_port_startup()` on open (and
`uart_set_info` / console re-init). Reachable from userspace open of the
console tty and from console registration.
**Step 5.3: TRACE CALLEES**
Record: `uart_console()`, assignment to `tport->closing_wait`. No
alloc/lock in the new code.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace (systemd) close of `/dev/ttyMSM*` / console →
`tty_release` → `uart_close` → `tty_port_close_start` → 30s wait. **Yes,
reachable from userspace.** Trigger is power-off/getty stop with GENI as
console, not an obscure debug path.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: No other serial driver sets `closing_wait =
ASYNC_CLOSING_WAIT_NONE` at startup. USB-serial/cdc-acm/mxser only
expose it via ioctl. This is GENI-specific policy, not a tree-wide tty
change.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.**
- Tree: `v7.3-rc4-27-gf0100363d8c37`, Makefile `VERSION=7 PATCHLEVEL=3
SUBLEVEL=0 EXTRAVERSION=-rc4`
- Driver: `drivers/tty/serial/qcom_geni_serial.c` since 2018
- `qcom_geni_serial_startup()` present
- `tty_port_close_start()` still waits on `closing_wait`
- Commit **acba38e675dd** is an ancestor of HEAD (already applied here).
That is not used as “already selected”; it confirms the surrounding
code matches.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean in this tree** (already applied). Context in this tree
has later PM-runtime/`tx_kick_enabled` lines around startup; the hunk
itself is still the same 8 lines.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This *is* the fix. No alternate fix for console `closing_wait`.
Reboot/panic quiesce commits do not replace it.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **serial / qcom_geni** (`CONFIG_SERIAL_QCOM_GENI`, optional
`CONFIG_SERIAL_QCOM_GENI_CONSOLE`). **IMPORTANT** (not core mm/VFS):
Snapdragon phones, laptops, and embedded boards. Console + watchdog is a
production path on those platforms.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively developed in 7.x (DMA, nbcon, runtime PM, SA8255p). The
wait-on-close bug is still in the old 2018 startup/close design, not a
new-feature-only issue.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific + config-specific:** `ARCH_QCOM` (or
`COMPILE_TEST`) with GENI UART used as **console**. Non-console GENI
ports unchanged.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Power-off/reboot (or last close of the console tty) while TX is
still pending. Common on Android/embedded with systemd feeding the
watchdog. Unprivileged users do not typically close the system console;
**root/systemd** does. Not a local-priv-esc, still a production shutdown
failure.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Watchdog reset / failed power-off.** **HIGH** (hang then
reset). Can interrupt an otherwise orderly shutdown (unmount/sync still
in progress). Not a KASAN/UAF class bug.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** High for GENI-console devices: avoids 30s stall /
watchdog reboot on power-off.
- **Risk:** Very low: 8 lines, console-only, existing flag, no locking
change.
- **Ratio:** Benefit clearly outweighs risk.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real user-visible failure: shutdown blocked 30s → watchdog reset on
Qualcomm GENI console.
- Small, obviously correct use of `ASYNC_CLOSING_WAIT_NONE`.
- Console-only; non-console ports unchanged.
- No new APIs/features.
- Driver and bug path exist in this tree since the 2018 GENI UART
driver.
- Applied by TTY maintainer (Greg KH) as v1 with no NAK.
- Matches stable “hardware workaround / hang fix” practice.
AGAINST:
- No public Reported-by / Tested-by / syzbot.
- Workaround, not a hardware TX-stuck root-cause fix; last console bytes
on close may be dropped.
- Author’s “shutdown can cancel TX” claim is imprecise for console
*close* (`tty_port_shutdown` skips consoles).
- Affects one SoC UART family, not all machines.
UNRESOLVED:
- Stable list discussion (lore.kernel.org/stable 403).
- Independent reproducer beyond the commit text.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — mechanism matches tty_port;
merged by maintainer. Internal Qualcomm testing implied, not
independently reproduced here.
2. Fixes a real bug that affects users? **PASS** — power-off
hang/watchdog on GENI console.
3. Important (crash/security/corruption/deadlock)? **PASS** — watchdog
reset / failed shutdown is a serious hang/reset, not cosmetic.
4. Small and contained? **PASS** — +8 lines, one function, one file.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — `startup()`,
`uart_console()`, `tty_port`, `ASYNC_CLOSING_WAIT_NONE` all exist
here.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/DT/build/docs exception. It is a
**hardware/driver workaround** for a shutdown hang, which stable
routinely takes.
**Step 9.4: MAKE YOUR DECISION**
This tree has the GENI serial driver and the 30s close-wait path. The
patch is a small, console-only workaround that prevents a production
watchdog reset on power-off. That is within stable rules for this
checkout.
===============================================================
What the commit solves for this tree
===============================================================
Without the change, closing the GENI console tty waits up to 30s for TX
to drain. GENI TX can sit in an unfinished sequencer command, so the
wait often runs to the timeout. If systemd feeds the watchdog, the
machine resets instead of powering off. Setting `closing_wait` to
`ASYNC_CLOSING_WAIT_NONE` only for console ports skips that drain. Cost
is possible loss of the last console bytes on close; that is preferable
to a watchdog reboot.
===============================================================
Verification
===============================================================
- **[Phase 1]** Parsed tags: Link to msgid `20260706093749.836121-1`;
SOB Aniket Randive + Greg KH; no Fixes/Reported-by/Cc:stable.
- **[Phase 2]** Diff: +8 in `qcom_geni_serial_startup()`; sets
`tport->closing_wait` for `uart_console()`.
- **[Phase 2]** `tty_port_init`: default `closing_wait = (3000 * HZ) /
100` (30s). `tty_port_close_start` waits unless
`ASYNC_CLOSING_WAIT_NONE`.
- **[Phase 2]** `tty_port_shutdown`: returns early if `port->console` —
console close does not call driver shutdown. Hangup *does* call
`uart_shutdown()`.
- **[Phase 2]** `qcom_geni_serial_tx_empty` reads
`SE_GENI_TX_FIFO_STATUS`; stuck TX keeps wait alive.
- **[Phase 3]** `git describe HEAD` → `v7.3-rc4-27-gf0100363d8c37`;
`make kernelversion` → `7.3.0-rc4`.
- **[Phase 3]** `git merge-base --is-ancestor acba38e675dd HEAD` → yes
(present; not used as “already selected”).
- **[Phase 3]** Blame: startup from c4f528795d1ad (2018); closing_wait
lines from acba38e675dd.
- **[Phase 3]** `git merge-base --is-ancestor acba38e675dd v7.3-rc1` →
yes; not in v7.2.
- **[Phase 4]** `b4 dig -c` → v1 lore URL; `-a` → only v1; `-w` →
gregkh, jirislaby, linux-serial, linux-arm-msm; `-m` → no review
replies.
- **[Phase 4]** WebFetch lore → 403; Openwall/LKML mirrors match the
committed patch.
- **[Phase 5]** Callers: `uart_port_startup` → `ops->startup`; userspace
close path confirmed.
- **[Phase 6]** File and `startup()` exist; Kconfig `SERIAL_QCOM_GENI` /
`SERIAL_QCOM_GENI_CONSOLE`.
- **[Phase 8]** Failure: watchdog reset on power-off, severity HIGH,
risk of fix very low.
- **UNVERIFIED:** stable@ discussion (lore/stable 403). Independent
public reproducer.
**YES**
drivers/tty/serial/qcom_geni_serial.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 5d59a2cfab3e2..0807ca83d8078 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1244,6 +1244,7 @@ static int qcom_geni_serial_startup(struct uart_port *uport)
{
int ret;
struct qcom_geni_serial_port *port = to_dev_port(uport);
+ struct tty_port *tport = &uport->state->port;
if (!port->setup) {
ret = qcom_geni_serial_port_setup(uport);
@@ -1251,6 +1252,13 @@ static int qcom_geni_serial_startup(struct uart_port *uport)
return ret;
}
+ /*
+ * Skip the close-time transmit drain for console ports so that
+ * shutdown can proceed without waiting for pending TX completion.
+ */
+ if (uart_console(uport))
+ tport->closing_wait = ASYNC_CLOSING_WAIT_NONE;
+
uart_port_lock_irq(uport);
qcom_geni_serial_start_rx(uport);
uart_port_unlock_irq(uport);
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH AUTOSEL 6.18-6.6] serial: qcom_geni: Disable closing_wait for console to prevent shutdown timeout
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-6.6] serial: qcom_geni: Disable closing_wait for console to prevent shutdown timeout Sasha Levin
@ 2026-10-19 15:51 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-19 15:51 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-serial
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] serial: qcom_geni: uncancelled console TX in qcom_geni_serial_startup()
--
commit 410ad55ea7c02110e68e77ae15503cb551a22bf5
Author: Aniket Randive <aniket.randive@oss.qualcomm.com>
serial: qcom_geni: Disable closing_wait for console to prevent shutdown timeout
This commit sets closing_wait to ASYNC_CLOSING_WAIT_NONE in
qcom_geni_serial_startup() for console ports so that closing the console
TTY bypasses tty_wait_until_sent() during system shutdown.
Link: https://patch.msgid.link/20260706093749.836121-1-aniket.randive@oss.qualcomm.com
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 8058b839b26ce..219ac88b0fab2 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1227,6 +1227,7 @@ static int qcom_geni_serial_startup(struct uart_port *uport)
> {
> int ret;
> struct qcom_geni_serial_port *port = to_dev_port(uport);
> + struct tty_port *tport = &uport->state->port;
>
> if (!port->setup) {
> ret = qcom_geni_serial_port_setup(uport);
> @@ -1234,6 +1235,13 @@ static int qcom_geni_serial_startup(struct uart_port *uport)
> return ret;
> }
>
> + /*
> + * Skip the close-time transmit drain for console ports so that
> + * shutdown can proceed without waiting for pending TX completion.
> + */
> + if (uart_console(uport))
> + tport->closing_wait = ASYNC_CLOSING_WAIT_NONE;
[Severity: Medium]
Does uart_shutdown() actually get called to cancel pending TX commands when a
console port is closed?
The commit message states that setting closing_wait to ASYNC_CLOSING_WAIT_NONE
bypasses tty_wait_until_sent() on close, allowing uart_shutdown() to cancel TX
commands cleanly.
However, when uart_console(uport) is true, serial_core_add_one_port() sets
port->console:
drivers/tty/serial/serial_core.c:serial_core_add_one_port() {
...
port->console = uart_console(uport);
...
}
When userspace closes the final file descriptor on the console TTY via
tty_release() -> uart_close() -> tty_port_close(), tty_port_close_start()
skips tty_wait_until_sent() without calling tty_driver_flush_buffer() (unless
tty->flow.tco_stopped is set):
drivers/tty/tty_port.c:tty_port_close_start() {
...
if (tty->flow.tco_stopped)
tty_driver_flush_buffer(tty);
if (port->closing_wait != ASYNC_CLOSING_WAIT_NONE)
tty_wait_until_sent(tty, port->closing_wait);
...
}
Then tty_port_close() calls tty_port_shutdown(), which returns early for
console ports without invoking port->ops->shutdown() (uart_tty_port_shutdown()
-> uart_port_shutdown() -> qcom_geni_serial_shutdown()):
drivers/tty/tty_port.c:tty_port_shutdown() {
struct tty_struct *tty = port->tty;
mutex_lock(&port->mutex);
if (port->console)
goto out;
...
}
After that, tty_port_close_end() clears TTY_PORT_ACTIVE via
tty_port_set_active(port, false), so even a subsequent uart_hangup() (which
checks tty_port_active(port)) will not call uart_flush_buffer() or
uart_shutdown().
Doesn't this leave pending GENI sequencer TX commands (UART_START_TX), TX
interrupts, and unsent bytes in tport->xmit_fifo active after the console TTY
is closed, causing subsequent qcom_geni_serial_console_write() calls to poll
until timeout before cancelling the stalled command and re-triggering
qcom_geni_serial_isr() -> qcom_geni_serial_handle_tx_fifo() on the leftover
xmit_fifo data?
> +
> uart_port_lock_irq(uport);
> qcom_geni_serial_start_rx(uport);
> uart_port_unlock_irq(uport);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-515-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread