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

Ibrahim Hashimov <[email protected]>
Newsgroups org.kernel.vger.linux-rdma,org.kernel.vger.linux-kernel,org.kernel.vger.linux-s390,org.kernel.vger.netdev
Message-ID <[email protected]>
> I believe Bryam already sent a similar patch to the mailist and I have
> already reviewed it.
> https://lore.kernel.org/netdev/[email protected]/

Thanks Dust -- yes, that fixes the same bug, so please drop mine. (I sent
a v2 earlier today, before I saw your note, in reply to a sashiko review;
please disregard it as a competing patch.)

One thing worth checking on Bryam's series, though: bounding only the cursor
count (clamping temp.count to rmb_desc->len) still leaves the advance
unbounded. smc_curs_diff() on a wrap increment returns
(size - old.count) + new.count, so a peer that sends prod (wrap=W, count=0)
then prod (wrap=W+1, count=size) makes diff_prod ~= 2*size even though both
counts are in range. smc_cdc_msg_recv_action() then atomic_add()s that into
bytes_to_rcv without clamping (despite the "0 <= bytes_to_rcv <=
rmb_desc->len" comment) -- which is exactly what smc_rx_recvmsg()'s second
copy chunk trusts.

A sashiko review of my patch flagged the same gap; I bounded the advance
too, with:

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

Might be worth folding into Bryam's version. Happy to send it as a
follow-up if that helps.

Thanks,
Ibrahim
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.