All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.