Re: [PATCH net] net/smc: validate peer CDC cursor against RMBE size before accepting it

[Bryam Vargas <hexlabsecurity@proton.me>]

Agreed, and thanks for dropping the competing one.

Patch 1/3 bounds only the cursor count, not the advance: smc_curs_diff()'s
differing-wrap branch returns (len - old.count) + new.count, so prod(W,0) then
prod(W+1,len) is diff_prod == 2*len with both counts in range, and
smc_cdc_msg_recv_action() adds that to bytes_to_rcv unclamped. wrap++/count==0 does the
same over several CDCs, so a per-cursor bound can't catch it.

The out-of-bounds copy you point at is closed by patch 2/3, though. It bounds the
derived readable length at the consumer:

	if (readable < 0 || readable > conn->rmb_desc->len)
		readable = conn->rmb_desc->len;

so copylen <= rmb_desc->len and smc_rx_recvmsg()'s second chunk (copylen - first,
offset 0) stays within the ring no matter how large bytes_to_rcv grew. I ran your
wrap++/count=0 vector end to end on the real SMC-D path under KASAN: with 1/3 alone it
trips slab-out-of-bounds in smc_rx_recvmsg (bytes_to_rcv reaches 6*len, read of size
5*len); with 2/3 applied recv() is bounded to rmb_desc->len and it is clean.

What 2/3 does not fix is the accounting: bytes_to_rcv is left > rmb_desc->len (and can
sign-overflow negative over many messages), so the documented
0 <= bytes_to_rcv <= rmb_desc->len invariant is still violated at the producer even
though the copy is now bounded. I have a net-next follow-up that records and aborts on
that. Your

	if (smc_curs_diff(size, &old, &temp) > size)
		return;

catches the single-CDC 2*len advance cleanly; the wrap++/count==0 form slips it, since
each CDC advances exactly len (diff == len, not > len) and it is the accumulated
bytes_to_rcv that overruns. So I'll pair your advance-bound as an early catch with a
check on bytes_to_rcv after the atomic_add and abort+record there, which covers both.
The insight is yours either way -- may I add your Suggested-by (or Co-developed-by)?

The v4 series otherwise stands as reviewed; I'll repost it for the netdev queue and
keep the accounting bound as the net-next follow-up.

Thanks,
Bryam