public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rds: zero per-item info buffer before handing it to visitors
@ 2026-04-17 14:19 Michael Bommarito
  2026-04-17 16:53 ` Sharath Srinivasan
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Michael Bommarito @ 2026-04-17 14:19 UTC (permalink / raw)
  To: Allison Henderson, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Simon Horman, netdev, linux-rdma, rds-devel, linux-kernel,
	Michael Bommarito

Yet another from my "clanker."  This only applies to people who
don't use CONFIG_INIT_STACK_ALL_ZERO, but I presume that's
still enough people that it's worth backporting since it can
be chained through leaked addresses to defeat KASLR.

rds_for_each_conn_info() and rds_walk_conn_path_info() both hand a
caller-allocated on-stack u64 buffer to a per-connection visitor and
then copy the full item_len bytes back to user space via
rds_info_copy() regardless of how much of the buffer the visitor
actually wrote.

rds_ib_conn_info_visitor() and rds6_ib_conn_info_visitor() only
write a subset of their output struct when the underlying
rds_connection is not in state RDS_CONN_UP (src/dst addr, tos, sl
and the two GIDs via explicit memsets).  Several u32 fields
(max_send_wr, max_recv_wr, max_send_sge, rdma_mr_max, rdma_mr_size,
cache_allocs) and the 2-byte alignment hole between sl and
cache_allocs remain as whatever stack contents preceded the visitor
call and are then memcpy_to_user()'d out to user space.

struct rds_info_rdma_connection and struct rds6_info_rdma_connection
are the only rds_info_* structs in include/uapi/linux/rds.h that are
not marked __attribute__((packed)), so they have a real alignment
hole.  The other info visitors (rds_conn_info_visitor,
rds6_conn_info_visitor, rds_tcp_tc_info, ...) write all fields of
their packed output struct today and are not known to be vulnerable,
but a future visitor that adds a conditional write-path would have
the same bug.

Reproduction on a kernel built without CONFIG_INIT_STACK_ALL_ZERO=y:
a local unprivileged user opens AF_RDS, sets SO_RDS_TRANSPORT=IB,
binds to a local address on an RDMA-capable netdev (rxe soft-RoCE on
any netdev is sufficient), sendto()'s any peer on the same subnet
(fails cleanly but installs an rds_connection in the global hash in
RDS_CONN_CONNECTING), then calls getsockopt(SOL_RDS,
RDS_INFO_IB_CONNECTIONS).  The returned 68-byte item contains 26
bytes of stack garbage including kernel text/data pointers:

    0..7   0a 63 00 01 0a 63 00 02     src=10.99.0.1 dst=10.99.0.2
    8..39  00 ...                      gids (memset-zeroed)
    40..47 e0 92 a3 81 ff ff ff ff     kernel pointer (max_send_wr)
    48..55 7f 37 b5 81 ff ff ff ff     kernel pointer (rdma_mr_max)
    56..59 01 00 08 00                 rdma_mr_size (garbage)
    60..61 00 00                       tos, sl
    62..63 00 00                       alignment padding
    64..67 18 00 00 00                 cache_allocs (garbage)

Fix by zeroing the per-item buffer in both rds_for_each_conn_info()
and rds_walk_conn_path_info() before invoking the visitor.  This
covers the IPv4/IPv6 IB visitors and hardens all current and future
visitors against the same class of bug.

No functional change for visitors that fully populate their output.

Fixes: ec16227e1414 ("RDS/IB: Infiniband transport")
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
 net/rds/connection.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index 412441aaa298..c10b7ed06c49 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -701,6 +701,13 @@ void rds_for_each_conn_info(struct socket *sock, unsigned int len,
 	     i++, head++) {
 		hlist_for_each_entry_rcu(conn, head, c_hash_node) {
 
+			/* Zero the per-item buffer before handing it to the
+			 * visitor so any field the visitor does not write -
+			 * including implicit alignment padding - cannot leak
+			 * stack contents to user space via rds_info_copy().
+			 */
+			memset(buffer, 0, item_len);
+
 			/* XXX no c_lock usage.. */
 			if (!visitor(conn, buffer))
 				continue;
@@ -750,6 +757,13 @@ static void rds_walk_conn_path_info(struct socket *sock, unsigned int len,
 			 */
 			cp = conn->c_path;
 
+			/* Zero the per-item buffer for the same reason as
+			 * rds_for_each_conn_info(): any byte the visitor
+			 * does not write (including alignment padding) must
+			 * not leak stack contents via rds_info_copy().
+			 */
+			memset(buffer, 0, item_len);
+
 			/* XXX no cp_lock usage.. */
 			if (!visitor(cp, buffer))
 				continue;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] rds: zero per-item info buffer before handing it to visitors
  2026-04-17 14:19 [PATCH] rds: zero per-item info buffer before handing it to visitors Michael Bommarito
@ 2026-04-17 16:53 ` Sharath Srinivasan
  2026-04-17 20:07 ` Allison Henderson
  2026-04-18 14:10 ` [PATCH net v2] net/rds: " Michael Bommarito
  2 siblings, 0 replies; 4+ messages in thread
From: Sharath Srinivasan @ 2026-04-17 16:53 UTC (permalink / raw)
  To: Michael Bommarito, Allison Henderson, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, netdev, linux-rdma, rds-devel, linux-kernel


On 2026-04-17 7:19 a.m., Michael Bommarito wrote:
> Yet another from my "clanker."  This only applies to people who
> don't use CONFIG_INIT_STACK_ALL_ZERO, but I presume that's
> still enough people that it's worth backporting since it can
> be chained through leaked addresses to defeat KASLR.
> 
> rds_for_each_conn_info() and rds_walk_conn_path_info() both hand a
> caller-allocated on-stack u64 buffer to a per-connection visitor and
> then copy the full item_len bytes back to user space via
> rds_info_copy() regardless of how much of the buffer the visitor
> actually wrote.
> 
> rds_ib_conn_info_visitor() and rds6_ib_conn_info_visitor() only
> write a subset of their output struct when the underlying
> rds_connection is not in state RDS_CONN_UP (src/dst addr, tos, sl
> and the two GIDs via explicit memsets).  Several u32 fields
> (max_send_wr, max_recv_wr, max_send_sge, rdma_mr_max, rdma_mr_size,
> cache_allocs) and the 2-byte alignment hole between sl and
> cache_allocs remain as whatever stack contents preceded the visitor
> call and are then memcpy_to_user()'d out to user space.
> 
> struct rds_info_rdma_connection and struct rds6_info_rdma_connection
> are the only rds_info_* structs in include/uapi/linux/rds.h that are
> not marked __attribute__((packed)), so they have a real alignment
> hole.  The other info visitors (rds_conn_info_visitor,
> rds6_conn_info_visitor, rds_tcp_tc_info, ...) write all fields of
> their packed output struct today and are not known to be vulnerable,
> but a future visitor that adds a conditional write-path would have
> the same bug.
> 
> Reproduction on a kernel built without CONFIG_INIT_STACK_ALL_ZERO=y:
> a local unprivileged user opens AF_RDS, sets SO_RDS_TRANSPORT=IB,
> binds to a local address on an RDMA-capable netdev (rxe soft-RoCE on
> any netdev is sufficient), sendto()'s any peer on the same subnet
> (fails cleanly but installs an rds_connection in the global hash in
> RDS_CONN_CONNECTING), then calls getsockopt(SOL_RDS,
> RDS_INFO_IB_CONNECTIONS).  The returned 68-byte item contains 26
> bytes of stack garbage including kernel text/data pointers:
> 
>     0..7   0a 63 00 01 0a 63 00 02     src=10.99.0.1 dst=10.99.0.2
>     8..39  00 ...                      gids (memset-zeroed)
>     40..47 e0 92 a3 81 ff ff ff ff     kernel pointer (max_send_wr)
>     48..55 7f 37 b5 81 ff ff ff ff     kernel pointer (rdma_mr_max)
>     56..59 01 00 08 00                 rdma_mr_size (garbage)
>     60..61 00 00                       tos, sl
>     62..63 00 00                       alignment padding
>     64..67 18 00 00 00                 cache_allocs (garbage)
> 
> Fix by zeroing the per-item buffer in both rds_for_each_conn_info()
> and rds_walk_conn_path_info() before invoking the visitor.  This
> covers the IPv4/IPv6 IB visitors and hardens all current and future
> visitors against the same class of bug.
> 
> No functional change for visitors that fully populate their output.
> 
> Fixes: ec16227e1414 ("RDS/IB: Infiniband transport")

LGTM. Reviewed-by: Sharath Srinivasan <sharath.srinivasan@oracle.com>

Thanks,
Sharath

> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> Assisted-by: Claude:claude-opus-4-7
> ---
>  net/rds/connection.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index 412441aaa298..c10b7ed06c49 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -701,6 +701,13 @@ void rds_for_each_conn_info(struct socket *sock, unsigned int len,
>  	     i++, head++) {
>  		hlist_for_each_entry_rcu(conn, head, c_hash_node) {
>  
> +			/* Zero the per-item buffer before handing it to the
> +			 * visitor so any field the visitor does not write -
> +			 * including implicit alignment padding - cannot leak
> +			 * stack contents to user space via rds_info_copy().
> +			 */
> +			memset(buffer, 0, item_len);
> +
>  			/* XXX no c_lock usage.. */
>  			if (!visitor(conn, buffer))
>  				continue;
> @@ -750,6 +757,13 @@ static void rds_walk_conn_path_info(struct socket *sock, unsigned int len,
>  			 */
>  			cp = conn->c_path;
>  
> +			/* Zero the per-item buffer for the same reason as
> +			 * rds_for_each_conn_info(): any byte the visitor
> +			 * does not write (including alignment padding) must
> +			 * not leak stack contents via rds_info_copy().
> +			 */
> +			memset(buffer, 0, item_len);
> +
>  			/* XXX no cp_lock usage.. */
>  			if (!visitor(cp, buffer))
>  				continue;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] rds: zero per-item info buffer before handing it to visitors
  2026-04-17 14:19 [PATCH] rds: zero per-item info buffer before handing it to visitors Michael Bommarito
  2026-04-17 16:53 ` Sharath Srinivasan
@ 2026-04-17 20:07 ` Allison Henderson
  2026-04-18 14:10 ` [PATCH net v2] net/rds: " Michael Bommarito
  2 siblings, 0 replies; 4+ messages in thread
From: Allison Henderson @ 2026-04-17 20:07 UTC (permalink / raw)
  To: Michael Bommarito, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Simon Horman, netdev, linux-rdma, rds-devel, linux-kernel

On Fri, 2026-04-17 at 10:19 -0400, Michael Bommarito wrote:
> Yet another from my "clanker."  This only applies to people who
> don't use CONFIG_INIT_STACK_ALL_ZERO, but I presume that's
> still enough people that it's worth backporting since it can
> be chained through leaked addresses to defeat KASLR.
> 
> rds_for_each_conn_info() and rds_walk_conn_path_info() both hand a
> caller-allocated on-stack u64 buffer to a per-connection visitor and
> then copy the full item_len bytes back to user space via
> rds_info_copy() regardless of how much of the buffer the visitor
> actually wrote.
> 
> rds_ib_conn_info_visitor() and rds6_ib_conn_info_visitor() only
> write a subset of their output struct when the underlying
> rds_connection is not in state RDS_CONN_UP (src/dst addr, tos, sl
> and the two GIDs via explicit memsets).  Several u32 fields
> (max_send_wr, max_recv_wr, max_send_sge, rdma_mr_max, rdma_mr_size,
> cache_allocs) and the 2-byte alignment hole between sl and
> cache_allocs remain as whatever stack contents preceded the visitor
> call and are then memcpy_to_user()'d out to user space.
> 
> struct rds_info_rdma_connection and struct rds6_info_rdma_connection
> are the only rds_info_* structs in include/uapi/linux/rds.h that are
> not marked __attribute__((packed)), so they have a real alignment
> hole.  The other info visitors (rds_conn_info_visitor,
> rds6_conn_info_visitor, rds_tcp_tc_info, ...) write all fields of
> their packed output struct today and are not known to be vulnerable,
> but a future visitor that adds a conditional write-path would have
> the same bug.
> 
> Reproduction on a kernel built without CONFIG_INIT_STACK_ALL_ZERO=y:
> a local unprivileged user opens AF_RDS, sets SO_RDS_TRANSPORT=IB,
> binds to a local address on an RDMA-capable netdev (rxe soft-RoCE on
> any netdev is sufficient), sendto()'s any peer on the same subnet
> (fails cleanly but installs an rds_connection in the global hash in
> RDS_CONN_CONNECTING), then calls getsockopt(SOL_RDS,
> RDS_INFO_IB_CONNECTIONS).  The returned 68-byte item contains 26
> bytes of stack garbage including kernel text/data pointers:
> 
>     0..7   0a 63 00 01 0a 63 00 02     src=10.99.0.1 dst=10.99.0.2
>     8..39  00 ...                      gids (memset-zeroed)
>     40..47 e0 92 a3 81 ff ff ff ff     kernel pointer (max_send_wr)
>     48..55 7f 37 b5 81 ff ff ff ff     kernel pointer (rdma_mr_max)
>     56..59 01 00 08 00                 rdma_mr_size (garbage)
>     60..61 00 00                       tos, sl
>     62..63 00 00                       alignment padding
>     64..67 18 00 00 00                 cache_allocs (garbage)
> 
> Fix by zeroing the per-item buffer in both rds_for_each_conn_info()
> and rds_walk_conn_path_info() before invoking the visitor.  This
> covers the IPv4/IPv6 IB visitors and hardens all current and future
> visitors against the same class of bug.
> 
> No functional change for visitors that fully populate their output.
> 
> Fixes: ec16227e1414 ("RDS/IB: Infiniband transport")
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> Assisted-by: Claude:claude-opus-4-7

Hi Micheal,

The change looks fine to me.  Since this is a bug fix, you'll want to cc stable
tree stable@vger.kernel.org, and note the target tree and component in the
subject line like this:  

[PATCH net v2] net/rds: zero per-item info buffer before handing it to visitors

Other than that, the patch looks good to me.  Thanks Micheal.

Reviewed-by: Allison Henderson <achender@kernel.org>

Allison

> ---
>  net/rds/connection.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index 412441aaa298..c10b7ed06c49 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -701,6 +701,13 @@ void rds_for_each_conn_info(struct socket *sock, unsigned int len,
>  	     i++, head++) {
>  		hlist_for_each_entry_rcu(conn, head, c_hash_node) {
>  
> +			/* Zero the per-item buffer before handing it to the
> +			 * visitor so any field the visitor does not write -
> +			 * including implicit alignment padding - cannot leak
> +			 * stack contents to user space via rds_info_copy().
> +			 */
> +			memset(buffer, 0, item_len);
> +
>  			/* XXX no c_lock usage.. */
>  			if (!visitor(conn, buffer))
>  				continue;
> @@ -750,6 +757,13 @@ static void rds_walk_conn_path_info(struct socket *sock, unsigned int len,
>  			 */
>  			cp = conn->c_path;
>  
> +			/* Zero the per-item buffer for the same reason as
> +			 * rds_for_each_conn_info(): any byte the visitor
> +			 * does not write (including alignment padding) must
> +			 * not leak stack contents via rds_info_copy().
> +			 */
> +			memset(buffer, 0, item_len);
> +
>  			/* XXX no cp_lock usage.. */
>  			if (!visitor(cp, buffer))
>  				continue;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net v2] net/rds: zero per-item info buffer before handing it to visitors
  2026-04-17 14:19 [PATCH] rds: zero per-item info buffer before handing it to visitors Michael Bommarito
  2026-04-17 16:53 ` Sharath Srinivasan
  2026-04-17 20:07 ` Allison Henderson
@ 2026-04-18 14:10 ` Michael Bommarito
  2 siblings, 0 replies; 4+ messages in thread
From: Michael Bommarito @ 2026-04-18 14:10 UTC (permalink / raw)
  To: Allison Henderson, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Sharath Srinivasan, Simon Horman, netdev, linux-rdma, rds-devel,
	linux-kernel, stable

rds_for_each_conn_info() and rds_walk_conn_path_info() both hand a
caller-allocated on-stack u64 buffer to a per-connection visitor and
then copy the full item_len bytes back to user space via
rds_info_copy() regardless of how much of the buffer the visitor
actually wrote.

rds_ib_conn_info_visitor() and rds6_ib_conn_info_visitor() only
write a subset of their output struct when the underlying
rds_connection is not in state RDS_CONN_UP (src/dst addr, tos, sl
and the two GIDs via explicit memsets). Several u32 fields
(max_send_wr, max_recv_wr, max_send_sge, rdma_mr_max, rdma_mr_size,
cache_allocs) and the 2-byte alignment hole between sl and
cache_allocs remain as whatever stack contents preceded the visitor
call and are then memcpy_to_user()'d out to user space.

struct rds_info_rdma_connection and struct rds6_info_rdma_connection
are the only rds_info_* structs in include/uapi/linux/rds.h that are
not marked __attribute__((packed)), so they have a real alignment
hole. The other info visitors (rds_conn_info_visitor,
rds6_conn_info_visitor, rds_tcp_tc_info, ...) write all fields of
their packed output struct today and are not known to be vulnerable,
but a future visitor that adds a conditional write-path would have
the same bug.

Reproduction on a kernel built without CONFIG_INIT_STACK_ALL_ZERO=y:
a local unprivileged user opens AF_RDS, sets SO_RDS_TRANSPORT=IB,
binds to a local address on an RDMA-capable netdev (rxe soft-RoCE on
any netdev is sufficient), sendto()'s any peer on the same subnet
(fails cleanly but installs an rds_connection in the global hash in
RDS_CONN_CONNECTING), then calls getsockopt(SOL_RDS,
RDS_INFO_IB_CONNECTIONS). The returned 68-byte item contains 26
bytes of stack garbage including kernel text/data pointers:

    0..7   0a 63 00 01 0a 63 00 02     src=10.99.0.1 dst=10.99.0.2
    8..39  00 ...                      gids (memset-zeroed)
    40..47 e0 92 a3 81 ff ff ff ff     kernel pointer (max_send_wr)
    48..55 7f 37 b5 81 ff ff ff ff     kernel pointer (rdma_mr_max)
    56..59 01 00 08 00                 rdma_mr_size (garbage)
    60..61 00 00                       tos, sl
    62..63 00 00                       alignment padding
    64..67 18 00 00 00                 cache_allocs (garbage)

Fix by zeroing the per-item buffer in both rds_for_each_conn_info()
and rds_walk_conn_path_info() before invoking the visitor. This
covers the IPv4/IPv6 IB visitors and hardens all current and future
visitors against the same class of bug.

No functional change for visitors that fully populate their output.

Changes in v2:
- retarget at the net tree (subject prefix "[PATCH net v2]",
  net/rds: prefix in the title)
- add Cc: stable@vger.kernel.org
- pick up Reviewed-by tags from Sharath Srinivasan and
  Allison Henderson

Fixes: ec16227e1414 ("RDS/IB: Infiniband transport")
Cc: stable@vger.kernel.org
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Reviewed-by: Sharath Srinivasan <sharath.srinivasan@oracle.com>
Reviewed-by: Allison Henderson <achender@kernel.org>
Assisted-by: Claude:claude-opus-4-7
---
 net/rds/connection.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index 412441aaa298..c10b7ed06c49 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -701,6 +701,13 @@ void rds_for_each_conn_info(struct socket *sock, unsigned int len,
 	     i++, head++) {
 		hlist_for_each_entry_rcu(conn, head, c_hash_node) {
 
+			/* Zero the per-item buffer before handing it to the
+			 * visitor so any field the visitor does not write -
+			 * including implicit alignment padding - cannot leak
+			 * stack contents to user space via rds_info_copy().
+			 */
+			memset(buffer, 0, item_len);
+
 			/* XXX no c_lock usage.. */
 			if (!visitor(conn, buffer))
 				continue;
@@ -750,6 +757,13 @@ static void rds_walk_conn_path_info(struct socket *sock, unsigned int len,
 			 */
 			cp = conn->c_path;
 
+			/* Zero the per-item buffer for the same reason as
+			 * rds_for_each_conn_info(): any byte the visitor
+			 * does not write (including alignment padding) must
+			 * not leak stack contents via rds_info_copy().
+			 */
+			memset(buffer, 0, item_len);
+
 			/* XXX no cp_lock usage.. */
 			if (!visitor(cp, buffer))
 				continue;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-18 14:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17 14:19 [PATCH] rds: zero per-item info buffer before handing it to visitors Michael Bommarito
2026-04-17 16:53 ` Sharath Srinivasan
2026-04-17 20:07 ` Allison Henderson
2026-04-18 14:10 ` [PATCH net v2] net/rds: " Michael Bommarito

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox