Hi,
On 12/12/2023 5:54 PM, Jiri Olsa wrote:
> 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@xxxxxxxxxxxxxxx> wrote:
>>>> From: Hou Tao <houtao1@xxxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxx>
>>>> Closes: https://lore.kernel.org/bpf/CABOYnLwwJY=yFAGie59LFsUsBAgHfroVqbzZ5edAXbFE3YiNVA@xxxxxxxxxxxxxx
>>>> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
>>>> ---
>>>> 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
Agreed. Only functions in available_filter_functions could use kprobe.
>
> 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
Thanks. Will post v2 if Alexei is also fine with such limitations.
>
> jirka
