Re: [RFC bpf-next v3 3/5] bpf: Prevent BPF programs from access the buffer pointed by user_optval.

Kui-Feng Lee
2023-08-17T18:12:42Z

On 8/16/23 18:17, Alexei Starovoitov wrote:
> On Tue, Aug 15, 2023 at 10:47:10AM -0700, thinker.li@gmail.com wrote:
>> From: Kui-Feng Lee <thinker.li@gmail.com>
>>
>> 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@gmail.com>
>> ---
>> 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;
>> + 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;
>
> This doesn't look correct.
> packet memory and user memory are non overlapping address spaces.
> There cannot be a packet memory that is also and user memory.

Got it! I will find a new type to replace this one.