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

Allison Henderson
2026-04-07T07:55:15Z

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);
>
> [ ... ]