Re: [PATCH net v1 2/2] net/rds: Restrict use of RDS/IB to the initial network namespace

[Jakub Kicinski <kuba@kernel.org>]

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: Restrict use of RDS/IB to the initial network namespace

Prevent using RDS/IB in network namespaces other than the initial one.
The existing RDS/IB code will not work properly in non-initial network
namespaces, and there are currently no plans to support such use.

Closes: https://syzkaller.appspot.com/bug?extid=da8e060735ae02c8f3d1

> diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
> --- a/net/rds/af_rds.c
> +++ b/net/rds/af_rds.c

[ ... ]

> @@ -373,6 +374,10 @@ static int rds_set_transport(struct net *net, struct rds_sock *rs,
>  	if (t_type < 0 || t_type >= RDS_TRANS_COUNT)
>  		return -EINVAL;
>
> +	/* Only RDS/TCP supports non-initial network namespaces */
> +	if (t_type != RDS_TRANS_TCP && !net_eq(net, &init_net))
> +		return -EPROTOTYPE;
> +

[ ... ]

> diff --git a/net/rds/ib.c b/net/rds/ib.c
> --- a/net/rds/ib.c
> +++ b/net/rds/ib.c
> @@ -490,8 +490,13 @@
>  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]);
> +	struct rds_ib_device *rds_ibdev;
>
> +	/* RDS/IB is only supported in the initial network namespace */
> +	if (!net_eq(net, &init_net))
> +		return -EPROTOTYPE;
> +
                                   ^^^^
Is the wording "is only supported" in this comment precise? Looking at the
existing codebase, rds_ib_cm_handle_connect() already has a comment that
says "RDS/IB is not currently netns aware, thus init_net" with the phrase
"for now" in rds_ib_laddr_check_cm() suggesting this could change. Before
this patch, it was technically possible to attempt using RDS/IB in
non-init namespaces (though it led to the UAF bug). The comment might be
clearer if it said "RDS/IB does not work correctly in non-initial network
namespaces" or "RDS/IB is restricted to the initial network namespace" to
better distinguish between the technical limitation and the enforcement.

> +	rds_ibdev = rds_ib_get_device(addr->s6_addr32[3]);
>  	if (rds_ibdev) {
>  		rds_ib_dev_put(rds_ibdev);
>

The commit message states "there are currently no plans to support such
use." While the existing code clearly does not support non-init namespaces
(with hardcoded &init_net references throughout), this statement about
future plans cannot be verified from code. The existing comments in
rds_ib_cm_handle_connect() and rds_ib_laddr_check_cm() use phrases like
"for now" and "not currently" which could suggest plans might exist or
could be placeholders. Is this policy statement necessary in the commit
message, or would it be sufficient to state the technical facts about the
current limitation?