On 8/15/23 10:47 AM, thinker.li@xxxxxxxxx wrote:
From: Kui-Feng Lee <thinker.li@xxxxxxxxx>
Since the buffer pointed by ctx->user_optval is in user space, BPF programs
in kernel space should not access it directly. They should use
bpf_copy_from_user() and bpf_copy_to_user() to move data between user and
kernel space.
Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
---
kernel/bpf/cgroup.c | 16 +++++++++--
kernel/bpf/verifier.c | 66 +++++++++++++++++++++----------------------
2 files changed, 47 insertions(+), 35 deletions(-)
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index b977768a28e5..425094e071ba 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -2494,12 +2494,24 @@ static bool cg_sockopt_is_valid_access(int off, int size,
case offsetof(struct bpf_sockopt, optval):
if (size != sizeof(__u64))
return false;
- info->reg_type = PTR_TO_PACKET;
+ if (prog->aux->sleepable)
+ /* Prohibit access to the memory pointed by optval
+ * in sleepable programs.
+ */
+ info->reg_type = PTR_TO_PACKET | MEM_USER;
Is MEM_USER needed to call bpf_copy_from_user()?
Also, from looking at patch 4, the optval could be changed from user memory to
kernel memory during runtime. Is it useful to check MEM_USER during the verifier
load time?
How about just return false here to disallow sleepable prog to use ->optval and
->optval_end. Enforce sleepable prog to stay with the bpf_dynptr_read/write API
and avoid needing the optval + len > optval_end check in the sleepable bpf prog.
WDYT?
Regarding ->optlen, do you think the sleepable prog can stay with the
bpf_dynptr_size() and bpf_dynptr_adjust() such that no need to expose optlen to
the sleepable prog also.
+ else
+ info->reg_type = PTR_TO_PACKET;
break;
case offsetof(struct bpf_sockopt, optval_end):
if (size != sizeof(__u64))
return false;
- info->reg_type = PTR_TO_PACKET_END;
+ if (prog->aux->sleepable)
+ /* Prohibit access to the memory pointed by
+ * optval_end in sleepable programs.
+ */
+ info->reg_type = PTR_TO_PACKET_END | MEM_USER;
+ else
+ info->reg_type = PTR_TO_PACKET_END;
break;
