netdev - RE: [PATCH 1/2] net: af_key: fix sleeping under rcu

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: <063D6719AE5E284EB5DD2968C1650D6D0F6D0092@AcuExch.aculab.com>
Date:	Tue, 4 Mar 2014 12:46:48 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Nikolay Aleksandrov' <nikolay@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:	Dave Jones <davej@...hat.com>,
	Steffen Klassert <steffen.klassert@...unet.com>,
	Fan Du <fan.du@...driver.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: RE: [PATCH 1/2] net: af_key: fix sleeping under rcu

From: Nikolay Aleksandrov
> There's a kmalloc with GFP_KERNEL in a helper
> (pfkey_sadb2xfrm_user_sec_ctx) used in pfkey_compile_policy which is
> called under rcu_read_lock. Adjust pfkey_sadb2xfrm_user_sec_ctx to have
> a gfp argument and adjust the users.
...
> @@ -3239,7 +3240,7 @@ static struct xfrm_policy *pfkey_compile_policy(struct sock *sk, int opt,
>  		}
>  		if ((*dir = verify_sec_ctx_len(p)))
>  			goto out;
> -		uctx = pfkey_sadb2xfrm_user_sec_ctx(sec_ctx);
> +		uctx = pfkey_sadb2xfrm_user_sec_ctx(sec_ctx, GFP_ATOMIC);
>  		*dir = security_xfrm_policy_alloc(&xp->security, uctx);
>  		kfree(uctx);

This looks like the only one that isn't passed GFP_KERNEL.
It looks as though it is missing the check for the allocation failing
(there might be a check inside security_xfrm_policy_alloc()).

In any case it looks as though this ought to be codeable without
the allocation of 'uctx' - since it is freed a line later.

I'd have thought that some of these paths should be allocating
the memory outside the rcu (where they can sleep) and then
possibly freeing the unused data later.

Returning ENOMEM doesn't really seem helpful.

	David



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html