netdev - Re: [PATCH iproute2 2/2] lib/libnetlink: update rtnl

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]

Date:   Fri, 8 Sep 2017 13:06:52 +0200
From:   Phil Sutter <phil@....cc>
To:     Hangbin Liu <liuhangbin@...il.com>
Cc:     netdev@...r.kernel.org,
        Stephen Hemminger <stephen@...workplumber.org>,
        Michal Kubecek <mkubecek@...e.cz>
Subject: Re: [PATCH iproute2 2/2] lib/libnetlink: update rtnl_talk to support
 malloc buff at run time

Hi Hangbin,

On Fri, Sep 08, 2017 at 06:14:57PM +0800, Hangbin Liu wrote:
[...]
> diff --git a/genl/ctrl.c b/genl/ctrl.c
> index 448988e..699657b 100644
> --- a/genl/ctrl.c
> +++ b/genl/ctrl.c
> @@ -55,6 +55,7 @@ int genl_ctrl_resolve_family(const char *family)
>  	};
>  	struct nlmsghdr *nlh = &req.n;
>  	struct genlmsghdr *ghdr = &req.g;
> +	struct nlmsghdr *answer = NULL;

I don't think it's necessary to assign NULL here or in any of the other
cases.

>  	if (rtnl_open_byproto(&rth, 0, NETLINK_GENERIC) < 0) {
>  		fprintf(stderr, "Cannot open generic netlink socket\n");
> @@ -63,19 +64,19 @@ int genl_ctrl_resolve_family(const char *family)
>  
>  	addattr_l(nlh, 128, CTRL_ATTR_FAMILY_NAME, family, strlen(family) + 1);
>  
> -	if (rtnl_talk(&rth, nlh, nlh, sizeof(req)) < 0) {
> +	if (rtnl_talk(&rth, nlh, &answer) < 0) {

I didn't check, but it's likely that at least in some cases 'nlh'
contains some buffer space to hold the answer (if it will be larger than
the input). If so, this could be dropped since the buffer is no longer
reused.

Apart from that, looks good to me!

Thanks, Phil