netdev - Re: [PATCH net] mpls: fix mpls route deletes to not check for route scope and type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87bnh7yprc.fsf@x220.int.ebiederm.org>
Date:	Tue, 26 May 2015 16:48:07 -0500
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Roopa Prabhu <roopa@...ulusnetworks.com>
Cc:	davem@...emloft.net, rshearma@...cade.com, netdev@...r.kernel.org,
	vivek@...ulusnetworks.com
Subject: Re: [PATCH net] mpls: fix mpls route deletes to not check for route scope and type

Roopa Prabhu <roopa@...ulusnetworks.com> writes:

> From: Roopa Prabhu <roopa@...ulusnetworks.com>
>
> This patch fixes incorrect -EINVAL error due to invalid
> scope and type for mpls route deletes.

Well this is embarrassing apparently I did not exercise this code path
in iproute.

Looking through my tests the closest I came was:
ip -M route flush table all

> iproute2 route modify code does not set protocol/scope/type
> for RTM_DELROUTE msgs. mpls code can skip checking for
> these too.

I am really not certain that is the case.  I expect if you check
you will find that rtm_scope is set to 0  aka RT_SCOPE_UNIVERSE.

For scope I don't much care.  The mpls concepts and the ip concepts
don't match.  With mpls packets can be sent from anywhere in the
universe to an address that is valid only on one link.

For rtm_type I think we do care.  IPv4 and IPv6 are a disaster when it
comes to interfaces for setting up multicast routes, and I don't see any
reason why we would need to replicate that disaster for mpls.

As such I would like rtm_type to actually mean something, as for mpls
the lookup for multicast packets and the lookup for unicast packets is
completely different.  Unicast packet addresses are defined by the
receiver, and multicast packet addresses are defined by the sender.

So can we instead fix iproute to set rtm_type == RTN_UNICAST?
At least for mpls.

Eric

> $ip -f mpls route add 100 as 200 via inet 10.1.1.2 dev swp1
>
> $ip -f mpls route show
> 100 as to 200 via inet 10.1.1.2 dev swp1
>
> $ip -f mpls route del 100 as 200 via inet 10.1.1.2 dev swp1
> RTNETLINK answers: Invalid argument
>
> $ip -f mpls route del 100
> RTNETLINK answers: Invalid argument
>
> After patch:
>
> $ip -f mpls route show
> 100 as to 200 via inet 10.1.1.2 dev swp1
>
> $ip -f mpls route del 100 as 200 via inet 10.1.1.2 dev swp1
>
> $ip -f mpls route show
>
> Reported-by: Vivek Venkataraman <vivek@...ulusnetworks.com>
> Suggested-by: Vivek Venkataraman <vivek@...ulusnetworks.com>
> Signed-off-by: Vivek Venkataraman <vivek@...ulusnetworks.com>
> Signed-off-by: Roopa Prabhu <roopa@...ulusnetworks.com>
> ---
>  net/mpls/af_mpls.c |   11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> index 7b3f732..18ab7bf 100644
> --- a/net/mpls/af_mpls.c
> +++ b/net/mpls/af_mpls.c
> @@ -693,10 +693,13 @@ static int rtm_to_route_config(struct sk_buff *skb,  struct nlmsghdr *nlh,
>  	 * (or source specific address in the case of multicast)
>  	 * all addresses have universal scope.
>  	 */
> -	if (rtm->rtm_scope != RT_SCOPE_UNIVERSE)
> -		goto errout;
> -	if (rtm->rtm_type != RTN_UNICAST)
> -		goto errout;
> +	if (nlh->nlmsg_type != RTM_DELROUTE) {
> +		if (rtm->rtm_scope != RT_SCOPE_UNIVERSE)
> +			goto errout;
> +		if (rtm->rtm_type != RTN_UNICAST)
> +			goto errout;
> +	}
> +
>  	if (rtm->rtm_flags != 0)
>  		goto errout;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html