From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roman Gushchin Subject: Re: [PATCH bpf] bpf: fix wrong helper enablement in cgroup local storage Date: Fri, 26 Oct 2018 22:57:18 +0000 Message-ID: <20181026225709.GA12467@tower.DHCP.thefacebook.com> References: <20181026224902.12455-1-daniel@iogearbox.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "ast@kernel.org" , "netdev@vger.kernel.org" To: Daniel Borkmann Return-path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:56336 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728133AbeJ0Hg7 (ORCPT ); Sat, 27 Oct 2018 03:36:59 -0400 In-Reply-To: <20181026224902.12455-1-daniel@iogearbox.net> Content-Language: en-US Content-ID: <4033A2FFA92039449E8F4390CAB6FFA4@namprd15.prod.outlook.com> Sender: netdev-owner@vger.kernel.org List-ID: On Sat, Oct 27, 2018 at 12:49:02AM +0200, Daniel Borkmann wrote: > Commit cd3394317653 ("bpf: introduce the bpf_get_local_storage() > helper function") enabled the bpf_get_local_storage() helper also > for BPF program types where it does not make sense to use them. >=20 > They have been added both in sk_skb_func_proto() and sk_msg_func_proto() > even though both program types are not invoked in combination with > cgroups, and neither through BPF_PROG_RUN_ARRAY(). In the latter the > bpf_cgroup_storage_set() is set shortly before BPF program invocation. >=20 > Later, the helper bpf_get_local_storage() retrieves this prior set > up per-cpu pointer and hands the buffer to the BPF program. The map > argument in there solely retrieves the enum bpf_cgroup_storage_type > from a local storage map associated with the program and based on the > type returns either the global or per-cpu storage. However, there > is no specific association between the program's map and the actual > content in bpf_cgroup_storage[]. >=20 > Meaning, any BPF program that would have been properly run from the > cgroup side through BPF_PROG_RUN_ARRAY() where bpf_cgroup_storage_set() > was performed, and that is later unloaded such that prog / maps are > teared down will cause a use after free if that pointer is retrieved > from programs that are not run through BPF_PROG_RUN_ARRAY() but have > the cgroup local storage helper enabled in their func proto. >=20 > Lets just remove it from the two sock_map program types to fix it. > Auditing through the types where this helper is enabled, it appears > that these are the only ones where it was mistakenly allowed. >=20 > Fixes: cd3394317653 ("bpf: introduce the bpf_get_local_storage() helper f= unction") > Signed-off-by: Daniel Borkmann > Cc: Roman Gushchin > Acked-by: John Fastabend Acked-by: Roman Gushchin Thanks, Daniel!