netdev - Re: [PATCH main v2 1/2] macsec: add extended packet number (XPN) support

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: <07bc7668-3107-bea2-58e0-75a77af57f7c@kernel.org>
Date:   Wed, 31 Aug 2022 20:52:42 -0600
From:   David Ahern <dsahern@...nel.org>
To:     Emeel Hakim <ehakim@...dia.com>, sd@...asysnail.net
Cc:     tariqt@...dia.com, raeds@...dia.com, netdev@...r.kernel.org
Subject: Re: [PATCH main v2 1/2] macsec: add extended packet number (XPN)
 support

On 8/24/22 3:17 AM, Emeel Hakim wrote:
> @@ -174,14 +181,34 @@ static int parse_sa_args(int *argcp, char ***argvp, struct sa_desc *sa)
>  
>  	while (argc > 0) {
>  		if (strcmp(*argv, "pn") == 0) {
> -			if (sa->pn != 0)
> +			if (sa->pn.pn32 != 0)

pn64 to cover the entire range? ie., pn and xpn on the same command line.

>  				duparg2("pn", "pn");
>  			NEXT_ARG();
> -			ret = get_u32(&sa->pn, *argv, 0);
> +			ret = get_u32(&sa->pn.pn32, *argv, 0);
>  			if (ret)
>  				invarg("expected pn", *argv);
> -			if (sa->pn == 0)
> +			if (sa->pn.pn32 == 0)
>  				invarg("expected pn != 0", *argv);
> +		} else if (strcmp(*argv, "xpn") == 0) {
> +			if (sa->pn.pn64 != 0)
> +				duparg2("xpn", "xpn");
> +			NEXT_ARG();
> +			ret = get_u64(&sa->pn.pn64, *argv, 0);
> +			if (ret)
> +				invarg("expected pn", *argv);
> +			if (sa->pn.pn64 == 0)
> +				invarg("expected pn != 0", *argv);
> +			sa->xpn = true;
> +		} else if (strcmp(*argv, "salt") == 0) {
> +			unsigned int len;
> +
> +			NEXT_ARG();
> +			if (!hexstring_a2n(*argv, sa->salt, MACSEC_SALT_LEN,
> +					   &len))
> +				invarg("expected salt", *argv);
> +		} else if (strcmp(*argv, "ssci") == 0) {
> +			NEXT_ARG();
> +			ret = get_u32(&sa->ssci, *argv, 0);

that can fail, so check ret and throw an error message

>  		} else if (strcmp(*argv, "key") == 0) {
>  			unsigned int len;
>  

...


> @@ -1388,6 +1458,14 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
>  				return ret;
>  			addattr8(n, MACSEC_BUFLEN,
>  				 IFLA_MACSEC_OFFLOAD, offload);
> +		} else if (strcmp(*argv, "xpn") == 0) {
> +			NEXT_ARG();
> +			int i;
> +
> +			i = parse_on_off("xpn", *argv, &ret);

drop the 'i' and just
xpn = parse_on_off("xpn", *argv, &ret);

besides you have i as an int when xpn is bool and parse_on_off returns a
bool.