Re: [PATCH bpf-next 1/4] bpf: Use __GFP_NOWARN for kvcalloc when attaching multiple uprobes

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Dec 12, 2023 at 11:44:34AM +0800, Hou Tao wrote:
> Hi,
> 
> On 12/12/2023 12:50 AM, Alexei Starovoitov wrote:
> > On Mon, Dec 11, 2023 at 3:27 AM Hou Tao <houtao@huaweicloud.com> wrote:
> >> From: Hou Tao <houtao1@huawei.com>
> >>
> >> An abnormally big cnt may be passed to link_create.uprobe_multi.cnt,
> >> and it will trigger the following warning in kvmalloc_node():
> >>
> >>         if (unlikely(size > INT_MAX)) {
> >>                 WARN_ON_ONCE(!(flags & __GFP_NOWARN));
> >>                 return NULL;
> >>         }
> >>
> >> Fix the warning by using __GFP_NOWARN when invoking kvzalloc() in
> >> bpf_uprobe_multi_link_attach().
> >>
> >> Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link")
> >> Reported-by: xingwei lee <xrivendell7@gmail.com>
> >> Closes: https://lore.kernel.org/bpf/CABOYnLwwJY=yFAGie59LFsUsBAgHfroVqbzZ5edAXbFE3YiNVA@mail.gmail.com
> >> Signed-off-by: Hou Tao <houtao1@huawei.com>
> >> ---
> >>  kernel/trace/bpf_trace.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> >> index 774cf476a892..07b9b5896d6c 100644
> >> --- a/kernel/trace/bpf_trace.c
> >> +++ b/kernel/trace/bpf_trace.c
> >> @@ -3378,7 +3378,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> >>         err = -ENOMEM;
> >>
> >>         link = kzalloc(sizeof(*link), GFP_KERNEL);
> >> -       uprobes = kvcalloc(cnt, sizeof(*uprobes), GFP_KERNEL);
> >> +       uprobes = kvcalloc(cnt, sizeof(*uprobes), GFP_KERNEL | __GFP_NOWARN);
> > __GFP_NOWARN will hide actual malloc failures.
> > Let's limit cnt instead. Both for k and u multi probes.
> 
> Do you mean there will be no warning messages when the malloc request
> can not be fulfilled, right ?  Because kvcalloc() will still return
> -ENOMEM when __GFP_NOWARN is used, so the userspace knows the malloc
> failed. And I also found out that __GFP_NOWARN only effect the
> invocation of vmalloc(), because kvmalloc_node() enable __GFP_NOWARN for
> kmalloc() by default when the passed size is greater than PAGE_SIZE.
> 
> I also though about fixing the problem by limitation, but I could not
> get good reference values for these limitations. For multiple kprobe,
> maybe the number of kallsyms can be used as an anchor (e.g, the number
> is 207617 on my local dev machine), how about using 
> __roundup_pow_of_two(207617 * 4) = 1 << 20 for multiple kprobes ? For
> multiple uprobes, maybe (1<<20) is also suitable.

I think available_filter_functions is more relevant, because kallsyms
has everything

on fedora kernel:
  # cat available_filter_functions | wc -l
  80177

anyway to be on the safe side with some other configs and possible
huge kernel modules the '1 << 20' looks good to me, also for uprobe
multi

jirka