[PATCH net] net/smc: queue the copied chunk when smc_tx_sendmsg() faults

Bryam Vargas via B4 Relay posted 1 patch 1 week, 3 days ago
net/smc/smc_tx.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH net] net/smc: queue the copied chunk when smc_tx_sendmsg() faults
Posted by Bryam Vargas via B4 Relay 1 week, 3 days ago
From: Bryam Vargas <hexlabsecurity@proton.me>

When a send wraps the end of the send buffer, smc_tx_sendmsg() copies
the data in two chunks. If memcpy_from_msg() faults on the second one,
smc_tx_sendmsg() returns send_done, which already counts the first
chunk, but leaves tx_curs_prep and sndbuf_space as they were. send()
reports bytes the peer never receives, and the next send() overwrites
them. Any caller can hit this on its own stream with a buffer whose
tail is unreadable.

On a fault in the second chunk, queue the first one. The next pass
retries the rest and returns send_done if it faults again, so send()
reports only what it queued, as TCP does.

Fixes: e6727f39004b ("smc: send data (through RDMA)")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Repro on v7.3-rc6 with KASAN (its smc_tx.c is the same as net's),
SMC-D loopback: send 33000 bytes, wait until the peer has read them
and SIOCOUTQ is 0, then send(S, MSG_DONTWAIT) from a buffer whose
first S - 500 bytes are readable and whose next page is PROT_NONE.
The 65504-byte sndbuf wraps after 32504 bytes, so at S = 65504 and
65536 the fault hits the second chunk:

  unpatched: send() returns 32504, SIOCOUTQ stays 0, and the peer
             gets only the 100-byte marker sent next.
  patched:   send() returns 32504 and the peer gets those bytes,
             intact, then the marker.

The 12 other sizes (16352 to 1048576) fault in the first chunk or not
at all, and give the same result on both kernels: -EFAULT, or 65504
sent and delivered. With the whole buffer readable, both deliver what
send() reports at every size. TCP at S = 65504 on the faulting buffer
returns 47616 and delivers exactly that. No KASAN splat. Logs on
request.

The rx side has the same shape: smc_rx_recvmsg() returns the first
chunk without moving the cursor when memcpy_to_msg() faults on the
second. Hidayath said in [1] that his v2 will fix it along with the
splice path, so this only touches smc_tx.c. It applies to net as is
and on top of [2], whose 3/4 also changes smc_tx_sendmsg(); the repro
gives the same results there.

[1] https://lore.kernel.org/all/7a174d1f-80d8-4294-ae66-829c9dacd662@linux.ibm.com/
[2] https://lore.kernel.org/all/20261013-b4-disp-f345196c-v7-0-ef3dddfbfa68@proton.me/
---
 net/smc/smc_tx.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 3144b4b1fe29..495fb1354728 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -249,6 +249,13 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
 			rc = memcpy_from_msg(sndbuf_base + chunk_off,
 					     msg, chunk_len);
 			if (rc) {
+				if (chunk) {
+					/* send_done counts the first chunk:
+					 * queue it and retry the rest
+					 */
+					copylen -= chunk_len;
+					break;
+				}
 				smc_sndbuf_sync_sg_for_device(conn);
 				if (send_done)
 					return send_done;

---
base-commit: 513444cc62e231a11c9b15ea88de0890da9b2b4f
change-id: 20261013-b4-disp-872a3689-f4666b3d8b6b

Best regards,
--  
Bryam Vargas <hexlabsecurity@proton.me>
Re: [PATCH net] net/smc: queue the copied chunk when smc_tx_sendmsg() faults
Posted by netdev-bot+sinfo@kernel.org 1 week, 3 days ago
Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.