Re: [PATCH mptcp-next v13 05/14] mptcp: check userspace pm flags

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 16/02/2024 08:42, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Just like MPTCP_PM_ADDR_FLAG_SIGNAL flag is checked in userspace PM
> announce mptcp_pm_nl_announce_doit(), PM flags should be checked in
> mptcp_pm_nl_subflow_create_doit() too.
> 
> If MPTCP_PM_ADDR_FLAG_SUBFLOW flag is not set, there's no flags field
> in the output of dump_addr. This looks a bit strange:
> 
>         id 10 flags  10.0.3.2
> 
> This patch uses mptcp_pm_parse_entry() instead of mptcp_pm_parse_addr()
> to get the PM flags of the entry and check it. MPTCP_PM_ADDR_FLAG_SIGNAL
> flag shouldn't be set here, and if MPTCP_PM_ADDR_FLAG_SUBFLOW flag is
> missing from the netlink attribute, always set this flag.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm_userspace.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index d6b7be3afbe5..73ad286247f1 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -360,11 +360,20 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
>  		goto create_err;
>  	}
>  
> -	err = mptcp_pm_parse_addr(laddr, info, &addr_l);
> +	err = mptcp_pm_parse_entry(laddr, info, true, &local);
>  	if (err < 0) {
>  		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
>  		goto create_err;
>  	}
> +	addr_l = local.addr;

Detail: do we still need addr_l? Can we not use local.addr directly below?

> +
> +	if (local.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
> +		GENL_SET_ERR_MSG(info, "invalid addr flags");
> +		err = -EINVAL;
> +		goto create_err;
> +	}
> +	if (!(local.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
> +		local.flags |= MPTCP_PM_ADDR_FLAG_SUBFLOW;

(not sure the 'if' is needed: why not always setting it?)

>  
>  	err = mptcp_pm_parse_addr(raddr, info, &addr_r);
>  	if (err < 0) {
> @@ -378,7 +387,6 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
>  		goto create_err;
>  	}
>  
> -	local.addr = addr_l;
>  	err = mptcp_userspace_pm_append_new_local_addr(msk, &local, false);
>  	if (err < 0) {
>  		GENL_SET_ERR_MSG(info, "did not match address and id");

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.