Re: [PATCH v3 2/3] net/smc: bound the receive length to the RMB in smc_rx_recvmsg()

[Bryam Vargas <hexlabsecurity@proton.me>]

On Fri, 19 Jun 2026 11:31:18 +0800, Dust Li wrote:
> I think we can decide after we see the real issue.

Here it is, as a truth table over the real smc_curs_diff. cons is fixed (the app
isn't reading), bytes_to_rcv is the running sum of per-CDC smc_curs_diff(prod_old,
prod_new), len = 65504:

  scenario                     b2r       count>=len  diff>len  occ>len  OOB no-clamp  OOB clamp
  honest steady / full / wrap  <= len    no          no        no       no            no
  attack single big diff       131007    no          yes       yes      yes           no
  attack count=len-1 wrapflip  327519    no          yes       yes      yes           no
  attack wrap++ count=0        327520    no          no        no       yes           no

Every attack row has count < len, so an input count check accepts it. The last
row is the one that matters: a peer that just increments prod.wrap with count=0
adds len to bytes_to_rcv every CDC, unbounded, and no cursor-level check sees it.
The per-CDC diff is exactly len, and smc_curs_diff(cons, prod) stays at len
because it can't see the wrap accumulation. The only thing that bounds it is
clamping bytes_to_rcv at the consumer. So #2 isn't subsumed by validating cursors
at the input -- the cursor view can't see the accumulator.

> should we also abort the connection like what we did in patch #1 ?

Yes for net-next. Two caveats: First, the detection
has to be on bytes_to_rcv itself, not on a cursor recompute -- the wrap++ row
walks past every cursor check, so an occupancy gate at the input wouldn't catch
it. Second, the abort supplements the clamp, it doesn't replace it: the clamp is
synchronous, the abort via queue_work isn't. The producer add runs in the tasklet
under bh_lock_sock, the consumer sub runs in smc_recvmsg under lock_sock which
drops the spinlock, so they race; between queue_work and abort_work running
smc_conn_kill, smc_recvmsg can read the inflated bytes_to_rcv and copy past the
RMB. The clamp at the consumer is what closes that window.

So v4: -stable keeps the consumer-side clamp on #2, and the same shape on #3 for
sndbuf_space and peer_rmbe_space -- no control-flow change. net-next keeps the
clamp and, when bytes_to_rcv goes over len (which an honest peer never does),
queues the abort the way patch #1 does. Patch #1 keeps its count-based abort for
the urgent index.

Bryam

The table above is this program (gcc -O2 -Wall -Wextra -fwrapv; self-checks, exit 0):

  #include <stdio.h>
  #include <stdint.h>
  typedef uint16_t u16; typedef uint32_t u32;
  union hc { struct { u16 reserved; u16 wrap; u32 count; }; };

  /* verbatim net/smc/smc_cdc.h:149-158 */
  static int smc_curs_diff(unsigned int size, const union hc *old, const union hc *new)
  {
          if (old->wrap != new->wrap) {
                  int v = (int)((size - old->count) + new->count);
                  return v > 0 ? v : 0;
          }
          { int v = (int)(new->count - old->count); return v > 0 ? v : 0; }
  }

  #define LEN 65504
  struct cur { u16 w; u32 c; };

  /* prod[]/cons[]: cursor positions after each CDC. honest=app drains so
   * occupancy stays <= len; attack=cons stuck. */
  static int run(const char *name, int honest, int n,
                 const struct cur *prod, const struct cur *cons)
  {
          union hc po = {0}, co = {0};
          long b2r = 0; int i, cnt_rej = 0, raw_rej = 0, occ_rej = 0, fail = 0;
          for (i = 0; i < n; i++) {
                  union hc p = { .wrap = prod[i].w, .count = prod[i].c };
                  union hc c = { .wrap = cons[i].w, .count = cons[i].c };
                  int dp = smc_curs_diff(LEN, &po, &p);
                  if (prod[i].c >= (u32)LEN) cnt_rej = 1;
                  if (dp > LEN) raw_rej = 1;
                  if (smc_curs_diff(LEN, &c, &p) > LEN) occ_rej = 1;
                  b2r += dp; b2r -= smc_curs_diff(LEN, &co, &c);
                  po = p; co = c;
          }
          int oob_noclamp = b2r > LEN;
          int oob_clamp   = (b2r > LEN ? LEN : b2r) > LEN;   /* always 0 */
          printf("  %-30s b2r=%-8ld cnt_rej=%d raw_rej=%d occ_rej=%d oob_noclamp=%d oob_clamp=%d\n",
                 name, b2r, cnt_rej, raw_rej, occ_rej, oob_noclamp, oob_clamp);
          if (honest) fail = (cnt_rej || raw_rej || occ_rej || oob_noclamp);
          else        fail = (oob_clamp || !oob_noclamp);
          return fail;
  }

  int main(void)
  {
          struct cur ps[][5] = {
                  {{0,5000}}, {{1,0}}, {{0,30000},{0,60000},{1,10000}},
                  {{1,LEN-1}},
                  {{1,LEN-1},{0,LEN-1},{1,LEN-1},{0,LEN-1}},
                  {{1,0},{2,0},{3,0},{4,0},{5,0}},
          };
          struct cur cs[][5] = {
                  {{0,4000}}, {{0,0}}, {{0,0},{0,30000},{0,50000}},
                  {{0,0}},
                  {{0,0},{0,0},{0,0},{0,0}},
                  {{0,0},{0,0},{0,0},{0,0},{0,0}},
          };
          const char *nm[] = { "honest: steady", "honest: full ring",
                  "honest: wrapping", "attack: single big diff",
                  "attack: count=len-1 wrapflip", "attack: wrap++ count=0" };
          int hon[] = { 1,1,1,0,0,0 };
          int nc[]  = { 1,1,3,1,4,5 };
          int i, fails = 0;
          for (i = 0; i < 6; i++)
                  fails += run(nm[i], hon[i], nc[i], ps[i], cs[i]);
          printf("RESULT: %s\n", fails ? "FAIL" : "PASS");
          return fails ? 1 : 0;
  }

(In-kernel KASAN confirming the over-read at count=65503 is available on request;
a small out-of-tree module driving the same smc_curs_diff over a real
rmb_desc->len allocation -- bytes_to_rcv 131007 -> 327519, slab-out-of-bounds in
the recv copy, clean with the clamp.)