netdev - Re: [PATCH iproute2-next 3/3] iptnl/ip6tnl: Unify local/remote endpoint and 6rd address parsing

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] [day] [month] [year] [list]

Date:   Sat, 10 Feb 2018 09:48:01 -0700
From:   David Ahern <dsahern@...il.com>
To:     Serhey Popovych <serhe.popovych@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH iproute2-next 3/3] iptnl/ip6tnl: Unify local/remote
 endpoint and 6rd address parsing

On 2/9/18 11:06 AM, Serhey Popovych wrote:
> @@ -289,29 +293,16 @@ get_failed:
>  		} else if (strcmp(*argv, "external") == 0) {
>  			metadata = 1;
>  		} else if (strcmp(*argv, "6rd-prefix") == 0) {
> -			inet_prefix prefix;
> -
>  			NEXT_ARG();
> -			if (get_prefix(&prefix, *argv, AF_INET6))
> +			if (get_prefix(&ip6rdprefix, *argv, AF_INET6))
>  				invarg("invalid 6rd_prefix\n", *argv);
> -			memcpy(&ip6rdprefix, prefix.data, 16);
> -			ip6rdprefixlen = prefix.bitlen;
>  		} else if (strcmp(*argv, "6rd-relay_prefix") == 0) {
> -			inet_prefix prefix;
> -
>  			NEXT_ARG();
> -			if (get_prefix(&prefix, *argv, AF_INET))
> +			if (get_prefix(&ip6rdrelayprefix, *argv, AF_INET))
>  				invarg("invalid 6rd-relay_prefix\n", *argv);
> -			memcpy(&ip6rdrelayprefix, prefix.data, 4);
> -			ip6rdrelayprefixlen = prefix.bitlen;
>  		} else if (strcmp(*argv, "6rd-reset") == 0) {
> -			inet_prefix prefix;
> -
> -			get_prefix(&prefix, "2002::", AF_INET6);
> -			memcpy(&ip6rdprefix, prefix.data, 16);
> -			ip6rdprefixlen = 16;
> -			ip6rdrelayprefix = 0;
> -			ip6rdrelayprefixlen = 0;
> +			get_prefix(&ip6rdprefix, "2002::/16", AF_INET6);
> +			ip6rdrelayprefix.flags = 0;
>  		} else if (strcmp(*argv, "fwmark") == 0) {
>  			NEXT_ARG();
>  			if (get_u32(&fwmark, *argv, 0))

I spent far too long staring at the patches to verify you are not
breaking anything. A key reason is the reset of inet_prefix.flags to 0.
That is a low-level detail that needs a properly named helper to make it
clear you are resetting the address to uninitialized.

That is needed in all of the places you have 'inet_prefix.flags = 0;' in
these.