Re: [PATCH net v4 3/3] net/smc: bound the send length to the send buffer in smc_tx_sendmsg()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Partial write failure in `smc_tx_sendmsg` causes silent data loss and stream corruption.
--

commit 1bdb319d30163889b9beebc58c1c90f65092496b
Author: Bryam Vargas <hexlabsecurity@proton.me>

net/smc: bound the send length to the send buffer in smc_tx_sendmsg()

This patch bounds the writespace to the send buffer length to prevent
an out-of-bounds write caused by a forged cursor. It ensures the
writespace is always properly bounded before starting the message copy.

> diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
> index 3144b4b1fe290..5916f02060fb9 100644
> --- a/net/smc/smc_tx.c
> +++ b/net/smc/smc_tx.c
> @@ -233,6 +233,19 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
>  		/* initialize variables for 1st iteration of subsequent loop */
>  		/* could be just 1 byte, even after smc_tx_wait above */
>  		writespace = atomic_read(&conn->sndbuf_space);
> +		/* sndbuf_space is advanced from the peer's wire-controlled
[ ... ]
> +		if (writespace < 0 || writespace > conn->sndbuf_desc->len)
> +			writespace = conn->sndbuf_desc->len;
>  		/* not more than what user space asked for */
>  		copylen = min_t(size_t, send_remaining, writespace);
>  		/* determine start of sndbuf */

[Severity: High]
This is a pre-existing issue, but does this error path cause silent data
loss?

If memcpy_from_msg() fails on the second iteration of the chunk loop,
send_done has already been incremented by the first chunk's length:

smc_tx_sendmsg() {
    ...
    for (chunk = 0; chunk < 2; chunk++) {
        rc = memcpy_from_msg(sndbuf_base + chunk_off,
                             msg, chunk_len);
        if (rc) {
            smc_sndbuf_sync_sg_for_device(conn);
            if (send_done)
                return send_done;
            goto out_err;
        }
        send_done += chunk_len;
        ...
    }
    ...
    /* update cursors */
    smc_curs_add(conn->sndbuf_desc->len, &prep, copylen);
    smc_curs_copy(&conn->tx_curs_prep, &prep, conn);
    ...
}

By returning send_done early when a multi-chunk copy partially succeeds,
we skip the cursor updates. Userspace will believe the first chunk was
successfully written and advance its state, but the kernel will overwrite
those uncommitted bytes on the next sendmsg() call because the cursor
was never advanced.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705-b4-disp-28a1bbca-v4-0-be089b98acc6@proton.me?part=3