netdev - Re: [PATCHv3 iproute2-next] rtnetlink: add new function rtnl_echo

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 for Android: free password hash cracker in your pocket

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

Message-ID: <Y2oWDRIIR6gjkM4a@shredder>
Date:   Tue, 8 Nov 2022 10:40:45 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     Hangbin Liu <liuhangbin@...il.com>
Cc:     netdev@...r.kernel.org, Guillaume Nault <gnault@...hat.com>,
        David Ahern <dsahern@...nel.org>,
        Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCHv3 iproute2-next] rtnetlink: add new function
 rtnl_echo_talk()

On Thu, Sep 29, 2022 at 04:10:16PM +0800, Hangbin Liu wrote:
> diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
> index 59f8f2fb..c87e847f 100644
> --- a/ip/ipnexthop.c
> +++ b/ip/ipnexthop.c
> @@ -919,12 +919,7 @@ static int ipnh_modify(int cmd, unsigned int flags, int argc, char **argv)
>  		.n.nlmsg_type = cmd,
>  		.nhm.nh_family = preferred_family,
>  	};
> -	struct nlmsghdr *answer;
>  	__u32 nh_flags = 0;
> -	int ret;
> -
> -	if (echo_request)
> -		req.n.nlmsg_flags |= NLM_F_ECHO | NLM_F_ACK;
>  
>  	while (argc > 0) {
>  		if (!strcmp(*argv, "id")) {
> @@ -1005,23 +1000,9 @@ static int ipnh_modify(int cmd, unsigned int flags, int argc, char **argv)
>  	req.nhm.nh_flags = nh_flags;
>  
>  	if (echo_request)
> -		ret = rtnl_talk(&rth, &req.n, &answer);
> -	else
> -		ret = rtnl_talk(&rth, &req.n, NULL);
> -
> -	if (ret < 0)
> -		return -2;
> +		return rtnl_echo_talk(&rth, &req.n, json, print_nexthop_nocache);
>  
> -	if (echo_request) {
> -		new_json_obj(json);
> -		open_json_object(NULL);
> -		print_nexthop_nocache(answer, (void *)stdout);
> -		close_json_object();
> -		delete_json_obj();
> -		free(answer);
> -	}
> -
> -	return 0;
> +	return rtnl_talk(&rth, &req.n, NULL);
>  }

Hangbin,

This change breaks the nexthop selftest:
tools/testing/selftests/net/fib_nexthops.sh

Which is specifically checking for "2" as the error code. Example:

# attempt to create nh without a device or gw - fails
run_cmd "$IP nexthop add id 1"
log_test $? 2 "Nexthop with no device or gateway"

I think it's better to restore the original error code than "fixing" all
the tests / applications that rely on it.

The return code of other subcommands was also changed by this patch, but
so far all the failures I have seen are related to "nexthop" subcommand.

Thanks