netdev - RE: [PATCH] rtnetlink: fix rtnl_calcit() and rtnl_dump

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]

Message-ID: <C5551D9AAB213A418B7FD5E4A6F30A0702FAEF48@ORSMSX106.amr.corp.intel.com>
Date:	Mon, 5 Mar 2012 06:14:32 +0000
From:	"Rose, Gregory V" <gregory.v.rose@...el.com>
To:	Eric Dumazet <eric.dumazet@...il.com>,
	David Miller <davem@...emloft.net>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH] rtnetlink: fix rtnl_calcit() and rtnl_dump_ifinfo()

> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@...il.com]
> Sent: Sunday, March 04, 2012 2:32 PM
> To: David Miller
> Cc: Rose, Gregory V; netdev@...r.kernel.org
> Subject: [PATCH] rtnetlink: fix rtnl_calcit() and rtnl_dump_ifinfo()
> 
> nlmsg_parse() might return an error, so test its return value before
> potential random memory accesses.
> 
> Errors introduced in commit 115c9b81928 (rtnetlink: Fix problem with
> buffer allocation)
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
> Cc: Greg Rose <gregory.v.rose@...el.com>
> ---
>  net/core/rtnetlink.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 606a6e8..f965dce 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1060,11 +1060,12 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb,
> struct netlink_callback *cb)
>  	rcu_read_lock();
>  	cb->seq = net->dev_base_seq;
> 
> -	nlmsg_parse(cb->nlh, sizeof(struct rtgenmsg), tb, IFLA_MAX,
> -		    ifla_policy);
> +	if (nlmsg_parse(cb->nlh, sizeof(struct rtgenmsg), tb, IFLA_MAX,
> +			ifla_policy) >= 0) {
> 
> -	if (tb[IFLA_EXT_MASK])
> -		ext_filter_mask = nla_get_u32(tb[IFLA_EXT_MASK]);
> +		if (tb[IFLA_EXT_MASK])
> +			ext_filter_mask = nla_get_u32(tb[IFLA_EXT_MASK]);
> +	}
> 
>  	for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
>  		idx = 0;
> @@ -1900,10 +1901,11 @@ static u16 rtnl_calcit(struct sk_buff *skb, struct
> nlmsghdr *nlh)
>  	u32 ext_filter_mask = 0;
>  	u16 min_ifinfo_dump_size = 0;
> 
> -	nlmsg_parse(nlh, sizeof(struct rtgenmsg), tb, IFLA_MAX,
> ifla_policy);
> -
> -	if (tb[IFLA_EXT_MASK])
> -		ext_filter_mask = nla_get_u32(tb[IFLA_EXT_MASK]);
> +	if (nlmsg_parse(nlh, sizeof(struct rtgenmsg), tb, IFLA_MAX,
> +			ifla_policy) >= 0) {
> +		if (tb[IFLA_EXT_MASK])
> +			ext_filter_mask = nla_get_u32(tb[IFLA_EXT_MASK]);
> +	}
> 
>  	if (!ext_filter_mask)
>  		return NLMSG_GOODSIZE;
> 

I guess I misunderstood some of the internal workings of nlmsg_parse.  Good catch.

Thanks,

- Greg