diff mbox series

[bpf-next,1/3] bpf: add signature to eBPF instructions

Message ID 20211203191844.69709-2-mcroce@linux.microsoft.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series bpf: add signature | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR pending PR summary
bpf/vmtest-bpf-next pending VM_Test
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 12515 this patch: 12518
netdev/cc_maintainers warning 4 maintainers not CCed: netdev@vger.kernel.org dhowells@redhat.com davem@davemloft.net herbert@gondor.apana.org.au
netdev/build_clang success Errors and warnings before: 2126 this patch: 2126
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 11680 this patch: 11681
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns WARNING: please write a paragraph that describes the config symbol fully
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Matteo Croce Dec. 3, 2021, 7:18 p.m. UTC 
From: Matteo Croce <mcroce@microsoft.com>

When loading a BPF program, pass a signature which is used to validate
the instructions.
The signature type is the same used to validate the kernel modules.

This happens when loading a program with, respectively, an invalid and
a valid signature:

    # ./core-bad
    [ 8524.417567] Invalid BPF signature for '__loader.prog': -EKEYREJECTED
    failed to open and/or load BPF object
    # ./core-ok

Signed-off-by: Matteo Croce <mcroce@microsoft.com>
---
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 crypto/asymmetric_keys/pkcs7_verify.c    |  7 +++-
 include/linux/verification.h             |  1 +
 include/uapi/linux/bpf.h                 |  2 +
 kernel/bpf/Kconfig                       |  8 ++++
 kernel/bpf/syscall.c                     | 47 +++++++++++++++++++++---
 6 files changed, 59 insertions(+), 7 deletions(-)

Comments

kernel test robot Dec. 3, 2021, 9:46 p.m. UTC | #1 
Hi Matteo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Matteo-Croce/bpf-add-signature/20211204-032018
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: nds32-allyesconfig (https://download.01.org/0day-ci/archive/20211204/202112040507.siNkODlN-lkp@intel.com/config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/fdfe32b9e64c6a208965002215d467ec383b6f57
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matteo-Croce/bpf-add-signature/20211204-032018
        git checkout fdfe32b9e64c6a208965002215d467ec383b6f57
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash kernel/bpf/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   kernel/bpf/syscall.c: In function 'bpf_prog_load':
>> kernel/bpf/syscall.c:2324:47: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
    2324 |                 if (copy_from_user(signature, (char *)attr->signature, attr->sig_len)) {
         |                                               ^


vim +2324 kernel/bpf/syscall.c

  2192	
  2193	static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
  2194	{
  2195		enum bpf_prog_type type = attr->prog_type;
  2196		struct bpf_prog *prog, *dst_prog = NULL;
  2197		struct btf *attach_btf = NULL;
  2198		int err;
  2199		char license[128];
  2200		bool is_gpl;
  2201	
  2202		if (CHECK_ATTR(BPF_PROG_LOAD))
  2203			return -EINVAL;
  2204	
  2205		if (attr->prog_flags & ~(BPF_F_STRICT_ALIGNMENT |
  2206					 BPF_F_ANY_ALIGNMENT |
  2207					 BPF_F_TEST_STATE_FREQ |
  2208					 BPF_F_SLEEPABLE |
  2209					 BPF_F_TEST_RND_HI32))
  2210			return -EINVAL;
  2211	
  2212		if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
  2213		    (attr->prog_flags & BPF_F_ANY_ALIGNMENT) &&
  2214		    !bpf_capable())
  2215			return -EPERM;
  2216	
  2217		/* copy eBPF program license from user space */
  2218		if (strncpy_from_bpfptr(license,
  2219					make_bpfptr(attr->license, uattr.is_kernel),
  2220					sizeof(license) - 1) < 0)
  2221			return -EFAULT;
  2222		license[sizeof(license) - 1] = 0;
  2223	
  2224		/* eBPF programs must be GPL compatible to use GPL-ed functions */
  2225		is_gpl = license_is_gpl_compatible(license);
  2226	
  2227		if (attr->insn_cnt == 0 ||
  2228		    attr->insn_cnt > (bpf_capable() ? BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
  2229			return -E2BIG;
  2230		if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
  2231		    type != BPF_PROG_TYPE_CGROUP_SKB &&
  2232		    !bpf_capable())
  2233			return -EPERM;
  2234	
  2235		if (is_net_admin_prog_type(type) && !capable(CAP_NET_ADMIN) && !capable(CAP_SYS_ADMIN))
  2236			return -EPERM;
  2237		if (is_perfmon_prog_type(type) && !perfmon_capable())
  2238			return -EPERM;
  2239	
  2240		/* attach_prog_fd/attach_btf_obj_fd can specify fd of either bpf_prog
  2241		 * or btf, we need to check which one it is
  2242		 */
  2243		if (attr->attach_prog_fd) {
  2244			dst_prog = bpf_prog_get(attr->attach_prog_fd);
  2245			if (IS_ERR(dst_prog)) {
  2246				dst_prog = NULL;
  2247				attach_btf = btf_get_by_fd(attr->attach_btf_obj_fd);
  2248				if (IS_ERR(attach_btf))
  2249					return -EINVAL;
  2250				if (!btf_is_kernel(attach_btf)) {
  2251					/* attaching through specifying bpf_prog's BTF
  2252					 * objects directly might be supported eventually
  2253					 */
  2254					btf_put(attach_btf);
  2255					return -ENOTSUPP;
  2256				}
  2257			}
  2258		} else if (attr->attach_btf_id) {
  2259			/* fall back to vmlinux BTF, if BTF type ID is specified */
  2260			attach_btf = bpf_get_btf_vmlinux();
  2261			if (IS_ERR(attach_btf))
  2262				return PTR_ERR(attach_btf);
  2263			if (!attach_btf)
  2264				return -EINVAL;
  2265			btf_get(attach_btf);
  2266		}
  2267	
  2268		bpf_prog_load_fixup_attach_type(attr);
  2269		if (bpf_prog_load_check_attach(type, attr->expected_attach_type,
  2270					       attach_btf, attr->attach_btf_id,
  2271					       dst_prog)) {
  2272			if (dst_prog)
  2273				bpf_prog_put(dst_prog);
  2274			if (attach_btf)
  2275				btf_put(attach_btf);
  2276			return -EINVAL;
  2277		}
  2278	
  2279		/* plain bpf_prog allocation */
  2280		prog = bpf_prog_alloc(bpf_prog_size(attr->insn_cnt), GFP_USER);
  2281		if (!prog) {
  2282			if (dst_prog)
  2283				bpf_prog_put(dst_prog);
  2284			if (attach_btf)
  2285				btf_put(attach_btf);
  2286			return -ENOMEM;
  2287		}
  2288	
  2289		prog->expected_attach_type = attr->expected_attach_type;
  2290		prog->aux->attach_btf = attach_btf;
  2291		prog->aux->attach_btf_id = attr->attach_btf_id;
  2292		prog->aux->dst_prog = dst_prog;
  2293		prog->aux->offload_requested = !!attr->prog_ifindex;
  2294		prog->aux->sleepable = attr->prog_flags & BPF_F_SLEEPABLE;
  2295	
  2296		err = security_bpf_prog_alloc(prog->aux);
  2297		if (err)
  2298			goto free_prog;
  2299	
  2300		prog->aux->user = get_current_user();
  2301		prog->len = attr->insn_cnt;
  2302	
  2303		err = -EFAULT;
  2304		if (copy_from_bpfptr(prog->insns,
  2305				     make_bpfptr(attr->insns, uattr.is_kernel),
  2306				     bpf_prog_insn_size(prog)) != 0)
  2307			goto free_prog_sec;
  2308	
  2309		err = bpf_obj_name_cpy(prog->aux->name, attr->prog_name,
  2310				       sizeof(attr->prog_name));
  2311		if (err < 0)
  2312			goto free_prog_sec;
  2313	
  2314	#ifdef CONFIG_BPF_SIG
  2315		if (attr->sig_len) {
  2316			char *signature;
  2317	
  2318			signature = kmalloc(attr->sig_len, GFP_USER);
  2319			if (!signature) {
  2320				err = -ENOMEM;
  2321				goto free_prog_sec;
  2322			}
  2323	
> 2324			if (copy_from_user(signature, (char *)attr->signature, attr->sig_len)) {
  2325				err = -EFAULT;
  2326				kfree(signature);
  2327				goto free_prog_sec;
  2328			}
  2329	
  2330			err = verify_pkcs7_signature(prog->insns,
  2331						     prog->len * sizeof(struct bpf_insn),
  2332						     signature, attr->sig_len,
  2333						     VERIFY_USE_SECONDARY_KEYRING,
  2334						     VERIFYING_BPF_SIGNATURE,
  2335						     NULL, NULL);
  2336			kfree(signature);
  2337	
  2338			if (err) {
  2339				pr_warn("Invalid BPF signature for '%s': %pe\n",
  2340					prog->aux->name, ERR_PTR(err));
  2341				goto free_prog_sec;
  2342			}
  2343		}
  2344	#endif
  2345	
  2346		prog->orig_prog = NULL;
  2347		prog->jited = 0;
  2348	
  2349		atomic64_set(&prog->aux->refcnt, 1);
  2350		prog->gpl_compatible = is_gpl ? 1 : 0;
  2351	
  2352		if (bpf_prog_is_dev_bound(prog->aux)) {
  2353			err = bpf_prog_offload_init(prog, attr);
  2354			if (err)
  2355				goto free_prog_sec;
  2356		}
  2357	
  2358		/* find program type: socket_filter vs tracing_filter */
  2359		err = find_prog_type(type, prog);
  2360		if (err < 0)
  2361			goto free_prog_sec;
  2362	
  2363		prog->aux->load_time = ktime_get_boottime_ns();
  2364	
  2365		/* run eBPF verifier */
  2366		err = bpf_check(&prog, attr, uattr);
  2367		if (err < 0)
  2368			goto free_used_maps;
  2369	
  2370		prog = bpf_prog_select_runtime(prog, &err);
  2371		if (err < 0)
  2372			goto free_used_maps;
  2373	
  2374		err = bpf_prog_alloc_id(prog);
  2375		if (err)
  2376			goto free_used_maps;
  2377	
  2378		/* Upon success of bpf_prog_alloc_id(), the BPF prog is
  2379		 * effectively publicly exposed. However, retrieving via
  2380		 * bpf_prog_get_fd_by_id() will take another reference,
  2381		 * therefore it cannot be gone underneath us.
  2382		 *
  2383		 * Only for the time /after/ successful bpf_prog_new_fd()
  2384		 * and before returning to userspace, we might just hold
  2385		 * one reference and any parallel close on that fd could
  2386		 * rip everything out. Hence, below notifications must
  2387		 * happen before bpf_prog_new_fd().
  2388		 *
  2389		 * Also, any failure handling from this point onwards must
  2390		 * be using bpf_prog_put() given the program is exposed.
  2391		 */
  2392		bpf_prog_kallsyms_add(prog);
  2393		perf_event_bpf_event(prog, PERF_BPF_EVENT_PROG_LOAD, 0);
  2394		bpf_audit_prog(prog, BPF_AUDIT_LOAD);
  2395	
  2396		err = bpf_prog_new_fd(prog);
  2397		if (err < 0)
  2398			bpf_prog_put(prog);
  2399		return err;
  2400	
  2401	free_used_maps:
  2402		/* In case we have subprogs, we need to wait for a grace
  2403		 * period before we can tear down JIT memory since symbols
  2404		 * are already exposed under kallsyms.
  2405		 */
  2406		__bpf_prog_put_noref(prog, prog->aux->func_cnt);
  2407		return err;
  2408	free_prog_sec:
  2409		free_uid(prog->aux->user);
  2410		security_bpf_prog_free(prog->aux);
  2411	free_prog:
  2412		if (prog->aux->attach_btf)
  2413			btf_put(prog->aux->attach_btf);
  2414		bpf_prog_free(prog);
  2415		return err;
  2416	}
  2417	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index ad8af3d70ac0..e4f2fee19c5f 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -26,6 +26,7 @@  const char *const key_being_used_for[NR__KEY_BEING_USED_FOR] = {
 	[VERIFYING_KEY_SIGNATURE]		= "key sig",
 	[VERIFYING_KEY_SELF_SIGNATURE]		= "key self sig",
 	[VERIFYING_UNSPECIFIED_SIGNATURE]	= "unspec sig",
+	[VERIFYING_BPF_SIGNATURE]		= "bpf sig",
 };
 EXPORT_SYMBOL_GPL(key_being_used_for);
 
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 0b4d07aa8811..ab645f23c021 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -411,12 +411,15 @@  int pkcs7_verify(struct pkcs7_message *pkcs7,
 
 	switch (usage) {
 	case VERIFYING_MODULE_SIGNATURE:
+	case VERIFYING_BPF_SIGNATURE:
 		if (pkcs7->data_type != OID_data) {
-			pr_warn("Invalid module sig (not pkcs7-data)\n");
+			pr_warn("Invalid %s (not pkcs7-data)\n",
+				key_being_used_for[usage]);
 			return -EKEYREJECTED;
 		}
 		if (pkcs7->have_authattrs) {
-			pr_warn("Invalid module sig (has authattrs)\n");
+			pr_warn("Invalid %s (has authattrs)\n",
+				key_being_used_for[usage]);
 			return -EKEYREJECTED;
 		}
 		break;
diff --git a/include/linux/verification.h b/include/linux/verification.h
index a655923335ae..71482644eea0 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -27,6 +27,7 @@  enum key_being_used_for {
 	VERIFYING_KEY_SIGNATURE,
 	VERIFYING_KEY_SELF_SIGNATURE,
 	VERIFYING_UNSPECIFIED_SIGNATURE,
+	VERIFYING_BPF_SIGNATURE,
 	NR__KEY_BEING_USED_FOR
 };
 extern const char *const key_being_used_for[NR__KEY_BEING_USED_FOR];
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c26871263f1f..bbb4435c7586 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1346,6 +1346,8 @@  union bpf_attr {
 		__aligned_u64	fd_array;	/* array of FDs */
 		__aligned_u64	core_relos;
 		__u32		core_relo_rec_size; /* sizeof(struct bpf_core_relo) */
+		__aligned_u64	signature;	/* instruction's signature */
+		__u32		sig_len;	/* signature size */
 	};
 
 	struct { /* anonymous struct used by BPF_OBJ_* commands */
diff --git a/kernel/bpf/Kconfig b/kernel/bpf/Kconfig
index d24d518ddd63..735979bb8672 100644
--- a/kernel/bpf/Kconfig
+++ b/kernel/bpf/Kconfig
@@ -79,6 +79,14 @@  config BPF_UNPRIV_DEFAULT_OFF
 
 	  If you are unsure how to answer this question, answer Y.
 
+config BPF_SIG
+	bool "BPF signature verification"
+	select SYSTEM_DATA_VERIFICATION
+	depends on BPF_SYSCALL
+	help
+	  Check BPF programs for valid signatures upon load: the signature
+	  is passed via the bpf() syscall together with the instructions.
+
 source "kernel/bpf/preload/Kconfig"
 
 config BPF_LSM
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b3ada4085f85..5aaa74a72b46 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -32,6 +32,10 @@ 
 #include <linux/rcupdate_trace.h>
 #include <linux/memcontrol.h>
 
+#ifdef CONFIG_BPF_SIG
+#include <linux/verification.h>
+#endif
+
 #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
 			  (map)->map_type == BPF_MAP_TYPE_CGROUP_ARRAY || \
 			  (map)->map_type == BPF_MAP_TYPE_ARRAY_OF_MAPS)
@@ -2184,7 +2188,7 @@  static bool is_perfmon_prog_type(enum bpf_prog_type prog_type)
 }
 
 /* last field in 'union bpf_attr' used by this command */
-#define	BPF_PROG_LOAD_LAST_FIELD core_relo_rec_size
+#define	BPF_PROG_LOAD_LAST_FIELD sig_len
 
 static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
 {
@@ -2302,6 +2306,43 @@  static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
 			     bpf_prog_insn_size(prog)) != 0)
 		goto free_prog_sec;
 
+	err = bpf_obj_name_cpy(prog->aux->name, attr->prog_name,
+			       sizeof(attr->prog_name));
+	if (err < 0)
+		goto free_prog_sec;
+
+#ifdef CONFIG_BPF_SIG
+	if (attr->sig_len) {
+		char *signature;
+
+		signature = kmalloc(attr->sig_len, GFP_USER);
+		if (!signature) {
+			err = -ENOMEM;
+			goto free_prog_sec;
+		}
+
+		if (copy_from_user(signature, (char *)attr->signature, attr->sig_len)) {
+			err = -EFAULT;
+			kfree(signature);
+			goto free_prog_sec;
+		}
+
+		err = verify_pkcs7_signature(prog->insns,
+					     prog->len * sizeof(struct bpf_insn),
+					     signature, attr->sig_len,
+					     VERIFY_USE_SECONDARY_KEYRING,
+					     VERIFYING_BPF_SIGNATURE,
+					     NULL, NULL);
+		kfree(signature);
+
+		if (err) {
+			pr_warn("Invalid BPF signature for '%s': %pe\n",
+				prog->aux->name, ERR_PTR(err));
+			goto free_prog_sec;
+		}
+	}
+#endif
+
 	prog->orig_prog = NULL;
 	prog->jited = 0;
 
@@ -2320,10 +2361,6 @@  static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
 		goto free_prog_sec;
 
 	prog->aux->load_time = ktime_get_boottime_ns();
-	err = bpf_obj_name_cpy(prog->aux->name, attr->prog_name,
-			       sizeof(attr->prog_name));
-	if (err < 0)
-		goto free_prog_sec;
 
 	/* run eBPF verifier */
 	err = bpf_check(&prog, attr, uattr);