Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* [PATCH net v3 0/2] net/rds: scope-aware sendmsg connection cache
@ 2026-10-03 16:35 Allison Henderson
  2026-10-03 16:35 ` [PATCH net v3 1/2] net/rds: keep the connected peer's scope id apart from the bound one Allison Henderson
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Allison Henderson @ 2026-10-03 16:35 UTC (permalink / raw)
  To: netdev, linux-rdma, pabeni, edumazet, kuba, horms; +Cc: achender

Hi all,

This is v3 of the sendmsg connection-cache scope fix (v1 at [1], v2 at
[2]), now two patches.

  Patch 1 gives the connected peer's link-local scope its own field.
  rds_connect() used to store it in rs_bound_scope_id, the scope of
  the socket's own bound address, which a later bind() to a
  non-link-local address resets to 0 and which a later connect()
  pollutes for a globally bound socket.  A send without a destination
  now takes its scope from the connected peer's field, and rds_bind()'s
  connected-socket check compares against it.

  Patch 2 compares c_dev_if with the request's scope in the cache-hit
  test, so the cache uses the same key as rds_conn_lookup().

Changes since v2 [2]:
 - The v2 "scope 0 matches anything" wildcard is gone: it sent a
   connected socket's traffic out whichever interface the socket last
   used explicitly (review of v2).  Instead the destination-less send
   carries the connected peer's scope (new patch 1), and the comparison
   is exact.
 - Rebased onto current net.

Changes since v1 [1]:
 - A request without a scope kept matching the cached connection
   (superseded by the above).

[1] https://lore.kernel.org/netdev/20260921215046.174745-1-achender@kernel.org/
[2] https://lore.kernel.org/netdev/20260927062312.169097-1-achender@kernel.org/

Thank you,
Allison


Allison Henderson (2):
  net/rds: keep the connected peer's scope id apart from the bound one
  net/rds: include the scope id in the sendmsg connection cache check

 net/rds/af_rds.c | 12 ++++++++----
 net/rds/bind.c   |  4 ++--
 net/rds/rds.h    |  2 ++
 net/rds/send.c   |  5 +++--
 4 files changed, 15 insertions(+), 8 deletions(-)

-- 
2.25.1


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

* [PATCH net v3 1/2] net/rds: keep the connected peer's scope id apart from the bound one
  2026-10-03 16:35 [PATCH net v3 0/2] net/rds: scope-aware sendmsg connection cache Allison Henderson
@ 2026-10-03 16:35 ` Allison Henderson
  2026-10-04 16:35   ` sashiko-bot
  2026-10-04 16:41   ` netdev-bot+sashiko
  2026-10-03 16:35 ` [PATCH net v3 2/2] net/rds: include the scope id in the sendmsg connection cache check Allison Henderson
  2026-10-03 16:39 ` [PATCH net v3 0/2] net/rds: scope-aware sendmsg connection cache netdev-bot+sinfo
  2 siblings, 2 replies; 8+ messages in thread
From: Allison Henderson @ 2026-10-03 16:35 UTC (permalink / raw)
  To: netdev, linux-rdma, pabeni, edumazet, kuba, horms; +Cc: achender

rds_connect() has nowhere to keep the scope of a link-local peer, so
it stores it in rs_bound_scope_id, the scope of the socket's own
bound address, for rds_bind() to check a later link-local bind
against.  That field is then also what a send without a destination
uses as the request's scope, and what a later bind() overwrites:
rds_add_bound() stores the bound address's scope unconditionally,
which for a non-link-local address is 0.

So after connect(fe80::x%ifA) followed by bind(global), a send() with
no destination asks for fe80::x with scope 0.  rds_conn_lookup() keys
on the interface, so that finds or creates a connection with c_dev_if
0, which the TCP transport then tries to connect through
sin6_scope_id 0 and tcp_v6_connect() rejects for a link-local peer:
the connected socket's data is queued on a connection that can never
come up.  In the other order, bind(global) then connect(fe80::x%ifB),
the connect leaves a global-bound socket with rs_bound_scope_id ifB,
so sends to other link-local peers are refused as off-link and sends
to global peers inherit a meaningless interface.

Give the connected peer its own rs_conn_scope_id.  rds_connect()
records it there and leaves the bound scope alone, rds_bind()'s
connected-socket check compares against it, and the destination-less
send takes its scope from it.

Fixes: 1e2b44e78eea ("rds: Enable RDS IPv6 support")
Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Allison Henderson <achender@kernel.org>
---
 net/rds/af_rds.c | 12 ++++++++----
 net/rds/bind.c   |  4 ++--
 net/rds/rds.h    |  2 ++
 net/rds/send.c   |  2 +-
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
index d5defe9172e3..ba726782addf 100644
--- a/net/rds/af_rds.c
+++ b/net/rds/af_rds.c
@@ -572,6 +572,7 @@ static int rds_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
 		}
 		ipv6_addr_set_v4mapped(sin->sin_addr.s_addr, &rs->rs_conn_addr);
 		rs->rs_conn_port = sin->sin_port;
+		rs->rs_conn_scope_id = 0;
 		break;
 
 #if IS_ENABLED(CONFIG_IPV6)
@@ -616,11 +617,14 @@ static int rds_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
 				ret = -EINVAL;
 				break;
 			}
-			/* Remember the connected address scope ID.  It will
-			 * be checked against the binding local address when
-			 * the socket is bound.
+			/* Remember the connected address scope ID.  It is
+			 * checked against the binding local address when
+			 * the socket is bound, and gives a send without a
+			 * destination its scope.
 			 */
-			rs->rs_bound_scope_id = sin6->sin6_scope_id;
+			rs->rs_conn_scope_id = sin6->sin6_scope_id;
+		} else {
+			rs->rs_conn_scope_id = 0;
 		}
 		rs->rs_conn_addr = sin6->sin6_addr;
 		rs->rs_conn_port = sin6->sin6_port;
diff --git a/net/rds/bind.c b/net/rds/bind.c
index f800d920d969..3ac59cd512a2 100644
--- a/net/rds/bind.c
+++ b/net/rds/bind.c
@@ -233,8 +233,8 @@ int rds_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int addr_len)
 	 * non-link local address (scope_id is 0).
 	 */
 	if (!ipv6_addr_any(&rs->rs_conn_addr) && scope_id &&
-	    rs->rs_bound_scope_id &&
-	    scope_id != rs->rs_bound_scope_id) {
+	    rs->rs_conn_scope_id &&
+	    scope_id != rs->rs_conn_scope_id) {
 		ret = -EINVAL;
 		goto out;
 	}
diff --git a/net/rds/rds.h b/net/rds/rds.h
index 2db49573dacd..9b1ffc49c0a6 100644
--- a/net/rds/rds.h
+++ b/net/rds/rds.h
@@ -646,6 +646,8 @@ struct rds_sock {
 	struct in6_addr		rs_conn_addr;
 #define rs_conn_addr_v4		rs_conn_addr.s6_addr32[3]
 	__be16			rs_conn_port;
+	/* scope of rs_conn_addr when it is link-local, 0 otherwise */
+	__u32			rs_conn_scope_id;
 	struct rds_transport    *rs_transport;
 
 	/*
diff --git a/net/rds/send.c b/net/rds/send.c
index 1afa981e5c06..9380b67675bd 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -1256,7 +1256,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
 		lock_sock(sk);
 		daddr = rs->rs_conn_addr;
 		dport = rs->rs_conn_port;
-		scope_id = rs->rs_bound_scope_id;
+		scope_id = rs->rs_conn_scope_id;
 		release_sock(sk);
 	}
 
-- 
2.25.1


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

* [PATCH net v3 2/2] net/rds: include the scope id in the sendmsg connection cache check
  2026-10-03 16:35 [PATCH net v3 0/2] net/rds: scope-aware sendmsg connection cache Allison Henderson
  2026-10-03 16:35 ` [PATCH net v3 1/2] net/rds: keep the connected peer's scope id apart from the bound one Allison Henderson
@ 2026-10-03 16:35 ` Allison Henderson
  2026-10-04 16:35   ` sashiko-bot
  2026-10-04 16:41   ` netdev-bot+sashiko
  2026-10-03 16:39 ` [PATCH net v3 0/2] net/rds: scope-aware sendmsg connection cache netdev-bot+sinfo
  2 siblings, 2 replies; 8+ messages in thread
From: Allison Henderson @ 2026-10-03 16:35 UTC (permalink / raw)
  To: netdev, linux-rdma, pabeni, edumazet, kuba, horms; +Cc: achender

rds_sendmsg() reuses the connection cached in rs->rs_conn when its
peer address and ToS match the request.  The interface index is part
of a connection's identity as well: rds_conn_create_outgoing() passes
the request's scope_id down as dev_if, and rds_conn_lookup() compares
c_dev_if, so sends to the same link-local address through two
interfaces are two different connections.  The cache-hit test never
looked at it.

A socket bound to a non-link-local address has rs_bound_scope_id 0,
and the scope check at the top of rds_sendmsg() accepts any non-zero
destination scope for such a socket.  So after a send to fe80::x%ifA,
a send to fe80::x%ifB hits the cached ifA connection and the datagram
leaves through ifA, to whichever peer answers to that address there.

Compare c_dev_if with the request's scope_id in the cache test, so
that such a send takes the lookup path and finds, or creates, the ifB
connection - the same key the lookup uses.  With the previous patch a
send without a destination carries the connected peer's scope, so the
comparison is exact for it too.

Fixes: 1e2b44e78eea ("rds: Enable RDS IPv6 support")
Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Allison Henderson <achender@kernel.org>
---
 net/rds/send.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/rds/send.c b/net/rds/send.c
index 9380b67675bd..26a937a9df6a 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -1342,7 +1342,8 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
 	/* rds_conn_create has a spinlock that runs with IRQ off.
 	 * Caching the conn in the socket helps a lot. */
 	if (rs->rs_conn && ipv6_addr_equal(&rs->rs_conn->c_faddr, &daddr) &&
-	    rs->rs_tos == rs->rs_conn->c_tos) {
+	    rs->rs_tos == rs->rs_conn->c_tos &&
+	    rs->rs_conn->c_dev_if == scope_id) {
 		conn = rs->rs_conn;
 	} else {
 		conn = rds_conn_create_outgoing(sock_net(sock->sk),
-- 
2.25.1


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

* Re: [PATCH net v3 0/2] net/rds: scope-aware sendmsg connection cache
  2026-10-03 16:35 [PATCH net v3 0/2] net/rds: scope-aware sendmsg connection cache Allison Henderson
  2026-10-03 16:35 ` [PATCH net v3 1/2] net/rds: keep the connected peer's scope id apart from the bound one Allison Henderson
  2026-10-03 16:35 ` [PATCH net v3 2/2] net/rds: include the scope id in the sendmsg connection cache check Allison Henderson
@ 2026-10-03 16:39 ` netdev-bot+sinfo
  2 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sinfo @ 2026-10-03 16:39 UTC (permalink / raw)
  To: Allison Henderson; +Cc: netdev, linux-rdma, pabeni, edumazet, kuba, horms

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.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

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.

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

* Re: [PATCH net v3 1/2] net/rds: keep the connected peer's scope id apart from the bound one
  2026-10-03 16:35 ` [PATCH net v3 1/2] net/rds: keep the connected peer's scope id apart from the bound one Allison Henderson
@ 2026-10-04 16:35   ` sashiko-bot
  2026-10-04 16:41   ` netdev-bot+sashiko
  1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-10-04 16:35 UTC (permalink / raw)
  To: Allison Henderson; +Cc: linux-rdma

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rds: getname() and recvmsg() break IPv6 link-local scope ID
- [High] rds: destination-less sendmsg() fails for global dests if bound link-local
--

commit 898dc27ebfe8db635a3eb7a78c44763fbfcf0c43
Author: Allison Henderson <achender@kernel.org>

net/rds: keep the connected peer's scope id apart from the bound one

This commit isolates the connected peer's scope ID into a new
rs_conn_scope_id field, leaving the bound scope ID in
rs_bound_scope_id alone. It updates rds_connect(), rds_bind(), and
rds_sendmsg() to use the new field to ensure destination-less sends
and link-local binds operate with the correct interface scopes.

> diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
> index d5defe9172e36..ba726782addf7 100644
> --- a/net/rds/af_rds.c
> +++ b/net/rds/af_rds.c
[ ... ]
> @@ -616,11 +617,14 @@ static int rds_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
>  				ret = -EINVAL;
>  				break;
>  			}
> -			/* Remember the connected address scope ID.  It will
> -			 * be checked against the binding local address when
> -			 * the socket is bound.
> +			/* Remember the connected address scope ID.  It is
> +			 * checked against the binding local address when
> +			 * the socket is bound, and gives a send without a
> +			 * destination its scope.
>  			 */
> -			rs->rs_bound_scope_id = sin6->sin6_scope_id;
> +			rs->rs_conn_scope_id = sin6->sin6_scope_id;

[Severity: High]
Does this change miss updating rds_getname() and rds_recvmsg()?

By no longer storing the peer's scope ID in rs->rs_bound_scope_id,
userspace programs calling getpeername() or receiving messages via recvmsg()
may receive a scope ID of 0 for connected link-local peers when the
local socket is unbound or globally bound.

For example, rds_getname() appears to still populate the address using
the bound scope:

net/rds/af_rds.c:rds_getname() {
    ...
    sin6->sin6_scope_id = rs->rs_bound_scope_id;
    ...
}

Similarly in rds_recvmsg():

net/rds/recv.c:rds_recvmsg() {
    ...
    sin6->sin6_scope_id = rs->rs_bound_scope_id;
    ...
}

Could this prevent userspace from properly parsing the address or
sending replies to the correct interface?

> +		} else {
> +			rs->rs_conn_scope_id = 0;
>  		}
>  		rs->rs_conn_addr = sin6->sin6_addr;
>  		rs->rs_conn_port = sin6->sin6_port;
[ ... ]
> diff --git a/net/rds/send.c b/net/rds/send.c
> index 1afa981e5c06d..9380b67675bdc 100644
> --- a/net/rds/send.c
> +++ b/net/rds/send.c
> @@ -1256,7 +1256,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
>  		lock_sock(sk);
>  		daddr = rs->rs_conn_addr;
>  		dport = rs->rs_conn_port;
> -		scope_id = rs->rs_bound_scope_id;
> +		scope_id = rs->rs_conn_scope_id;
>  		release_sock(sk);
>  	}
>  

[Severity: High]
Does this cause destination-less sendmsg() calls to bypass the bound
scope fallback for global connections?

If an RDS socket is bound to an IPv6 link-local address (with a valid
scope) and connected to a global IPv6 address, a destination-less
sendmsg() will have namelen == 0.

It looks like it will assign scope_id from rs_conn_scope_id (which is
0 for global peers), but then skip the fallback logic entirely:

net/rds/send.c:rds_sendmsg() {
    ...
    } else {
        /* We only care about consistency with ->connect() */
        ...
        scope_id = rs->rs_conn_scope_id;
    }

    lock_sock(sk);
    if (ipv6_addr_any(&rs->rs_bound_addr) || ipv6_addr_any(&daddr)) {
        ...
    } else if (namelen != 0) {
        ...
        if (scope_id != rs->rs_bound_scope_id) {
            if (!scope_id) {
                scope_id = rs->rs_bound_scope_id;
    ...
}

Because the fallback logic is inside the else if (namelen != 0) block,
scope_id remains 0. The connection could then be created with a dev_if
of 0, causing the TCP transport to attempt a kernel_bind() to a
link-local source address with no scope ID, which the IPv6 stack will
reject.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003163526.250805-1-achender@kernel.org?part=1

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

* Re: [PATCH net v3 2/2] net/rds: include the scope id in the sendmsg connection cache check
  2026-10-03 16:35 ` [PATCH net v3 2/2] net/rds: include the scope id in the sendmsg connection cache check Allison Henderson
@ 2026-10-04 16:35   ` sashiko-bot
  2026-10-04 16:41   ` netdev-bot+sashiko
  1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-10-04 16:35 UTC (permalink / raw)
  To: Allison Henderson; +Cc: linux-rdma

> rds_sendmsg() reuses the connection cached in rs->rs_conn when its
> peer address and ToS match the request.  The interface index is part
> of a connection's identity as well: rds_conn_create_outgoing() passes
> the request's scope_id down as dev_if, and rds_conn_lookup() compares
> c_dev_if, so sends to the same link-local address through two
> interfaces are two different connections.  The cache-hit test never
> looked at it.
> 
> A socket bound to a non-link-local address has rs_bound_scope_id 0,
> and the scope check at the top of rds_sendmsg() accepts any non-zero
> destination scope for such a socket.  So after a send to fe80::x%ifA,
> a send to fe80::x%ifB hits the cached ifA connection and the datagram
> leaves through ifA, to whichever peer answers to that address there.
> 
> Compare c_dev_if with the request's scope_id in the cache test, so
> that such a send takes the lookup path and finds, or creates, the ifB
> connection - the same key the lookup uses.  With the previous patch a
> send without a destination carries the connected peer's scope, so the
> comparison is exact for it too.
> 
> Fixes: 1e2b44e78eea ("rds: Enable RDS IPv6 support")
> Assisted-by: Claude-Code:claude-fable-5
> Signed-off-by: Allison Henderson <achender@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003163526.250805-1-achender@kernel.org?part=2


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

* Re: [PATCH net v3 1/2] net/rds: keep the connected peer's scope id apart from the bound one
  2026-10-03 16:35 ` [PATCH net v3 1/2] net/rds: keep the connected peer's scope id apart from the bound one Allison Henderson
  2026-10-04 16:35   ` sashiko-bot
@ 2026-10-04 16:41   ` netdev-bot+sashiko
  1 sibling, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-10-04 16:41 UTC (permalink / raw)
  To: achender; +Cc: netdev, linux-rdma, pabeni, edumazet, kuba, horms

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

Critical: 0 · High: 1 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Regression in rds_sendmsg() (net/rds/send.c:1259) for
  destination-less sends.
- [Medium] The patch moves the connected peer's scope into the new
  rs_conn_scope_id but does not update rds_getname()'s peer branch…
- [Medium] rds_recvmsg() (net/rds/recv.c:788-793) builds the IPv6 sender
  name from inc->i_saddr and rs->rs_bound_scope_id, i.e. the socket's…

--

net/rds: keep the connected peer's scope id apart from the bound one

This patch adds a separate rs_conn_scope_id field to struct rds_sock.
With it, rds_connect() no longer stores a link-local peer's scope in
rs_bound_scope_id. The connected-socket check in rds_bind() and the
destination-less path in rds_sendmsg() now use the new field.

> diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
> index d5defe9172e36..ba726782addf7 100644
> --- a/net/rds/af_rds.c
> +++ b/net/rds/af_rds.c

[ ... ]

> @@ -616,11 +617,14 @@ static int rds_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
>  				ret = -EINVAL;
>  				break;
>  			}
> -			/* Remember the connected address scope ID.  It will
> -			 * be checked against the binding local address when
> -			 * the socket is bound.
> +			/* Remember the connected address scope ID.  It is
> +			 * checked against the binding local address when
> +			 * the socket is bound, and gives a send without a
> +			 * destination its scope.
>  			 */
> -			rs->rs_bound_scope_id = sin6->sin6_scope_id;
> +			rs->rs_conn_scope_id = sin6->sin6_scope_id;

[Severity: Medium]
The peer scope now lives only in rs_conn_scope_id. Does rds_getname()
still give getpeername() the right scope? Its peer branch still pairs
the connected address with the bound scope:

net/rds/af_rds.c:rds_getname() {
    ...
			sin6->sin6_addr = rs->rs_conn_addr;
			sin6->sin6_flowinfo = 0;
			/* scope_id is the same as in the bound address. */
			sin6->sin6_scope_id = rs->rs_bound_scope_id;
    ...
}

Before this patch, connect(fe80::x%ifA) on an unbound socket reported
fe80::x%ifA. With this patch it reports scope 0. bind(global) followed
by connect(fe80::x%ifB) also goes from ifB to 0. bind(fe80::a%ifA)
followed by connect(2001::1) reports ifA for a global peer.

rds_connect() and rds_sendmsg() both reject a link-local sockaddr_in6
with sin6_scope_id == 0. So the address getpeername() returns for a
link-local peer can't be passed back to connect() or sendto().

Should this read rs->rs_conn_scope_id instead? That would match the new
field's comment in rds.h ("scope of rs_conn_addr when it is link-local,
0 otherwise"). The later patch in this series doesn't change
rds_getname() either.

[Severity: Medium]
There is a similar question for rds_recvmsg(). It takes the sender's
scope from the socket's bound scope:

net/rds/recv.c:rds_recvmsg() {
    ...
				sin6->sin6_addr = inc->i_saddr;
				sin6->sin6_flowinfo = 0;
				sin6->sin6_scope_id = rs->rs_bound_scope_id;
    ...
}

Take bind(global) followed by connect(fe80::x%ifB). Before this patch,
rds_connect() left ifB in rs_bound_scope_id, so recvmsg() from the
connected peer reported fe80::x%ifB. Now rs_bound_scope_id stays 0.

The connection's laddr is global, so c_bound_if is 0. The message is
still delivered to the socket via:

rds_recv_incoming()->rds_find_bound(daddr, dport, conn->c_bound_if)

Does recvmsg() now report fe80::x with scope 0? rds_sendmsg() would
reject that address if it is used to reply. The same applies to the
connections that connect(fe80::x%ifA) followed by bind(global) can now
set up.

Using the bound scope as the sender's scope is older behavior. It also
affects unconnected global-bound sockets. The change in the connected
case comes from this patch, and the commit message doesn't mention it.

[ ... ]

> diff --git a/net/rds/send.c b/net/rds/send.c
> index 1afa981e5c06d..9380b67675bdc 100644
> --- a/net/rds/send.c
> +++ b/net/rds/send.c
> @@ -1256,7 +1256,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
>  		lock_sock(sk);
>  		daddr = rs->rs_conn_addr;
>  		dport = rs->rs_conn_port;
> -		scope_id = rs->rs_bound_scope_id;
> +		scope_id = rs->rs_conn_scope_id;

[Severity: High]
Can this break destination-less sends on a socket that is bound to a
link-local address and connected to a global peer? For example,
bind(fe80::y%ifA) and connect(2001:db8::1), in either order.

rds_connect() now sets rs_conn_scope_id to 0 for the global peer. So a
send() with msg_namelen == 0 gets scope_id 0, where it used to get ifA
from rs_bound_scope_id.

Later in rds_sendmsg(), the fallback to the bound scope only runs when
namelen != 0:

	} else if (namelen != 0) {
		...
		if (scope_id != rs->rs_bound_scope_id) {
			if (!scope_id) {
				scope_id = rs->rs_bound_scope_id;
		...

So rds_conn_create_outgoing() is called with laddr fe80::y and dev_if
0, which sets both c_dev_if and c_bound_if to 0.
rds_tcp_conn_path_connect() then binds with:

		sin6.sin6_addr = conn->c_laddr;
		...
		sin6.sin6_scope_id = conn->c_dev_if;

__inet6_bind() rejects a link-local bind without an interface with
-EINVAL.

Wouldn't every reconnect attempt fail, leaving the connected socket's
data queued on a connection that never comes up?

sendto(2001:db8::1) on the same socket still works because it inherits
rs_bound_scope_id. That means the two send paths now pick different
connections for the same peer.

The later patch "net/rds: include the scope id in the sendmsg
connection cache check" only changes the rs_conn cache check. It
doesn't seem to address this.

Would something like this work instead?

		scope_id = rs->rs_conn_scope_id ?: rs->rs_bound_scope_id;

>  		release_sock(sk);
>  	}
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261003163526.250805-1-achender%40kernel.org

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

* Re: [PATCH net v3 2/2] net/rds: include the scope id in the sendmsg connection cache check
  2026-10-03 16:35 ` [PATCH net v3 2/2] net/rds: include the scope id in the sendmsg connection cache check Allison Henderson
  2026-10-04 16:35   ` sashiko-bot
@ 2026-10-04 16:41   ` netdev-bot+sashiko
  1 sibling, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-10-04 16:41 UTC (permalink / raw)
  To: achender; +Cc: netdev, linux-rdma, pabeni, edumazet, kuba, horms

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

Critical: 0 · High: 1 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] In rds_sendmsg() (net/rds/send.c), send() and sendto() to the
  same peer now build different connection keys.
- [Medium] In rds_sendmsg() (net/rds/send.c:1344-1347), the cache test
  reads rs->rs_conn with plain loads and no lock: the NULL check, the…

--

net/rds: include the scope id in the sendmsg connection cache check

This patch adds a comparison of rs->rs_conn->c_dev_if against the
request's scope_id to the connection cache test in rds_sendmsg(). The aim
is that sends to the same link-local address through different interfaces
go through the lookup path and use the matching connection, not the cached
one.

> diff --git a/net/rds/send.c b/net/rds/send.c
> index 9380b67675bdc..26a937a9df6af 100644
> --- a/net/rds/send.c
> +++ b/net/rds/send.c
> @@ -1342,7 +1342,8 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
>  	/* rds_conn_create has a spinlock that runs with IRQ off.
>  	 * Caching the conn in the socket helps a lot. */
>  	if (rs->rs_conn && ipv6_addr_equal(&rs->rs_conn->c_faddr, &daddr) &&
> -	    rs->rs_tos == rs->rs_conn->c_tos) {
> +	    rs->rs_tos == rs->rs_conn->c_tos &&
> +	    rs->rs_conn->c_dev_if == scope_id) {

[Severity: High]
On a socket bound to a link-local address and connected to a
non-link-local peer, can send() and sendto() to the same peer now end up
with different connection keys?

Take bind(fe80::a%X) followed by connect(G), where G is a global address.
rds_bind() stores rs_bound_scope_id = X, and rds_connect() takes the
non-link-local branch:

net/rds/af_rds.c:rds_connect() {
    ...
		} else {
			rs->rs_conn_scope_id = 0;
		}
    ...
}

A sendto(G) starts with scope_id 0. The namelen != 0 block in
rds_sendmsg() then falls back to the bound scope:

		if (scope_id != rs->rs_bound_scope_id) {
			if (!scope_id) {
				scope_id = rs->rs_bound_scope_id;

So sendto(G) creates and caches a connection with c_dev_if = X.

A send() with no destination takes scope_id = rs->rs_conn_scope_id, which
is 0. It never gets that fallback, because the fallback sits inside the
else if (namelen != 0) branch.

With the new c_dev_if == scope_id test, X != 0, so the cache misses.
rds_conn_create_outgoing(..., 0) then returns a connection with a
link-local c_laddr and c_dev_if = 0, and c_bound_if = 0 from
__rds_conn_create(). rs->rs_conn is overwritten with that connection.

For TCP, rds_tcp_conn_path_connect() binds the kernel socket with:

		sin6.sin6_addr = conn->c_laddr;
		...
		sin6.sin6_scope_id = conn->c_dev_if;

That socket has no sk_bound_dev_if, so __inet6_bind() rejects the
link-local bind:

			/* Binding to link-local address requires an interface */
			if (!sk->sk_bound_dev_if) {
				err = -EINVAL;

Wouldn't that connection never come up, so that data queued by send()
stalls? An application that mixes sendto(G) and send() would also switch
rs_conn between the two connections on every call.

Before this series, send() used rs_bound_scope_id, which matches what
sendto() computes. The earlier patch "net/rds: keep the connected peer's
scope id apart from the bound one" switched send() to rs_conn_scope_id.
This patch then removes the faddr/tos-only cache hit that was hiding the
mismatch.

The commit message says:

  With the previous patch a send without a destination carries the
  connected peer's scope, so the comparison is exact for it too.

Does that hold when the bound address is link-local and the peer is not?

Would it help to apply the rs_bound_scope_id fallback to the namelen == 0
path too, or to store the effective scope in rds_connect(), so both paths
use the same key?

>  		conn = rs->rs_conn;

[Severity: Medium]
Can rs->rs_conn change between the checks above and this assignment?

By this point lock_sock() has been released, and every access to
rs->rs_conn in this test is a plain load. Other senders on the same
socket store to it without a lock in the else branch:

		rs->rs_conn = conn;

That seems to allow a sequence like this:

T1: sendto(fe80::x%ifB) passes the checks against the cached connB
T2: sendto(fe80::x%ifA) misses the cache and stores rs->rs_conn = connA
T1: reloads rs->rs_conn and queues the ifB message on connA

The result would be the same wrong-interface send this patch is trying to
close, reached through a race. It requires the compiler to reload
rs->rs_conn, which C allows for plain accesses.

The unsnapshotted read was already there before this patch. With this
change, though, a socket that alternates interfaces rewrites rs_conn on
every send.

Would reading rs->rs_conn once with READ_ONCE(), running all the
comparisons on that snapshot, and publishing with WRITE_ONCE() close this?

>  	} else {
>  		conn = rds_conn_create_outgoing(sock_net(sock->sk),

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261003163526.250805-1-achender%40kernel.org

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

end of thread, other threads:[~2026-10-04 16:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-03 16:35 [PATCH net v3 0/2] net/rds: scope-aware sendmsg connection cache Allison Henderson
2026-10-03 16:35 ` [PATCH net v3 1/2] net/rds: keep the connected peer's scope id apart from the bound one Allison Henderson
2026-10-04 16:35   ` sashiko-bot
2026-10-04 16:41   ` netdev-bot+sashiko
2026-10-03 16:35 ` [PATCH net v3 2/2] net/rds: include the scope id in the sendmsg connection cache check Allison Henderson
2026-10-04 16:35   ` sashiko-bot
2026-10-04 16:41   ` netdev-bot+sashiko
2026-10-03 16:39 ` [PATCH net v3 0/2] net/rds: scope-aware sendmsg connection cache netdev-bot+sinfo

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