[PATCH] nvmet-tcp: add bounds checks in nvmet_tcp_build_pdu_iovec

[yunje shin]

Hi,

When I wrote the patch, I followed the existing convention of calling
nvmet_tcp_fatal_error() directly. But since the function returns void,
having reviewed the AI agent's analysis, the concern seems to have
some merit - the caller could overwrite rcv_state with
NVMET_TCP_RECV_DATA after the failure, causing the state machine to
proceed with an uninitialized cmd->recv_msg.msg_iter.

Maurizio's approach of changing the return type to int and letting
callers propagate the error makes sense. This also aligns with his
earlier patch [1] to remove redundant nvmet_tcp_fatal_error() calls
from lower-level functions.

To confirm this, would the following sequence trigger the issue?

1. Send a crafted H2C Data PDU with an invalid data_offset or length
   that causes sg_idx >= sg_cnt in nvmet_tcp_build_pdu_iovec()
2. The bounds check triggers -> nvmet_tcp_fatal_error() sets
   rcv_state = NVMET_TCP_RECV_ERR -> early return (void)
3. Back in nvmet_tcp_handle_h2c_data_pdu(), the caller overwrites
   rcv_state with NVMET_TCP_RECV_DATA (ignoring the failure)
4. The state machine proceeds to nvmet_tcp_try_recv_data(),
   which calls recvmsg() using the uninitialized cmd->recv_msg.msg_iter

[1] https://lore.kernel.org/linux-nvme/20250305132642.1271523-1-mlombard@redhat.com/

Thanks,
Yunje

On Tue, Mar 3, 2026 at 9:17 PM Maurizio Lombardi <mlombard at arkamax.eu> wrote:
>
> On Tue Mar 3, 2026 at 11:59 AM CET, Maurizio Lombardi wrote:
> >
> > In my opinion this is a valid concern.
> > I suspect that it's really possible to create a malicious sequence
> > that would make the target crash.
> >
> > I would change nvmet_tcp_build_pdu_iovec() to return an error
> > code and its caller so they propagate it up to nvmet_tcp_done_recv_pdu()
>
> Additionally, the current design leaves it somewhat ambiguous as to
> which function should be responsible for calling nvmet_tcp_fatal_error().
> In my opinion, this should be handled by the upper-layer function
> nvmet_tcp_try_recv(), allowing the lower-level functions to simply
> return an error code.
>
> I submitted a patch to clean this up exactly one year ago,
> but it has been forgotten and I only just remembered it.
>
> https://lore.kernel.org/linux-nvme/20250305132642.1271523-1-mlombard@redhat.com/
>
> I could prepare a patchset to fix all this stuff
>
> Maurizio