netdev - Re: Fixing CVE-2017-16939 in v4.4.y and possibly v3.18.y

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:   Sat, 2 Dec 2017 16:20:40 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     netdev@...r.kernel.org, stable@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: Fixing CVE-2017-16939 in v4.4.y and possibly v3.18.y

On 12/01/2017 11:48 AM, Michal Kubecek wrote:
> On Thu, Nov 30, 2017 at 10:37:40AM -0800, Guenter Roeck wrote:
>> Hi,
>>
>> The fix for CVE-2017-16939 has been applied to v4.9.y, but not to v4.4.y
>> and older kernels. However, I confirmed that running the published POC
>> (see https://blogs.securiteam.com/index.php/archives/3535) does crash a 4.4
>> kernel.
>>
>> I confirmed that the following two patches fix the problem in v4.4.y.
>> Please consider applying them to v4.4.y (and possibly v3.18.y).
>>
>> fc9e50f5a5a4e ("netlink: add a start callback for starting a netlink dump")
>> 1137b5e2529a8 ("ipsec: Fix aborted xfrm policy dump crash")
>>
>> My apologies for the noise if this is already under consideration.
> 
> It's a bit too big hammer. As Nicolai Stange noticed when we were

The hammer is just as big as the upstream hammer. Personally I prefer the
upstream patch; I don't see a reason to deviate from upstream just because
the upstream solution is more complex than necessary.

> handling this for SLE12 (where fc9e50f5a5a4e would break kABI), it's

I didn't know that this is even a concern for stable releases. Is there
some guideline that kABI changes should be avoided in stable releases ?

Thanks,
Guenter

> much simpler to use the flag we already have in cb->args[0] to let
> xfrm_dump_policy_done() call xfrm_policy_walk_done() only if the walk
> structure has been initialized. Thus all you need is the patch below.
> 
> Michal Kubecek
> 
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index 7a5a64e70b4d..c01c7a7eb4d3 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -1655,7 +1655,9 @@ static int xfrm_dump_policy_done(struct netlink_callback *cb)
>   	struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *) &cb->args[1];
>   	struct net *net = sock_net(cb->skb->sk);
>   
> -	xfrm_policy_walk_done(walk, net);
> +	/* cb->args[0] is set when walk is initialized */
> +	if (cb->args[0])
> +		xfrm_policy_walk_done(walk, net);
>   	return 0;
>   }
>   
>