diff mbox series

[v3,bpf-next,1/3] bpf: Prevent write to ksym memory

Message ID 20211109003052.3499225-2-haoluo@google.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series bpf: Prevent writing read-only memory | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
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 success Errors and warnings before: 11541 this patch: 11541
netdev/cc_maintainers warning 6 maintainers not CCed: songliubraving@fb.com joe@cilium.io john.fastabend@gmail.com yhs@fb.com netdev@vger.kernel.org kafai@fb.com
netdev/build_clang success Errors and warnings before: 2378 this patch: 2378
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 11262 this patch: 11262
netdev/checkpatch warning WARNING: line length of 113 exceeds 80 columns WARNING: line length of 81 exceeds 80 columns WARNING: line length of 91 exceeds 80 columns WARNING: line length of 97 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next success VM_Test

Commit Message

Hao Luo Nov. 9, 2021, 12:30 a.m. UTC
A ksym could be used to refer a global variable in the kernel.
Previously if this variable is of non-struct type, bpf verifier resolves
its value type to be PTR_TO_MEM, which allows the program to write
to the memory. This patch introduces PTR_TO_RDONLY_MEM, which is
similar to PTR_TO_MEM, but forbids writing. This should prevent
program from writing kernel memory through ksyms.

Right now a PTR_TO_RDONLY_MEM can not be passed into any helper as
argument. But a PTR_TO_RDONLY_MEM can be read into a PTR_TO_MEM (e.g.
stack variable), which can be passed to helpers such as bpf_snprintf.

The following patch will add checks to differentiate the read-write
arguments and read-only arguments, and support for passing
PTR_TO_RDONLY_MEM to helper's read-only arguments.

Fixes: 63d9b80dcf2c ("bpf: Introducte bpf_this_cpu_ptr()")
Fixes: eaa6bcb71ef6 ("bpf: Introduce bpf_per_cpu_ptr()")
Fixes: 4976b718c355 ("bpf: Introduce pseudo_btf_id")
Signed-off-by: Hao Luo <haoluo@google.com>
---
 Changes since v2:
  - Rebase

 Changes since v1:
  - Added Fixes tag.
  - Removed PTR_TO_RDONLY_MEM[_OR_NULL] from reg_type_may_be_refcounted.

 include/linux/bpf.h            |  6 ++++--
 include/uapi/linux/bpf.h       |  4 ++--
 kernel/bpf/helpers.c           |  4 ++--
 kernel/bpf/verifier.c          | 36 ++++++++++++++++++++++++++--------
 tools/include/uapi/linux/bpf.h |  4 ++--
 5 files changed, 38 insertions(+), 16 deletions(-)
diff mbox series

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index df3410bff4b0..64494d5964fa 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -356,8 +356,8 @@  enum bpf_return_type {
 	RET_PTR_TO_SOCK_COMMON_OR_NULL,	/* returns a pointer to a sock_common or NULL */
 	RET_PTR_TO_ALLOC_MEM_OR_NULL,	/* returns a pointer to dynamically allocated memory or NULL */
 	RET_PTR_TO_BTF_ID_OR_NULL,	/* returns a pointer to a btf_id or NULL */
-	RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL, /* returns a pointer to a valid memory or a btf_id or NULL */
-	RET_PTR_TO_MEM_OR_BTF_ID,	/* returns a pointer to a valid memory or a btf_id */
+	RET_PTR_TO_RDONLY_MEM_OR_BTF_ID_OR_NULL, /* returns a pointer to a readonly memory or a btf_id or NULL */
+	RET_PTR_TO_RDONLY_MEM_OR_BTF_ID, /* returns a pointer to a readonly memory or a btf_id */
 	RET_PTR_TO_BTF_ID,		/* returns a pointer to a btf_id */
 };
 
@@ -460,6 +460,8 @@  enum bpf_reg_type {
 	PTR_TO_PERCPU_BTF_ID,	 /* reg points to a percpu kernel variable */
 	PTR_TO_FUNC,		 /* reg points to a bpf program function */
 	PTR_TO_MAP_KEY,		 /* reg points to a map element key */
+	PTR_TO_RDONLY_MEM,	 /* reg points to valid readonly memory region */
+	PTR_TO_RDONLY_MEM_OR_NULL, /* reg points to valid readonly memory region or null */
 	__BPF_REG_TYPE_MAX,
 };
 
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 509eee5f0393..19c8511c1b14 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1142,8 +1142,8 @@  enum bpf_link_type {
  * insn[0].off:      0
  * insn[1].off:      0
  * ldimm64 rewrite:  address of the kernel variable
- * verifier type:    PTR_TO_BTF_ID or PTR_TO_MEM, depending on whether the var
- *                   is struct/union.
+ * verifier type:    PTR_TO_BTF_ID or PTR_TO_RDONLY_MEM, depending on whether
+ *                   the var is struct/union.
  */
 #define BPF_PSEUDO_BTF_ID	3
 /* insn[0].src_reg:  BPF_PSEUDO_FUNC
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 1ffd469c217f..14531757087f 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -667,7 +667,7 @@  BPF_CALL_2(bpf_per_cpu_ptr, const void *, ptr, u32, cpu)
 const struct bpf_func_proto bpf_per_cpu_ptr_proto = {
 	.func		= bpf_per_cpu_ptr,
 	.gpl_only	= false,
-	.ret_type	= RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL,
+	.ret_type	= RET_PTR_TO_RDONLY_MEM_OR_BTF_ID_OR_NULL,
 	.arg1_type	= ARG_PTR_TO_PERCPU_BTF_ID,
 	.arg2_type	= ARG_ANYTHING,
 };
@@ -680,7 +680,7 @@  BPF_CALL_1(bpf_this_cpu_ptr, const void *, percpu_ptr)
 const struct bpf_func_proto bpf_this_cpu_ptr_proto = {
 	.func		= bpf_this_cpu_ptr,
 	.gpl_only	= false,
-	.ret_type	= RET_PTR_TO_MEM_OR_BTF_ID,
+	.ret_type	= RET_PTR_TO_RDONLY_MEM_OR_BTF_ID,
 	.arg1_type	= ARG_PTR_TO_PERCPU_BTF_ID,
 };
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1aafb43f61d1..eb3ae4a140ac 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -453,6 +453,7 @@  static bool reg_type_may_be_null(enum bpf_reg_type type)
 	       type == PTR_TO_TCP_SOCK_OR_NULL ||
 	       type == PTR_TO_BTF_ID_OR_NULL ||
 	       type == PTR_TO_MEM_OR_NULL ||
+	       type == PTR_TO_RDONLY_MEM_OR_NULL ||
 	       type == PTR_TO_RDONLY_BUF_OR_NULL ||
 	       type == PTR_TO_RDWR_BUF_OR_NULL;
 }
@@ -571,6 +572,8 @@  static const char * const reg_type_str[] = {
 	[PTR_TO_PERCPU_BTF_ID]	= "percpu_ptr_",
 	[PTR_TO_MEM]		= "mem",
 	[PTR_TO_MEM_OR_NULL]	= "mem_or_null",
+	[PTR_TO_RDONLY_MEM]	= "rdonly_mem",
+	[PTR_TO_RDONLY_MEM_OR_NULL] = "rdonly_mem_or_null",
 	[PTR_TO_RDONLY_BUF]	= "rdonly_buf",
 	[PTR_TO_RDONLY_BUF_OR_NULL] = "rdonly_buf_or_null",
 	[PTR_TO_RDWR_BUF]	= "rdwr_buf",
@@ -1183,6 +1186,9 @@  static void mark_ptr_not_null_reg(struct bpf_reg_state *reg)
 	case PTR_TO_MEM_OR_NULL:
 		reg->type = PTR_TO_MEM;
 		break;
+	case PTR_TO_RDONLY_MEM_OR_NULL:
+		reg->type = PTR_TO_RDONLY_MEM;
+		break;
 	case PTR_TO_RDONLY_BUF_OR_NULL:
 		reg->type = PTR_TO_RDONLY_BUF;
 		break;
@@ -2741,6 +2747,8 @@  static bool is_spillable_regtype(enum bpf_reg_type type)
 	case PTR_TO_PERCPU_BTF_ID:
 	case PTR_TO_MEM:
 	case PTR_TO_MEM_OR_NULL:
+	case PTR_TO_RDONLY_MEM:
+	case PTR_TO_RDONLY_MEM_OR_NULL:
 	case PTR_TO_FUNC:
 	case PTR_TO_MAP_KEY:
 		return true;
@@ -3367,6 +3375,7 @@  static int __check_mem_access(struct bpf_verifier_env *env, int regno,
 			off, size, regno, reg->id, off, mem_size);
 		break;
 	case PTR_TO_MEM:
+	case PTR_TO_RDONLY_MEM:
 	default:
 		verbose(env, "invalid access to memory, mem_size=%u off=%d size=%d\n",
 			mem_size, off, size);
@@ -4377,6 +4386,16 @@  static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 					      reg->mem_size, false);
 		if (!err && t == BPF_READ && value_regno >= 0)
 			mark_reg_unknown(env, regs, value_regno);
+	} else if (reg->type == PTR_TO_RDONLY_MEM) {
+		if (t == BPF_WRITE) {
+			verbose(env, "R%d cannot write into %s\n",
+				regno, reg_type_str[reg->type]);
+			return -EACCES;
+		}
+		err = check_mem_region_access(env, regno, off, size,
+					      reg->mem_size, false);
+		if (!err && value_regno >= 0)
+			mark_reg_unknown(env, regs, value_regno);
 	} else if (reg->type == PTR_TO_CTX) {
 		enum bpf_reg_type reg_type = SCALAR_VALUE;
 		struct btf *btf = NULL;
@@ -6579,8 +6598,8 @@  static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
 		regs[BPF_REG_0].mem_size = meta.mem_size;
-	} else if (fn->ret_type == RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL ||
-		   fn->ret_type == RET_PTR_TO_MEM_OR_BTF_ID) {
+	} else if (fn->ret_type == RET_PTR_TO_RDONLY_MEM_OR_BTF_ID_OR_NULL ||
+		   fn->ret_type == RET_PTR_TO_RDONLY_MEM_OR_BTF_ID) {
 		const struct btf_type *t;
 
 		mark_reg_known_zero(env, regs, BPF_REG_0);
@@ -6599,12 +6618,12 @@  static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 				return -EINVAL;
 			}
 			regs[BPF_REG_0].type =
-				fn->ret_type == RET_PTR_TO_MEM_OR_BTF_ID ?
-				PTR_TO_MEM : PTR_TO_MEM_OR_NULL;
+				fn->ret_type == RET_PTR_TO_RDONLY_MEM_OR_BTF_ID ?
+				PTR_TO_RDONLY_MEM : PTR_TO_RDONLY_MEM_OR_NULL;
 			regs[BPF_REG_0].mem_size = tsize;
 		} else {
 			regs[BPF_REG_0].type =
-				fn->ret_type == RET_PTR_TO_MEM_OR_BTF_ID ?
+				fn->ret_type == RET_PTR_TO_RDONLY_MEM_OR_BTF_ID ?
 				PTR_TO_BTF_ID : PTR_TO_BTF_ID_OR_NULL;
 			regs[BPF_REG_0].btf = meta.ret_btf;
 			regs[BPF_REG_0].btf_id = meta.ret_btf_id;
@@ -9410,7 +9429,7 @@  static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
 
 		dst_reg->type = aux->btf_var.reg_type;
 		switch (dst_reg->type) {
-		case PTR_TO_MEM:
+		case PTR_TO_RDONLY_MEM:
 			dst_reg->mem_size = aux->btf_var.mem_size;
 			break;
 		case PTR_TO_BTF_ID:
@@ -11557,7 +11576,7 @@  static int check_pseudo_btf_id(struct bpf_verifier_env *env,
 			err = -EINVAL;
 			goto err_put;
 		}
-		aux->btf_var.reg_type = PTR_TO_MEM;
+		aux->btf_var.reg_type = PTR_TO_RDONLY_MEM;
 		aux->btf_var.mem_size = tsize;
 	} else {
 		aux->btf_var.reg_type = PTR_TO_BTF_ID;
@@ -13379,7 +13398,8 @@  static int do_check_common(struct bpf_verifier_env *env, int subprog)
 				mark_reg_known_zero(env, regs, i);
 			else if (regs[i].type == SCALAR_VALUE)
 				mark_reg_unknown(env, regs, i);
-			else if (regs[i].type == PTR_TO_MEM_OR_NULL) {
+			else if (regs[i].type == PTR_TO_MEM_OR_NULL ||
+				 regs[i].type == PTR_TO_RDONLY_MEM_OR_NULL) {
 				const u32 mem_size = regs[i].mem_size;
 
 				mark_reg_known_zero(env, regs, i);
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 509eee5f0393..19c8511c1b14 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1142,8 +1142,8 @@  enum bpf_link_type {
  * insn[0].off:      0
  * insn[1].off:      0
  * ldimm64 rewrite:  address of the kernel variable
- * verifier type:    PTR_TO_BTF_ID or PTR_TO_MEM, depending on whether the var
- *                   is struct/union.
+ * verifier type:    PTR_TO_BTF_ID or PTR_TO_RDONLY_MEM, depending on whether
+ *                   the var is struct/union.
  */
 #define BPF_PSEUDO_BTF_ID	3
 /* insn[0].src_reg:  BPF_PSEUDO_FUNC