On Mon, Oct 25, 2021 at 04:12:55PM -0700, Hao Luo wrote:
> Some helper functions may modify its arguments, for example,
> bpf_d_path, bpf_get_stack etc. Previously, their argument types
> were marked as ARG_PTR_TO_MEM, which is compatible with read-only
> mem types, such as PTR_TO_RDONLY_BUF. Therefore it's legitimate
> to modify a read-only memory by passing it into one of such helper
> functions.
>
> This patch introduces a new arg type ARG_PTR_TO_WRITABLE_MEM to
> annotate the arguments that may be modified by the helpers. For
> arguments that are of ARG_PTR_TO_MEM, it's ok to take any mem type,
> while for ARG_PTR_TO_WRITABLE_MEM, readonly mem reg types are not
> acceptable.
>
> In short, when a helper may modify its input parameter, use
> ARG_PTR_TO_WRITABLE_MEM instead of ARG_PTR_TO_MEM.
>
> So far the difference between ARG_PTR_TO_MEM and ARG_PTR_TO_WRITABLE_MEM
> is PTR_TO_RDONLY_BUF and PTR_TO_RDONLY_MEM. PTR_TO_RDONLY_BUF is
> only used in bpf_iter prog as the type of key, which hasn't been
> used in the affected helper functions. PTR_TO_RDONLY_MEM currently
> has no consumers.
>
> Signed-off-by: Hao Luo <haoluo@xxxxxxxxxx>
> ---
> Changes since v1:
> - new patch, introduced ARG_PTR_TO_WRITABLE_MEM to differentiate
> read-only and read-write mem arg types.
>
> include/linux/bpf.h | 9 +++++++++
> kernel/bpf/cgroup.c | 2 +-
> kernel/bpf/helpers.c | 2 +-
> kernel/bpf/verifier.c | 18 ++++++++++++++++++
> kernel/trace/bpf_trace.c | 6 +++---
> net/core/filter.c | 6 +++---
> 6 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 7b47e8f344cb..586ce67d63a9 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -341,6 +341,15 @@ enum bpf_arg_type {
> ARG_PTR_TO_STACK_OR_NULL, /* pointer to stack or NULL */
> ARG_PTR_TO_CONST_STR, /* pointer to a null terminated read-only string */
> ARG_PTR_TO_TIMER, /* pointer to bpf_timer */
> + ARG_PTR_TO_WRITABLE_MEM, /* pointer to valid memory. Compared to
> + * ARG_PTR_TO_MEM, this arg_type is not
> + * compatible with RDONLY memory. If the
> + * argument may be updated by the helper,
> + * use this type.
> + */
> + ARG_PTR_TO_WRITABLE_MEM_OR_NULL, /* pointer to memory or null, similar to
> + * ARG_PTR_TO_WRITABLE_MEM.
> + */
Instead of adding new types,
can we do something like this instead:
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index c8a78e830fca..5dbd2541aa86 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -68,7 +68,8 @@ struct bpf_reg_state {
u32 btf_id;
};
- u32 mem_size; /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
+ u32 rd_mem_size; /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
+ u32 wr_mem_size; /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
/* Max size from any of the above. */
struct {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c6616e325803..ad46169d422b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4374,7 +4374,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
return -EACCES;
}
err = check_mem_region_access(env, regno, off, size,
- reg->mem_size, false);
+ t == BPF_WRITE ? reg->wr_mem_size : reg->rd_mem_size, false);
if (!err && t == BPF_READ && value_regno >= 0)
mark_reg_unknown(env, regs, value_regno);
} else if (reg->type == PTR_TO_CTX) {
@@ -11511,7 +11511,8 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
goto err_put;
}
aux->btf_var.reg_type = PTR_TO_MEM;
- aux->btf_var.mem_size = tsize;
+ aux->btf_var.rd_mem_size = tsize;
+ aux->btf_var.wr_mem_size = 0;
} else {
aux->btf_var.reg_type = PTR_TO_BTF_ID;
aux->btf_var.btf = btf;
