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@xxxxxxxxxx>
---
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 --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
--
2.34.0.rc0.344.g81b53c2807-goog
