netdev - Re: [PATCH v2] netlink: fixup regression in RTM

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: <3595c056-11ad-d7c8-6025-3b095e529b94@gmail.com>
Date:   Wed, 2 Jan 2019 17:16:40 -0700
From:   David Ahern <dsahern@...il.com>
To:     baloo@...di.net, netdev@...r.kernel.org
Cc:     "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH v2] netlink: fixup regression in RTM_GETADDR

On 12/30/18 7:10 PM, baloo@...di.net wrote:
> From: Arthur Gautier <baloo@...di.net>
> 
> This commit fixes a regression in AF_INET/RTM_GETADDR and
> AF_INET6/RTM_GETADDR.
> 
> Before this commit, the kernel would stop dumping addresses once the first
> skb was full and end the stream with NLMSG_DONE(-EMSGSIZE). The error
> shouldn't be sent back to netlink_dump so the callback is kept alive. The
> userspace is expected to call back with a new empty skb.
> 
> Changes from V1:
>  - The error is not handled in netlink_dump anymore but rather in
>    inet_dump_ifaddr and inet6_dump_addr directly as suggested by
>    David Ahern.
> 
> Fixes: d7e38611b81e ("net/ipv4: Put target net when address dump fails due to bad attributes")
> Fixes: 242afaa6968c ("net/ipv6: Put target net when address dump fails due to bad attributes")
> 
> Cc: David Ahern <dsahern@...il.com>
> Cc: "David S . Miller" <davem@...emloft.net>
> Cc: netdev@...r.kernel.org
> Signed-off-by: Arthur Gautier <baloo@...di.net>
> ---
>  net/ipv4/devinet.c  | 2 +-
>  net/ipv6/addrconf.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index 608a6f4223fb9..fecd0e7672b5d 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -1826,7 +1826,7 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
>  	if (fillargs.netnsid >= 0)
>  		put_net(tgt_net);
>  
> -	return err < 0 ? err : skb->len;
> +	return skb->len ? : err;
>  }
>  
>  static void rtmsg_ifa(int event, struct in_ifaddr *ifa, struct nlmsghdr *nlh,
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 045597b9a7c05..e3cb53b0ef670 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -5154,7 +5154,7 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
>  	if (fillargs.netnsid >= 0)
>  		put_net(tgt_net);
>  
> -	return err < 0 ? err : skb->len;
> +	return skb->len ? : err;
>  }
>  
>  static int inet6_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
> 

That looks correct. The only error for in_dev_dump_addr is -EMSGSIZE and
that should not be returned. The other errors happen with skb->len == 0.
Same with the ipv6 version.

Reviewed-by: David Ahern <dsahern@...il.com>