Re: [PATCH net-next] net/smc: abort the connection when the peer overruns the RMB
From: Hidayath Khan
Date: Tue Aug 11 2026 - 13:45:30 EST
On 08/08/26 1:42 pm, Bryam Vargas wrote:
Hidayath,Hi Bryam,
I have a standalone net-next patch that aborts the connection whenThat covers the follow-up I said I would send, and the placement is better than
bytes_to_rcv + diff_prod exceeds rmb_desc->len. The check sits before
the atomic_add(), so the accumulator is never written with an out-of-range
value
what I described: I had said a check after the atomic_add, which only notices
the counter is already out of range. Yours doesn't let it get there. Consider my
follow-up withdrawn -- I am not sending a competing patch.
If it is useful for the Fixes decision: I ran the wrap++/count==0 vector on the
real SMC-D path under KASAN while working on the cursor series. With only the
per-cursor bound applied, bytes_to_rcv reaches 6*len and smc_rx_recvmsg() trips
slab-out-of-bounds on a read of 5*len; each CDC advances exactly len, so
diff == len and an advance-bound does not fire -- it's the accumulation that
overruns, which is what your check catches. Logs on request if you want them in
the commit message.
Yes, please send the logs. I would like to put the splat in the commit
message and credit you for the reproduction. Right now my changelog only
talks about the accounting damage -- SIOCINQ showing a length that is not
there, and poll() staying readable with nothing to read. A real
slab-out-of-bounds read is a much stronger claim. Any format is fine, I
will trim it.
Your point that diff == len on every message, so an advance bound never
fires, is also worth putting in the changelog. I had argued that only from
the arithmetic, not from a run.
Agreed. From reading the code they are in different parts of the function
Two heads-up on collisions, since both are in flight this week rather than
merged:
smc_cdc_msg_recv_action() is also touched by "net/smc: order the CDC receive
path against buffer publication" (v4, 20260728-b4-disp-52ee4e7d-v4-1-0dda94b0f397@xxxxxxxxx),
which hoists sndbuf_desc to the top of the function and gates the tx-trigger on
it. Your hunk sits just above that gate, so whichever lands second will want a
look rather than a blind rebase. I'd rather flag it now than after a conflict.
-- my hunk sits above the tx-trigger gate you add.
But my v2 also adds an out_of_sync check to
smcd_cdc_rx_tsklet(), and your v4 edits the same line:
yours: keeps "if (!conn || conn->killed)" and adds the rmb_desc
smp_load_acquire() after it
mine: rewrites it to "if (!conn || conn->killed || conn->out_of_sync)"
So they conflict.
Confirmed, that one conflicts too. My v2 hunk moves
And you mentioned running the abort_work cancel for both transports in v2 --
that edits smc_conn_free()'s SMC-D branch, which "net/smc: unregister the
connection before draining the rx tasklet"
(20260808-b4-disp-22f119e6-v2-1-61647601a6f3@xxxxxxxxx) also rewrites: it drops
the !list_empty guard around smc_ism_unset_conn(), moves the drain ahead of the
detach, and clears conn->sndbuf_desc before freeing it. Same branch, same week.
cancel_work_sync(&conn->abort_work) out of the non-SMC-D branch so it runs
for both transports.
So my v2 now conflicts with both of your patches. Both of yours are posted
and mine is not, so I will rebase on top of both rather than ask you to
work around me.
If you would rather take the abort_work cancel into your
teardown 1/2 while you are already in that branch, please say so and I will
drop that hunk.
Thanks, I will send it separately.
On the shared bitfield -- agreed it needs a layout change rather than something
folded into a fix, and it's yours; I'd noted it and left it alone for the
same reason.
One more thing, for information. I have sent "net/smc: fix use-after-free
in smc_rx_pipe_buf_release()" to the list. It clears conn->rmb_desc in
smc_buf_unuse(), which is the rmb version of the sndbuf_desc clear in your
teardown 1/2. These are different functions, so from inspection they
should not conflict.
Thanks,
Thanks,
Bryam
Hidayath