Re: [PATCH net v7 4/4] net/smc: bound the link failover cursor advance to the send buffer

[Bryam Vargas]

On Thu, 15 Oct 2026 20:15:24 +0000, netdev-bot+sashiko@xxxxxxxxxx wrote:
> [Severity: High]
> Does this fully close the destination side of the overrun that the commit
> message describes? The new cons.count check only decides whether the
> catch-up branch runs. The forged cursor is not rejected and stays in
> conn->local_rx_ctrl.cons.

No, and v8's message no longer claims it. 4/4 bounds our side, the
source cursors and sndbuf_space; smc_write_space() still sizes the
window from the peer's cursor, so a forged one can take the next write
past the peer's own RMB, where its HCA refuses it and the link goes
down. Measured on rxe: with the consumer cursor forged two wraps ahead
at the peer's RMB + 4096, the first write after the switch, from
prod.count 1 with a whole-RMB window, ends one byte past the peer's RMB
and is refused; a cursor one wrap ahead gets the send's own write
refused the same way with or without the patch, no failover needed. The
write targets the forger's own memory in both cases. Bounding cons at
import, as you suggest, is a different change: cons indexes the peer's
RMB (1/4's reason for leaving it alone), and a forged one harms only
that RMB.

> [Severity: Medium]
> This isn't a bug introduced by this patch, but the two users of the wire
> consumer cursor now handle it differently. smc_switch_cursor() bounds its
> use of local_rx_ctrl.cons. smc_cdc_msg_recv_action() in net/smc/smc_cdc.c
> still credits the window without a limit:

They do. The second chunk can run past the peer's RMB, same answer as
above: the forger's own memory, and its HCA refuses the write; the
receive-path credit is pre-existing and not this series.

> [Severity: Medium]
> This is a pre-existing issue, but smc_curs_comp() in net/smc/smc_cdc.h
> compares the u16 wrap fields as plain numbers:

Yes: at the 65535 -> 0 rollover a cursor ahead reads as behind, the
catch-up is skipped and smc_write_space() goes negative; nothing more is
sent, so, with nothing left for the peer to consume, no cons update
reopens the window and the connection stalls until it is closed. One
window per 65536 wraps of the peer's RMB, before and after this patch.

> [Severity: Low]
> Two cases leave local_tx_ctrl.prod behind local_rx_ctrl.cons: the prep
> term of this min() wins, or the branch is skipped because
> cons.count >= peer_rmbe_size. The comment above still says the catch-up
> fixes prod < cons.

Intended: neither case happens with a conforming peer; a forging one
stalls its own connection, or has its next write refused as above.
v8's comment says why the catch-up is bounded.

> [Severity: High]
> This isn't a bug introduced by this patch, but the RDMA write takes its
> offsets from these cursors. Is the source side of smcr_tx_rdma_writes() in
> net/smc/smc_tx.c bounded for the second destination chunk?

By inspection it is not; reachable only once the receive-path credit
above has pushed peer_rmbe_space past the peer's RMB. Not reproduced;
pre-existing, SMC-R only, not this series.

> [Severity: High]
> This is a pre-existing issue, but smc_switch_cursor() reads and updates
> tx_curs_fin, local_tx_ctrl_fin and sndbuf_space while holding only
> conn->send_lock. smc_switch_conns() takes that lock:

Yes to both: the interleaving credits the handler's bytes a second time,
the race from your 3/4 comment (send_lock on one side, bh_lock_sock() on
the other), and after the over-credit the copy stays inside the buffer
but can overwrite data the peer has not received yet. Pre-existing.