Linux kernel -stable discussions
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: stable@vger.kernel.org
Cc: Sasha Levin <sashal@kernel.org>
Subject: [PATCH 5.10.y 2/2] [AUTOMATED CONFLICT RESOLUTION] Documentation for 6b4f48728faa8bb514368f7eacda05565dea8696
Date: Sat, 25 Jul 2026 06:55:27 -0400	[thread overview]
Message-ID: <20260725105527.1856073-2-sashal@kernel.org> (raw)
In-Reply-To: <20260725105527.1856073-1-sashal@kernel.org>

This commit documents the automated conflict resolution performed by claude
for upstream commit 6b4f48728faa8bb514368f7eacda05565dea8696.

claude's resolution process:
================================================================================
Build still in progress (no output yet). Waiting for completion — I'll report once it lands.
================================================================================

Backport verification analysis (via claude):
================================================================================
## Verdict: correct, faithful, minimal — context-only adaptation

### 1. Intent preservation

The upstream fix is two lines added as the first statement inside the `type == P9_TFLUSH` retry branch of `p9_client_rpc()`. The backport adds those **exact same two lines, byte-identical, at the exact same logical position** (`net/9p/client.c:773-774`):

```c
	if ((err == -ERESTARTSYS) && (c->status == Connected)
				  && (type == P9_TFLUSH)) {
		if (fatal_signal_pending(current))
			goto recalc_sigpending;
		sigpending = 1;
		clear_thread_flag(TIF_SIGPENDING);
		goto again;
	}
```

The `recalc_sigpending:` label exists in 5.10's `p9_client_rpc()` at `client.c:797`, in the same relative position and with byte-identical body to upstream:

```c
recalc_sigpending:
	if (sigpending) {
		spin_lock_irqsave(&current->sighand->siglock, flags);
		recalc_sigpending();
		spin_unlock_irqrestore(&current->sighand->siglock, flags);
	}
	if (err < 0)
		goto reterr;
```

So both post-conditions the commit message promises hold in 5.10:
- `sigpending == 1` (a signal was pending at RPC entry) → `recalc_sigpending()` restores `TIF_SIGPENDING`.
- `sigpending == 0` → `TIF_SIGPENDING` was never cleared on this path, so it's still set. Correct either way.
- `err == -ERESTARTSYS < 0` → `goto reterr` → `p9_tag_remove()` → `ERR_PTR(safe_errno(-ERESTARTSYS))`. `safe_errno()` (`client.c:94`) passes `-512` through unchanged (`-512 > -MAX_ERRNO`), so the caller genuinely sees `-ERESTARTSYS`, not a mangled `-EPROTO`.

### 2. Differences vs upstream — all pre-existing context, none functional

The only diff-level deltas are blob hashes, hunk line numbers (600 → 771), and the `if` condition's parenthesization:

| upstream | 5.10 |
|---|---|
| `if (err == -ERESTARTSYS && c->status == Connected &&`<br>`    type == P9_TFLUSH) {` | `if ((err == -ERESTARTSYS) && (c->status == Connected)`<br>`			  && (type == P9_TFLUSH)) {` |

That is purely the v5.17-era 9p checkpatch-style reflow, semantically identical. Other pre-existing divergences in the enclosing function (`wait_event_killable` vs `io_wait_event_killable`, no `READ_ONCE(req->status)`, `p9_req_put(req)` vs `p9_req_put(c, req)`, `p9_tag_remove()` vs `p9_req_put()` at `reterr:`) are all upstream-only refactors from 6.1/6.2 and are **orthogonal to this fix** — none of them is touched, and none changes the signal semantics the fix depends on.

Critically, the killable-wait semantics are the same: 5.10's `wait_event_killable` returns `-ERESTARTSYS` only via `signal_pending_state(TASK_KILLABLE, ...)`, which for `TASK_KILLABLE` (`TASK_WAKEKILL|TASK_UNINTERRUPTIBLE`, no `TASK_INTERRUPTIBLE` bit) requires `__fatal_signal_pending()`. So at the new check `TIF_SIGPENDING` is still set and `SIGKILL` is still in `current->pending.signal` — nothing between the wait returning and the check dequeues signals or calls `recalc_sigpending()`. `fatal_signal_pending()` therefore evaluates exactly as upstream intends. `io_wait_event_killable` differs only in io_schedule accounting.

### 3. Concerns checked and cleared

- **Build/compile**: `<linux/sched/signal.h>` is already included (`client.c:20`), and `signal_pending()` is already used ~25 lines above in the same function — `fatal_signal_pending()` needs no new include. `goto recalc_sigpending` targets a label already jumped to by the pre-existing `trans_mod->request()` failure path, so no "label used but not defined". Label vs. function name `recalc_sigpending` live in different C namespaces (already the case pre-patch). `git show --check` is clean; diffstat is exactly `1 file changed, 2 insertions(+)` — no stray hunks.
- **Applicability**: `Fixes: 91b8534fa8f5` (v3.7) is confirmed an ancestor of HEAD, and the commit message explicitly states the defect reaches back to 5.4. 5.10.261 is in scope.
- **No duplicate/competing fix**: `fatal_signal_pending` appears exactly once in all of `net/9p/`, at the new line. No prior stable-only patch to collide with.
- **`p9_client_zc_rpc()`**: correctly untouched — 5.10's zc variant has no `again:`/`P9_TFLUSH` retry loop, matching upstream, so the one-site fix is complete.
- **Abandoned-TFLUSH lifetime (the one thing worth naming)**: the new early exit means `p9_client_flush()` returns `-ERESTARTSYS` before calling `trans_mod->cancelled(oldreq)`, so `oldreq` stays on the transport's `req_list`. In 5.10 the flush req itself then goes through `reterr: p9_tag_remove()`, which does an *eager* `idr_remove()` + put, whereas upstream ≥6.1 removes from the idr only on the final `p9_req_put()`. I checked whether this makes 5.10 more exposed: it doesn't. `trans_fd` takes its own reference at `trans_fd.c:475` before moving a req to `req_list`, so refcount goes 2→1 and nothing is freed; a late response for a now-unknown tag hits the `!m->rreq` guard in `p9_read_work()` (`trans_fd.c:336`) which logs "Unexpected packet tag" and errors out rather than dereferencing; and `p9_conn_cancel()` completes and puts the stranded req at disconnect. Identical exposure to the pre-existing `err == -ERESTARTSYS && c->status == Connected` path and to `trans_mod->request()` failure. **No UAF, no leak beyond disconnect, and no 5.10-specific regression.**
- **Behavioral note (upstream-inherited, not a backport defect)**: as analyzed above, whenever this branch is reached `fatal_signal_pending()` is necessarily true, so the `sigpending = 1; clear_thread_flag(); goto again;` retry becomes effectively unreachable. That is upstream's deliberate semantics ("return `-ERESTARTSYS` to the caller"), and 5.10 reproduces it exactly because its wait is equally killable-only. The backport does not amplify or alter this.

### 4. Overall assessment

**High quality — accept.** This is the minimal possible adaptation: the added code is byte-identical to upstream, the target label and its body are identical, all supporting semantics (`safe_errno`, killable-wait signal conditions, `sigpending` bookkeeping) verify out in 5.10, and the surrounding version drift is entirely cosmetic/orthogonal. The only "resolution" required was matching the older parenthesized `if` context.

Build verification is the one item still outstanding — `stable build log` (x86_64 + aarch64) is running now; I'll report the result when it lands. Given the change adds no new symbols or includes, a failure would be surprising.
================================================================================

This is an empty commit for documentation purposes only.
-- 
2.53.0


      reply	other threads:[~2026-07-25 10:55 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20 14:32 FAILED: patch "[PATCH] net/9p: fix infinite loop in p9_client_rpc on fatal signal" failed to apply to 5.10-stable tree gregkh
2026-07-25 10:55 ` [PATCH 5.10.y 1/2] net/9p: fix infinite loop in p9_client_rpc on fatal signal Sasha Levin
2026-07-25 10:55   ` Sasha Levin [this message]

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20260725105527.1856073-2-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

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

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