[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260126174620.1393182-1-horms@kernel.org>
Date: Mon, 26 Jan 2026 17:46:20 +0000
From: Simon Horman <horms@...nel.org>
To: achender@...nel.org
Cc: Simon Horman <horms@...nel.org>,
pabeni@...hat.com,
rds-devel@....oracle.com,
netdev@...r.kernel.org,
allison.henderson@...cle.com,
edumazet@...gle.com,
linux-kselftest@...r.kernel.org,
kuba@...nel.org,
linux-rdma@...r.kernel.org
Subject: Re: [net-next,v1,1/7] net/rds: new extension header: rdma bytes
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net/rds: new extension header: rdma bytes
This patch introduces a new extension header type RDS_EXTHDR_RDMA_BYTES
to allow RDMA initiators to exchange byte counts with their targets for
statistics and policy purposes. It also expands rds_message_add_extension()
to support multiple extensions per message.
> diff --git a/net/rds/rds.h b/net/rds/rds.h
> index 8a549fe687ac..cadfd7ec0ba9 100644
> --- a/net/rds/rds.h
> +++ b/net/rds/rds.h
[ ... ]
> @@ -258,6 +259,19 @@ struct rds_ext_header_rdma_dest {
> __be32 h_rdma_offset;
> };
>
> +/*
> + * This extension header tells the peer about delivered RDMA byte count.
> + */
> +#define RDS_EXTHDR_RDMA_BYTES 4
> +
> +struct rds_ext_header_rdma_bytes {
> + __be32 h_rdma_bytes; /* byte count */
> + u8 h_rflags; /* direction of RDMA, write or read */
> +};
This struct has 5 bytes of actual data but sizeof() will return 8 bytes
due to alignment padding. The 3 padding bytes after h_rflags are not
initialized when this struct is used in rds_ib_xmit(). Since
rds_message_add_extension() copies sizeof(struct) bytes via memcpy(),
those uninitialized padding bytes get transmitted to the peer.
Should this struct be marked __packed, or should the declaration in
rds_ib_xmit() use a zeroing initializer like = {} to avoid leaking
kernel stack contents over the network?
> diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c
> index f9d28ddd168d..8282ff61b0b3 100644
> --- a/net/rds/ib_send.c
> +++ b/net/rds/ib_send.c
> @@ -578,10 +578,27 @@ int rds_ib_xmit(struct rds_connection *conn, struct rds_message *rm,
> * used by the peer to release use-once RDMA MRs. */
> if (rm->rdma.op_active) {
> struct rds_ext_header_rdma ext_hdr;
> + struct rds_ext_header_rdma_bytes rdma_bytes_ext_hdr;
>
> ext_hdr.h_rdma_rkey = cpu_to_be32(rm->rdma.op_rkey);
> rds_message_add_extension(&rm->m_inc.i_hdr,
> - RDS_EXTHDR_RDMA, &ext_hdr, sizeof(ext_hdr));
> + RDS_EXTHDR_RDMA, &ext_hdr);
> +
> + /* prepare the rdma bytes ext header */
> + rdma_bytes_ext_hdr.h_rflags = rm->rdma.op_write ?
> + RDS_FLAG_RDMA_WR_BYTES : RDS_FLAG_RDMA_RD_BYTES;
> + rdma_bytes_ext_hdr.h_rdma_bytes =
> + cpu_to_be32(rm->rdma.op_bytes);
Here rdma_bytes_ext_hdr is declared on the stack without initialization.
Only h_rflags and h_rdma_bytes are assigned, leaving the 3 padding bytes
uninitialized. When rds_message_add_extension() calls memcpy() with
len = sizeof(struct rds_ext_header_rdma_bytes) = 8, those uninitialized
bytes are copied into the header and sent to the peer.
[ ... ]
--
pw-bot: cr
Powered by blists - more mailing lists