Re: [PATCH net v1 1/2] net/rds: Optimize rds_ib_laddr_check

[Allison Henderson <achender@kernel.org>]

On Mon, 2026-04-06 at 18:19 -0700, Jakub Kicinski wrote:
> 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: Optimize rds_ib_laddr_check
> 
> This patch optimizes rds_ib_laddr_check() to avoid excessive
> rdma_bind_addr() calls by checking if an address has been used before.
> It renames the original function to rds_ib_laddr_check_cm() and
> introduces a new wrapper that first checks rds_ib_get_device() before
> falling back to the CM-based check.
> 
> > diff --git a/net/rds/ib.c b/net/rds/ib.c
> > index ac6affa33ce7..73e01984ee9a 100644
> > --- a/net/rds/ib.c
> > +++ b/net/rds/ib.c
> > @@ -401,8 +401,8 @@ static void rds6_ib_ic_info(struct socket *sock, unsigned int len,
> >   * allowed to influence which paths have priority.  We could call userspace
> >   * asserting this policy "routing".
> >   */
> > -static int rds_ib_laddr_check(struct net *net, const struct in6_addr *addr,
> > -			      __u32 scope_id)
> > +static int rds_ib_laddr_check_cm(struct net *net, const struct in6_addr *addr,
> > +				 __u32 scope_id)
> >  {
> >  	int ret;
> >  	struct rdma_cm_id *cm_id;
> 
> [ ... ]
> 
> > @@ -487,6 +487,20 @@ static int rds_ib_laddr_check(struct net *net, const struct in6_addr *addr,
> >  	return ret;
> >  }
> > 
> > +static int rds_ib_laddr_check(struct net *net, const struct in6_addr *addr,
> > +			      __u32 scope_id)
> > +{
> > +	struct rds_ib_device *rds_ibdev = rds_ib_get_device(addr->s6_addr32[3]);
> > +
> > +	if (rds_ibdev) {
> > +		rds_ib_dev_put(rds_ibdev);
> > +
> > +		return 0;
> > +	}
> > +
> > +	return rds_ib_laddr_check_cm(net, addr, scope_id);
> > +}
> > +
> 
> Can this code handle pure IPv6 addresses correctly? The function extracts
> addr->s6_addr32[3] and passes it to rds_ib_get_device(), which looks up
> devices by IPv4 address.
> 
> For IPv4-mapped IPv6 addresses, addr->s6_addr32[3] contains the IPv4
> address. But for pure IPv6 addresses (when CONFIG_IPV6 is enabled),
> addr->s6_addr32[3] contains the last 32 bits of the IPv6 address, which
> could accidentally match a stored IPv4 address.
> 
> When this collision occurs, the function returns 0 (success) without
> calling rds_ib_laddr_check_cm(), which performs the full validation
> including rdma_bind_addr() and IPv6 link-local scope_id checks. This
> means a pure IPv6 address could be incorrectly accepted as valid for IB
> transport.
> 
> Should this be guarded with an ipv6_addr_v4mapped(addr) check? For
> example:
> 
>     if (ipv6_addr_v4mapped(addr)) {
>         rds_ibdev = rds_ib_get_device(addr->s6_addr32[3]);
>         ...
>     }
> 
> The existing callers of rds_ib_get_device (rds_ib_get_mr,
> rds_ib_update_ipaddr) operate in post-connection contexts where the
> address type is already validated, but rds_ib_laddr_check is the
> validation gate called during bind/transport-selection for any address
> type.

Ok  I think the correct ports to fix this would be these two commits:
https://github.com/oracle/linux-uek/commit/caffdcae6593	 
caffdcae6593 ("rds: Enable RDS IPv6 support")

https://github.com/oracle/linux-uek/commit/cb82f22bbff5  
cb82f22bbff5 ("rds: Changed IP address internal representation to struct in6_addr")

But that is starting to snowball quite a bit outside the scope of the syzbot bug, and a lot of rds port patches tend to
domino like this if we don't find a way to contain it.  So for the syzbot report, I think the ai suggestion is
reasonable.  So, if folks agree, I am fine to add that check in v2.

Thank you!
Allison

> 
> >  static void rds_ib_unregister_client(void)
> >  {
> >  	ib_unregister_client(&rds_ib_client);
> 
> [ ... ]