Re: [tipc-discussion] [net v2] tipc: fix unique bearer names sanity check

[Jon M. <jm...@re...>]

On 3/29/21 10:16 PM, Hoang Le wrote:
> When enabling a bearer

s/with identify//g


> by name, we don't sanity check
> its name with higher slot in bearer list. This lead to duplicate
> bearer names bypassed the check.
This may have the effect that the name of an already enabled bearer 
bypasses the check.
>
> To fix the above issue, we just perform an extra checking with all
> existing bearers.
>
> Fixes: cb30a63384bc9 ("tipc: refactor function tipc_enable_bearer()")
> Signed-off-by: Hoang Le <hoa...@de...>
> ---
>   net/tipc/bearer.c | 45 ++++++++++++++++++++++++++-------------------
>   1 file changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index d47e0b940ac9..94eddc67d52e 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -256,6 +256,7 @@ static int tipc_enable_bearer(struct net *net, const char *name,
>   	int bearer_id = 0;
>   	int res = -EINVAL;
>   	char *errstr = "";
> +	u32 i;
>   
>   	if (!bearer_name_validate(name, &b_names)) {
>   		errstr = "illegal name";
> @@ -280,31 +281,37 @@ static int tipc_enable_bearer(struct net *net, const char *name,
>   		prio = m->priority;
>   
>   	/* Check new bearer vs existing ones and find free bearer id if any */
> -	while (bearer_id < MAX_BEARERS) {
> -		b = rtnl_dereference(tn->bearer_list[bearer_id]);
> -		if (!b)
> -			break;
> +	bearer_id = MAX_BEARERS;
> +	i = MAX_BEARERS;
> +	while (i-- != 0) {
> +		b = rtnl_dereference(tn->bearer_list[i]);
> +		if (!b) {
> +			bearer_id = i;
> +			continue;
> +		}
>   		if (!strcmp(name, b->name)) {
>   			errstr = "already enabled";
>   			NL_SET_ERR_MSG(extack, "Already enabled");
>   			goto rejected;
>   		}
> -		bearer_id++;
> -		if (b->priority != prio)
> -			continue;
> -		if (++with_this_prio <= 2)
> -			continue;
> -		pr_warn("Bearer <%s>: already 2 bearers with priority %u\n",
> -			name, prio);
> -		if (prio == TIPC_MIN_LINK_PRI) {
> -			errstr = "cannot adjust to lower";
> -			NL_SET_ERR_MSG(extack, "Cannot adjust to lower");
> -			goto rejected;
> +
> +		if (b->priority == prio &&
> +		    (++with_this_prio > 2)) {
> +			pr_warn("Bearer <%s>: already 2 bearers with priority %u\n",
> +				name, prio);
> +
> +			if (prio == TIPC_MIN_LINK_PRI) {
> +				errstr = "cannot adjust to lower";
> +				NL_SET_ERR_MSG(extack, "Cannot adjust to lower");
> +				goto rejected;
> +			}
> +
> +			pr_warn("Bearer <%s>: trying with adjusted priority\n", name);
> +			prio--;
> +			bearer_id = MAX_BEARERS;
> +			i = MAX_BEARERS;
> +			with_this_prio = 1;
>   		}
> -		pr_warn("Bearer <%s>: trying with adjusted priority\n", name);
> -		prio--;
> -		bearer_id = 0;
> -		with_this_prio = 1;
>   	}
>   
>   	if (bearer_id >= MAX_BEARERS) {
Acked-by: Jon Maloy <jm...@re...>