[PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions

[PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

When using regular expression matching with "kprobe multi", it scans all
the functions under "/proc/kallsyms" that can be matched. However, not all
of them can be traced by kprobe.multi. If any one of the functions fails
to be traced, it will result in the failure of all functions. The best
approach is to filter out the functions that cannot be traced to ensure
proper tracking of the functions.

But, the addition of these checks will frequently probe whether a function
complies with "available_filter_functions" and ensure that it has not been
filtered by kprobe's blacklist. As a result, it may take a longer time
during startup. The function implementation is referenced from BCC's
"kprobe_exists()"

Here is the test eBPF program [1].
[1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867

Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 tools/lib/bpf/libbpf.c | 47 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ad1ec893b41b..6a201267fa08 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10421,6 +10421,50 @@ struct kprobe_multi_resolve {
 	size_t cnt;
 };
 
+static bool filter_available_function(const char *name)
+{
+	char addr_range[256];
+	char sym_name[256];
+	FILE *f;
+	int ret;
+
+	f = fopen("/sys/kernel/debug/kprobes/blacklist", "r");
+	if (!f)
+		goto avail_filter;
+
+	while (true) {
+		ret = fscanf(f, "%s %s%*[^\n]\n", addr_range, sym_name);
+		if (ret == EOF && feof(f))
+			break;
+		if (ret != 2)
+			break;
+		if (!strcmp(name, sym_name)) {
+			fclose(f);
+			return false;
+		}
+	}
+	fclose(f);
+
+avail_filter:
+	f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
+	if (!f)
+		return true;
+
+	while (true) {
+		ret = fscanf(f, "%s%*[^\n]\n", sym_name);
+		if (ret == EOF && feof(f))
+			break;
+		if (ret != 1)
+			break;
+		if (!strcmp(name, sym_name)) {
+			fclose(f);
+			return true;
+		}
+	}
+	fclose(f);
+	return false;
+}
+
 static int
 resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
 			const char *sym_name, void *ctx)
@@ -10431,6 +10475,9 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
 	if (!glob_match(sym_name, res->pattern))
 		return 0;
 
+	if (!filter_available_function(sym_name))
+		return 0;
+
 	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
 				res->cnt + 1);
 	if (err)
-- 
2.25.1

Re: [PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions

[Jiri Olsa]

On Tue, May 23, 2023 at 09:25:47PM +0800, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
> 
> When using regular expression matching with "kprobe multi", it scans all
> the functions under "/proc/kallsyms" that can be matched. However, not all
> of them can be traced by kprobe.multi. If any one of the functions fails
> to be traced, it will result in the failure of all functions. The best
> approach is to filter out the functions that cannot be traced to ensure
> proper tracking of the functions.
> 
> But, the addition of these checks will frequently probe whether a function
> complies with "available_filter_functions" and ensure that it has not been
> filtered by kprobe's blacklist. As a result, it may take a longer time
> during startup. The function implementation is referenced from BCC's
> "kprobe_exists()"
> 
> Here is the test eBPF program [1].
> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
> 
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
>  tools/lib/bpf/libbpf.c | 47 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ad1ec893b41b..6a201267fa08 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10421,6 +10421,50 @@ struct kprobe_multi_resolve {
>  	size_t cnt;
>  };
>  
> +static bool filter_available_function(const char *name)
> +{
> +	char addr_range[256];
> +	char sym_name[256];
> +	FILE *f;
> +	int ret;
> +
> +	f = fopen("/sys/kernel/debug/kprobes/blacklist", "r");
> +	if (!f)
> +		goto avail_filter;
> +
> +	while (true) {
> +		ret = fscanf(f, "%s %s%*[^\n]\n", addr_range, sym_name);
> +		if (ret == EOF && feof(f))
> +			break;
> +		if (ret != 2)
> +			break;
> +		if (!strcmp(name, sym_name)) {
> +			fclose(f);
> +			return false;
> +		}
> +	}
> +	fclose(f);

so available_filter_functions already contains all traceable symbols
for kprobe_multi/fprobe

kprobes/blacklist is kprobe specific and does not apply to fprobe,
is there a crash when attaching function from kprobes/blacklist ?

> +
> +avail_filter:
> +	f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> +	if (!f)
> +		return true;
> +
> +	while (true) {
> +		ret = fscanf(f, "%s%*[^\n]\n", sym_name);
> +		if (ret == EOF && feof(f))
> +			break;
> +		if (ret != 1)
> +			break;
> +		if (!strcmp(name, sym_name)) {
> +			fclose(f);
> +			return true;
> +		}
> +	}
> +	fclose(f);
> +	return false;
> +}
> +
>  static int
>  resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>  			const char *sym_name, void *ctx)
> @@ -10431,6 +10475,9 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>  	if (!glob_match(sym_name, res->pattern))
>  		return 0;
>  
> +	if (!filter_available_function(sym_name))
> +		return 0;

I think it'd be better to parse available_filter_functions directly
for kprobe_multi instead of filtering out kallsyms entries

we could add libbpf_available_filter_functions_parse function with
similar callback to go over available_filter_functions file


jirka

> +
>  	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
>  				res->cnt + 1);
>  	if (err)
> -- 
> 2.25.1
> 
> 

Re: [PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions

[Andrii Nakryiko]

On Tue, May 23, 2023 at 9:17 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, May 23, 2023 at 09:25:47PM +0800, Jackie Liu wrote:
> > From: Jackie Liu <liuyun01@kylinos.cn>
> >
> > When using regular expression matching with "kprobe multi", it scans all
> > the functions under "/proc/kallsyms" that can be matched. However, not all
> > of them can be traced by kprobe.multi. If any one of the functions fails
> > to be traced, it will result in the failure of all functions. The best
> > approach is to filter out the functions that cannot be traced to ensure
> > proper tracking of the functions.
> >
> > But, the addition of these checks will frequently probe whether a function
> > complies with "available_filter_functions" and ensure that it has not been
> > filtered by kprobe's blacklist. As a result, it may take a longer time
> > during startup. The function implementation is referenced from BCC's
> > "kprobe_exists()"
> >
> > Here is the test eBPF program [1].
> > [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
> >
> > Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> > ---
> >  tools/lib/bpf/libbpf.c | 47 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index ad1ec893b41b..6a201267fa08 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -10421,6 +10421,50 @@ struct kprobe_multi_resolve {
> >       size_t cnt;
> >  };
> >
> > +static bool filter_available_function(const char *name)
> > +{
> > +     char addr_range[256];
> > +     char sym_name[256];
> > +     FILE *f;
> > +     int ret;
> > +
> > +     f = fopen("/sys/kernel/debug/kprobes/blacklist", "r");
> > +     if (!f)
> > +             goto avail_filter;
> > +
> > +     while (true) {
> > +             ret = fscanf(f, "%s %s%*[^\n]\n", addr_range, sym_name);
> > +             if (ret == EOF && feof(f))
> > +                     break;
> > +             if (ret != 2)
> > +                     break;
> > +             if (!strcmp(name, sym_name)) {
> > +                     fclose(f);
> > +                     return false;
> > +             }
> > +     }
> > +     fclose(f);
>
> so available_filter_functions already contains all traceable symbols
> for kprobe_multi/fprobe
>
> kprobes/blacklist is kprobe specific and does not apply to fprobe,
> is there a crash when attaching function from kprobes/blacklist ?
>
> > +
> > +avail_filter:
> > +     f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> > +     if (!f)
> > +             return true;
> > +
> > +     while (true) {
> > +             ret = fscanf(f, "%s%*[^\n]\n", sym_name);
> > +             if (ret == EOF && feof(f))
> > +                     break;
> > +             if (ret != 1)
> > +                     break;
> > +             if (!strcmp(name, sym_name)) {
> > +                     fclose(f);
> > +                     return true;
> > +             }
> > +     }
> > +     fclose(f);
> > +     return false;
> > +}
> > +
> >  static int
> >  resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> >                       const char *sym_name, void *ctx)
> > @@ -10431,6 +10475,9 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> >       if (!glob_match(sym_name, res->pattern))
> >               return 0;
> >
> > +     if (!filter_available_function(sym_name))
> > +             return 0;
>
> I think it'd be better to parse available_filter_functions directly
> for kprobe_multi instead of filtering out kallsyms entries

yep, available_filter_functions should be cheaper to parse than
kallsyms. We can probably fallback to kallsyms still, if
available_filter_functions are missing.

Furthermore, me and Steven chatted at lsfmm2023 about having an
available_filter_functions-like file with kernel function addresses
(not just names), which would speed up attachment as well. It could be
useful in some other scenarios as well (e.g., I think retsnoop has to
join kallsyms and available_filter_functions). I think it's still a
good idea to add this new file, given kernel has all this information
readily available anyways.


>
> we could add libbpf_available_filter_functions_parse function with
> similar callback to go over available_filter_functions file

or iterator ;)

but either way, current approach will do linear scan for each matched
function, which is hugely inefficient, so definitely a no go

>
>
> jirka
>
> > +
> >       err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
> >                               res->cnt + 1);
> >       if (err)
> > --
> > 2.25.1
> >
> >

Re: [PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions

[Jiri Olsa]

On Tue, May 23, 2023 at 11:22:46AM -0700, Andrii Nakryiko wrote:

SNIP

> > > +avail_filter:
> > > +     f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> > > +     if (!f)
> > > +             return true;
> > > +
> > > +     while (true) {
> > > +             ret = fscanf(f, "%s%*[^\n]\n", sym_name);
> > > +             if (ret == EOF && feof(f))
> > > +                     break;
> > > +             if (ret != 1)
> > > +                     break;
> > > +             if (!strcmp(name, sym_name)) {
> > > +                     fclose(f);
> > > +                     return true;
> > > +             }
> > > +     }
> > > +     fclose(f);
> > > +     return false;
> > > +}
> > > +
> > >  static int
> > >  resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> > >                       const char *sym_name, void *ctx)
> > > @@ -10431,6 +10475,9 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> > >       if (!glob_match(sym_name, res->pattern))
> > >               return 0;
> > >
> > > +     if (!filter_available_function(sym_name))
> > > +             return 0;
> >
> > I think it'd be better to parse available_filter_functions directly
> > for kprobe_multi instead of filtering out kallsyms entries
> 
> yep, available_filter_functions should be cheaper to parse than
> kallsyms. We can probably fallback to kallsyms still, if
> available_filter_functions are missing.
> 
> Furthermore, me and Steven chatted at lsfmm2023 about having an
> available_filter_functions-like file with kernel function addresses
> (not just names), which would speed up attachment as well. It could be
> useful in some other scenarios as well (e.g., I think retsnoop has to
> join kallsyms and available_filter_functions). I think it's still a
> good idea to add this new file, given kernel has all this information
> readily available anyways.

yes, would be useful for this, and likely in other places

jirka

> 
> 
> >
> > we could add libbpf_available_filter_functions_parse function with
> > similar callback to go over available_filter_functions file
> 
> or iterator ;)
> 
> but either way, current approach will do linear scan for each matched
> function, which is hugely inefficient, so definitely a no go
> 
> >
> >
> > jirka
> >
> > > +
> > >       err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
> > >                               res->cnt + 1);
> > >       if (err)
> > > --
> > > 2.25.1
> > >
> > >

Re: [PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions

[Jackie Liu]

Hi Jiri.

在 2023/5/24 00:17, Jiri Olsa 写道:
> On Tue, May 23, 2023 at 09:25:47PM +0800, Jackie Liu wrote:
>> From: Jackie Liu <liuyun01@kylinos.cn>
>>
>> When using regular expression matching with "kprobe multi", it scans all
>> the functions under "/proc/kallsyms" that can be matched. However, not all
>> of them can be traced by kprobe.multi. If any one of the functions fails
>> to be traced, it will result in the failure of all functions. The best
>> approach is to filter out the functions that cannot be traced to ensure
>> proper tracking of the functions.
>>
>> But, the addition of these checks will frequently probe whether a function
>> complies with "available_filter_functions" and ensure that it has not been
>> filtered by kprobe's blacklist. As a result, it may take a longer time
>> during startup. The function implementation is referenced from BCC's
>> "kprobe_exists()"
>>
>> Here is the test eBPF program [1].
>> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
>>
>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>> ---
>>   tools/lib/bpf/libbpf.c | 47 ++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 47 insertions(+)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index ad1ec893b41b..6a201267fa08 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -10421,6 +10421,50 @@ struct kprobe_multi_resolve {
>>   	size_t cnt;
>>   };
>>   
>> +static bool filter_available_function(const char *name)
>> +{
>> +	char addr_range[256];
>> +	char sym_name[256];
>> +	FILE *f;
>> +	int ret;
>> +
>> +	f = fopen("/sys/kernel/debug/kprobes/blacklist", "r");
>> +	if (!f)
>> +		goto avail_filter;
>> +
>> +	while (true) {
>> +		ret = fscanf(f, "%s %s%*[^\n]\n", addr_range, sym_name);
>> +		if (ret == EOF && feof(f))
>> +			break;
>> +		if (ret != 2)
>> +			break;
>> +		if (!strcmp(name, sym_name)) {
>> +			fclose(f);
>> +			return false;
>> +		}
>> +	}
>> +	fclose(f);
> 
> so available_filter_functions already contains all traceable symbols
> for kprobe_multi/fprobe
> 
> kprobes/blacklist is kprobe specific and does not apply to fprobe,
> is there a crash when attaching function from kprobes/blacklist ?

No, I haven't got crash before, Simply because BCC's kprobe_exists has
implemented it so I added this, Yes, I also don't think 
kprobes/blacklist will affect FPROBE, so I will remove it.

> 
>> +
>> +avail_filter:
>> +	f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
>> +	if (!f)
>> +		return true;
>> +
>> +	while (true) {
>> +		ret = fscanf(f, "%s%*[^\n]\n", sym_name);
>> +		if (ret == EOF && feof(f))
>> +			break;
>> +		if (ret != 1)
>> +			break;
>> +		if (!strcmp(name, sym_name)) {
>> +			fclose(f);
>> +			return true;
>> +		}
>> +	}
>> +	fclose(f);
>> +	return false;
>> +}
>> +
>>   static int
>>   resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>>   			const char *sym_name, void *ctx)
>> @@ -10431,6 +10475,9 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>>   	if (!glob_match(sym_name, res->pattern))
>>   		return 0;
>>   
>> +	if (!filter_available_function(sym_name))
>> +		return 0;
> 
> I think it'd be better to parse available_filter_functions directly
> for kprobe_multi instead of filtering out kallsyms entries
> 
> we could add libbpf_available_filter_functions_parse function with
> similar callback to go over available_filter_functions file
> 

Sure, if available_filter_functions not found, fallback to /proc/kallsyms.

-- 
BR, Jackie Liu

> 
> jirka
> 
>> +
>>   	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
>>   				res->cnt + 1);
>>   	if (err)
>> -- 
>> 2.25.1
>>
>>

Re: [PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions

[Jackie Liu]

Hi Jiri.

在 2023/5/24 09:03, Jackie Liu 写道:
> Hi Jiri.
> 
> 在 2023/5/24 00:17, Jiri Olsa 写道:
>> On Tue, May 23, 2023 at 09:25:47PM +0800, Jackie Liu wrote:
>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>
>>> When using regular expression matching with "kprobe multi", it scans all
>>> the functions under "/proc/kallsyms" that can be matched. However, 
>>> not all
>>> of them can be traced by kprobe.multi. If any one of the functions fails
>>> to be traced, it will result in the failure of all functions. The best
>>> approach is to filter out the functions that cannot be traced to ensure
>>> proper tracking of the functions.
>>>
>>> But, the addition of these checks will frequently probe whether a 
>>> function
>>> complies with "available_filter_functions" and ensure that it has not 
>>> been
>>> filtered by kprobe's blacklist. As a result, it may take a longer time
>>> during startup. The function implementation is referenced from BCC's
>>> "kprobe_exists()"
>>>
>>> Here is the test eBPF program [1].
>>> [1] 
>>> https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
>>>
>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>> ---
>>>   tools/lib/bpf/libbpf.c | 47 ++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 47 insertions(+)
>>>
>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>> index ad1ec893b41b..6a201267fa08 100644
>>> --- a/tools/lib/bpf/libbpf.c
>>> +++ b/tools/lib/bpf/libbpf.c
>>> @@ -10421,6 +10421,50 @@ struct kprobe_multi_resolve {
>>>       size_t cnt;
>>>   };
>>> +static bool filter_available_function(const char *name)
>>> +{
>>> +    char addr_range[256];
>>> +    char sym_name[256];
>>> +    FILE *f;
>>> +    int ret;
>>> +
>>> +    f = fopen("/sys/kernel/debug/kprobes/blacklist", "r");
>>> +    if (!f)
>>> +        goto avail_filter;
>>> +
>>> +    while (true) {
>>> +        ret = fscanf(f, "%s %s%*[^\n]\n", addr_range, sym_name);
>>> +        if (ret == EOF && feof(f))
>>> +            break;
>>> +        if (ret != 2)
>>> +            break;
>>> +        if (!strcmp(name, sym_name)) {
>>> +            fclose(f);
>>> +            return false;
>>> +        }
>>> +    }
>>> +    fclose(f);
>>
>> so available_filter_functions already contains all traceable symbols
>> for kprobe_multi/fprobe
>>
>> kprobes/blacklist is kprobe specific and does not apply to fprobe,
>> is there a crash when attaching function from kprobes/blacklist ?
> 
> No, I haven't got crash before, Simply because BCC's kprobe_exists has
> implemented it so I added this, Yes, I also don't think 
> kprobes/blacklist will affect FPROBE, so I will remove it.
> 
>>
>>> +
>>> +avail_filter:
>>> +    f = 
>>> fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
>>> +    if (!f)
>>> +        return true;
>>> +
>>> +    while (true) {
>>> +        ret = fscanf(f, "%s%*[^\n]\n", sym_name);
>>> +        if (ret == EOF && feof(f))
>>> +            break;
>>> +        if (ret != 1)
>>> +            break;
>>> +        if (!strcmp(name, sym_name)) {
>>> +            fclose(f);
>>> +            return true;
>>> +        }
>>> +    }
>>> +    fclose(f);
>>> +    return false;
>>> +}
>>> +
>>>   static int
>>>   resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>>>               const char *sym_name, void *ctx)
>>> @@ -10431,6 +10475,9 @@ resolve_kprobe_multi_cb(unsigned long long 
>>> sym_addr, char sym_type,
>>>       if (!glob_match(sym_name, res->pattern))
>>>           return 0;
>>> +    if (!filter_available_function(sym_name))
>>> +        return 0;
>>
>> I think it'd be better to parse available_filter_functions directly
>> for kprobe_multi instead of filtering out kallsyms entries
>>
>> we could add libbpf_available_filter_functions_parse function with
>> similar callback to go over available_filter_functions file
>>
> 
> Sure, if available_filter_functions not found, fallback to /proc/kallsyms.
> 

Um.

It is difficult to judge available_filter_functions directly, because we
not only need the function name, but also obtain its address and other
information, but we can indeed obtain the function set from
available_filter_functions first, and then obtain the function address
from /proc/kallsyms. which will be slightly faster than reading
available_filter_functions later, because if this function does not
exist in available_filter_functions, it will take a long time to read
the entire file.

Of course, it would be better if the kernel directly provided an
available_filter_functions -like file containing function address
information.

-- 
Jackie Liu

Re: [PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions

[Jiri Olsa]

On Wed, May 24, 2023 at 09:19:48AM +0800, Jackie Liu wrote:
> Hi Jiri.
> 
> 在 2023/5/24 09:03, Jackie Liu 写道:
> > Hi Jiri.
> > 
> > 在 2023/5/24 00:17, Jiri Olsa 写道:
> > > On Tue, May 23, 2023 at 09:25:47PM +0800, Jackie Liu wrote:
> > > > From: Jackie Liu <liuyun01@kylinos.cn>
> > > > 
> > > > When using regular expression matching with "kprobe multi", it scans all
> > > > the functions under "/proc/kallsyms" that can be matched.
> > > > However, not all
> > > > of them can be traced by kprobe.multi. If any one of the functions fails
> > > > to be traced, it will result in the failure of all functions. The best
> > > > approach is to filter out the functions that cannot be traced to ensure
> > > > proper tracking of the functions.
> > > > 
> > > > But, the addition of these checks will frequently probe whether
> > > > a function
> > > > complies with "available_filter_functions" and ensure that it
> > > > has not been
> > > > filtered by kprobe's blacklist. As a result, it may take a longer time
> > > > during startup. The function implementation is referenced from BCC's
> > > > "kprobe_exists()"
> > > > 
> > > > Here is the test eBPF program [1].
> > > > [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
> > > > 
> > > > Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> > > > ---
> > > >   tools/lib/bpf/libbpf.c | 47 ++++++++++++++++++++++++++++++++++++++++++
> > > >   1 file changed, 47 insertions(+)
> > > > 
> > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > > index ad1ec893b41b..6a201267fa08 100644
> > > > --- a/tools/lib/bpf/libbpf.c
> > > > +++ b/tools/lib/bpf/libbpf.c
> > > > @@ -10421,6 +10421,50 @@ struct kprobe_multi_resolve {
> > > >       size_t cnt;
> > > >   };
> > > > +static bool filter_available_function(const char *name)
> > > > +{
> > > > +    char addr_range[256];
> > > > +    char sym_name[256];
> > > > +    FILE *f;
> > > > +    int ret;
> > > > +
> > > > +    f = fopen("/sys/kernel/debug/kprobes/blacklist", "r");
> > > > +    if (!f)
> > > > +        goto avail_filter;
> > > > +
> > > > +    while (true) {
> > > > +        ret = fscanf(f, "%s %s%*[^\n]\n", addr_range, sym_name);
> > > > +        if (ret == EOF && feof(f))
> > > > +            break;
> > > > +        if (ret != 2)
> > > > +            break;
> > > > +        if (!strcmp(name, sym_name)) {
> > > > +            fclose(f);
> > > > +            return false;
> > > > +        }
> > > > +    }
> > > > +    fclose(f);
> > > 
> > > so available_filter_functions already contains all traceable symbols
> > > for kprobe_multi/fprobe
> > > 
> > > kprobes/blacklist is kprobe specific and does not apply to fprobe,
> > > is there a crash when attaching function from kprobes/blacklist ?
> > 
> > No, I haven't got crash before, Simply because BCC's kprobe_exists has
> > implemented it so I added this, Yes, I also don't think
> > kprobes/blacklist will affect FPROBE, so I will remove it.
> > 
> > > 
> > > > +
> > > > +avail_filter:
> > > > +    f =
> > > > fopen("/sys/kernel/debug/tracing/available_filter_functions",
> > > > "r");
> > > > +    if (!f)
> > > > +        return true;
> > > > +
> > > > +    while (true) {
> > > > +        ret = fscanf(f, "%s%*[^\n]\n", sym_name);
> > > > +        if (ret == EOF && feof(f))
> > > > +            break;
> > > > +        if (ret != 1)
> > > > +            break;
> > > > +        if (!strcmp(name, sym_name)) {
> > > > +            fclose(f);
> > > > +            return true;
> > > > +        }
> > > > +    }
> > > > +    fclose(f);
> > > > +    return false;
> > > > +}
> > > > +
> > > >   static int
> > > >   resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> > > >               const char *sym_name, void *ctx)
> > > > @@ -10431,6 +10475,9 @@ resolve_kprobe_multi_cb(unsigned long
> > > > long sym_addr, char sym_type,
> > > >       if (!glob_match(sym_name, res->pattern))
> > > >           return 0;
> > > > +    if (!filter_available_function(sym_name))
> > > > +        return 0;
> > > 
> > > I think it'd be better to parse available_filter_functions directly
> > > for kprobe_multi instead of filtering out kallsyms entries
> > > 
> > > we could add libbpf_available_filter_functions_parse function with
> > > similar callback to go over available_filter_functions file
> > > 
> > 
> > Sure, if available_filter_functions not found, fallback to /proc/kallsyms.
> > 
> 
> Um.
> 
> It is difficult to judge available_filter_functions directly, because we
> not only need the function name, but also obtain its address and other
> information, but we can indeed obtain the function set from
> available_filter_functions first, and then obtain the function address
> from /proc/kallsyms. which will be slightly faster than reading
> available_filter_functions later, because if this function does not
> exist in available_filter_functions, it will take a long time to read
> the entire file.
> 
> Of course, it would be better if the kernel directly provided an
> available_filter_functions -like file containing function address
> information.

you don't need to resolve symbols, you can pass just array of symbols
to create kprobe_multi link and they will get resolved in kernel:

	struct bpf_link_create_opts {

			struct {
				__u32 flags;
				__u32 cnt;
		--->		const char **syms;
				const unsigned long *addrs;
				const __u64 *cookies;
			} kprobe_multi;
	}

I resolved the symbols in bpf_program__attach_kprobe_multi_opts mostly
because the address was available right away when parsing kallsyms,
but passing just symbols for pattern is fine

jirka

Re: [PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions

[Jackie Liu]

Hi Jiri.

在 2023/5/24 14:47, Jiri Olsa 写道:
> On Wed, May 24, 2023 at 09:19:48AM +0800, Jackie Liu wrote:
>> Hi Jiri.
>>
>> 在 2023/5/24 09:03, Jackie Liu 写道:
>>> Hi Jiri.
>>>
>>> 在 2023/5/24 00:17, Jiri Olsa 写道:
>>>> On Tue, May 23, 2023 at 09:25:47PM +0800, Jackie Liu wrote:
>>>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>>>
>>>>> When using regular expression matching with "kprobe multi", it scans all
>>>>> the functions under "/proc/kallsyms" that can be matched.
>>>>> However, not all
>>>>> of them can be traced by kprobe.multi. If any one of the functions fails
>>>>> to be traced, it will result in the failure of all functions. The best
>>>>> approach is to filter out the functions that cannot be traced to ensure
>>>>> proper tracking of the functions.
>>>>>
>>>>> But, the addition of these checks will frequently probe whether
>>>>> a function
>>>>> complies with "available_filter_functions" and ensure that it
>>>>> has not been
>>>>> filtered by kprobe's blacklist. As a result, it may take a longer time
>>>>> during startup. The function implementation is referenced from BCC's
>>>>> "kprobe_exists()"
>>>>>
>>>>> Here is the test eBPF program [1].
>>>>> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
>>>>>
>>>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>>>> ---
>>>>>    tools/lib/bpf/libbpf.c | 47 ++++++++++++++++++++++++++++++++++++++++++
>>>>>    1 file changed, 47 insertions(+)
>>>>>
>>>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>>>> index ad1ec893b41b..6a201267fa08 100644
>>>>> --- a/tools/lib/bpf/libbpf.c
>>>>> +++ b/tools/lib/bpf/libbpf.c
>>>>> @@ -10421,6 +10421,50 @@ struct kprobe_multi_resolve {
>>>>>        size_t cnt;
>>>>>    };
>>>>> +static bool filter_available_function(const char *name)
>>>>> +{
>>>>> +    char addr_range[256];
>>>>> +    char sym_name[256];
>>>>> +    FILE *f;
>>>>> +    int ret;
>>>>> +
>>>>> +    f = fopen("/sys/kernel/debug/kprobes/blacklist", "r");
>>>>> +    if (!f)
>>>>> +        goto avail_filter;
>>>>> +
>>>>> +    while (true) {
>>>>> +        ret = fscanf(f, "%s %s%*[^\n]\n", addr_range, sym_name);
>>>>> +        if (ret == EOF && feof(f))
>>>>> +            break;
>>>>> +        if (ret != 2)
>>>>> +            break;
>>>>> +        if (!strcmp(name, sym_name)) {
>>>>> +            fclose(f);
>>>>> +            return false;
>>>>> +        }
>>>>> +    }
>>>>> +    fclose(f);
>>>>
>>>> so available_filter_functions already contains all traceable symbols
>>>> for kprobe_multi/fprobe
>>>>
>>>> kprobes/blacklist is kprobe specific and does not apply to fprobe,
>>>> is there a crash when attaching function from kprobes/blacklist ?
>>>
>>> No, I haven't got crash before, Simply because BCC's kprobe_exists has
>>> implemented it so I added this, Yes, I also don't think
>>> kprobes/blacklist will affect FPROBE, so I will remove it.
>>>
>>>>
>>>>> +
>>>>> +avail_filter:
>>>>> +    f =
>>>>> fopen("/sys/kernel/debug/tracing/available_filter_functions",
>>>>> "r");
>>>>> +    if (!f)
>>>>> +        return true;
>>>>> +
>>>>> +    while (true) {
>>>>> +        ret = fscanf(f, "%s%*[^\n]\n", sym_name);
>>>>> +        if (ret == EOF && feof(f))
>>>>> +            break;
>>>>> +        if (ret != 1)
>>>>> +            break;
>>>>> +        if (!strcmp(name, sym_name)) {
>>>>> +            fclose(f);
>>>>> +            return true;
>>>>> +        }
>>>>> +    }
>>>>> +    fclose(f);
>>>>> +    return false;
>>>>> +}
>>>>> +
>>>>>    static int
>>>>>    resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>>>>>                const char *sym_name, void *ctx)
>>>>> @@ -10431,6 +10475,9 @@ resolve_kprobe_multi_cb(unsigned long
>>>>> long sym_addr, char sym_type,
>>>>>        if (!glob_match(sym_name, res->pattern))
>>>>>            return 0;
>>>>> +    if (!filter_available_function(sym_name))
>>>>> +        return 0;
>>>>
>>>> I think it'd be better to parse available_filter_functions directly
>>>> for kprobe_multi instead of filtering out kallsyms entries
>>>>
>>>> we could add libbpf_available_filter_functions_parse function with
>>>> similar callback to go over available_filter_functions file
>>>>
>>>
>>> Sure, if available_filter_functions not found, fallback to /proc/kallsyms.
>>>
>>
>> Um.
>>
>> It is difficult to judge available_filter_functions directly, because we
>> not only need the function name, but also obtain its address and other
>> information, but we can indeed obtain the function set from
>> available_filter_functions first, and then obtain the function address
>> from /proc/kallsyms. which will be slightly faster than reading
>> available_filter_functions later, because if this function does not
>> exist in available_filter_functions, it will take a long time to read
>> the entire file.
>>
>> Of course, it would be better if the kernel directly provided an
>> available_filter_functions -like file containing function address
>> information.
> 
> you don't need to resolve symbols, you can pass just array of symbols
> to create kprobe_multi link and they will get resolved in kernel:
> 
> 	struct bpf_link_create_opts {
> 
> 			struct {
> 				__u32 flags;
> 				__u32 cnt;
> 		--->		const char **syms;
> 				const unsigned long *addrs;
> 				const __u64 *cookies;
> 			} kprobe_multi;
> 	}
> 
> I resolved the symbols in bpf_program__attach_kprobe_multi_opts mostly
> because the address was available right away when parsing kallsyms,
> but passing just symbols for pattern is fine

I see, let me try. Thanks.

-- 
Jackie Liu

> 
> jirka

[PATCH v3] libbpf: kprobe.multi: Filter with available_filter_functions

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

When using regular expression matching with "kprobe multi", it scans all
the functions under "/proc/kallsyms" that can be matched. However, not all
of them can be traced by kprobe.multi. If any one of the functions fails
to be traced, it will result in the failure of all functions. The best
approach is to filter out the functions that cannot be traced to ensure
proper tracking of the functions.

Use available_filter_functions check first, if failed, fallback to
kallsyms.

Here is the test eBPF program [1].
[1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867

Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 v1: 0.27s user 5.09s system 99% cpu 5.392 total
 v2: 0.37s user 1.54s system 98% cpu 1.947 total
 v3: 0.10s user 0.98s system 97% cpu 1.107 total

 I saw that reading available_filter_functions takes 0.98s and kallsyms only
 takes 0.12s.  There is a big difference in performance between them.

 tools/lib/bpf/libbpf.c          | 80 ++++++++++++++++++++++++++++++---
 tools/lib/bpf/libbpf_internal.h |  4 +-
 2 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ad1ec893b41b..f3e3c92bdf8a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
 struct kprobe_multi_resolve {
 	const char *pattern;
 	unsigned long *addrs;
+	const char **syms;
 	size_t cap;
 	size_t cnt;
 };
 
 static int
-resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
-			const char *sym_name, void *ctx)
+kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
+				 const char *sym_name, void *ctx)
 {
 	struct kprobe_multi_resolve *res = ctx;
 	int err;
@@ -10440,6 +10441,69 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
 	return 0;
 }
 
+static int resolve_kprobe_multi_cb(const char *sym_name, void *ctx)
+{
+	struct kprobe_multi_resolve *res = ctx;
+	int err;
+
+	if (!glob_match(sym_name, res->pattern))
+		return 0;
+
+	err = libbpf_ensure_mem((void **) &res->syms, &res->cap, sizeof(const char *),
+				res->cnt + 1);
+	if (err)
+		return err;
+
+	res->syms[res->cnt++] = strdup(sym_name);
+	return 0;
+}
+
+int libbpf_available_filter_functions_parse(available_filter_functions_cb_t cb,
+					    void *ctx)
+{
+	char sym_name[256];
+	FILE *f;
+	int ret, err = 0;
+
+	f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
+	if (!f) {
+		pr_warn("failed to open available_filter_functions, fallback to /proc/kallsyms.\n");
+		goto fallback;
+	}
+
+	while (true) {
+		ret = fscanf(f, "%s%*[^\n]\n", sym_name);
+		if (ret == EOF && feof(f))
+			break;
+		if (ret != 1) {
+			pr_warn("failed to read available_filter_functions entry: %d\n",
+				ret);
+			break;
+		}
+
+		err = cb(sym_name, ctx);
+		if (err)
+			break;
+	}
+
+	fclose(f);
+	return err;
+
+fallback:
+	return libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb, ctx);
+}
+
+static void kprobe_multi_resolve_resource_free(struct kprobe_multi_resolve *res)
+{
+	if (res->syms) {
+		while (res->cnt)
+			free((char *)res->syms[--res->cnt]);
+		free(res->syms);
+	} else {
+		free(res->addrs);
+	}
+}
+
 struct bpf_link *
 bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 				      const char *pattern,
@@ -10476,14 +10540,18 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 		return libbpf_err_ptr(-EINVAL);
 
 	if (pattern) {
-		err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
+		err = libbpf_available_filter_functions_parse(resolve_kprobe_multi_cb,
+							      &res);
 		if (err)
 			goto error;
 		if (!res.cnt) {
 			err = -ENOENT;
 			goto error;
 		}
-		addrs = res.addrs;
+		if (res.syms)
+			syms = res.syms;
+		else
+			addrs = res.addrs;
 		cnt = res.cnt;
 	}
 
@@ -10511,12 +10579,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 		goto error;
 	}
 	link->fd = link_fd;
-	free(res.addrs);
+	kprobe_multi_resolve_resource_free(&res);
 	return link;
 
 error:
 	free(link);
-	free(res.addrs);
+	kprobe_multi_resolve_resource_free(&res);
 	return libbpf_err_ptr(err);
 }
 
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index e4d05662a96c..fdf6b464481f 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -481,8 +481,10 @@ __s32 btf__find_by_name_kind_own(const struct btf *btf, const char *type_name,
 
 typedef int (*kallsyms_cb_t)(unsigned long long sym_addr, char sym_type,
 			     const char *sym_name, void *ctx);
-
 int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *arg);
+typedef int (*available_filter_functions_cb_t)(const char *sym_name, void *ctx);
+int libbpf_available_filter_functions_parse(available_filter_functions_cb_t cb,
+					    void *arg);
 
 /* handle direct returned errors */
 static inline int libbpf_err(int ret)
-- 
2.25.1

Re: [PATCH v3] libbpf: kprobe.multi: Filter with available_filter_functions

[Jiri Olsa]

On Wed, May 24, 2023 at 04:41:54PM +0800, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
> 
> When using regular expression matching with "kprobe multi", it scans all
> the functions under "/proc/kallsyms" that can be matched. However, not all
> of them can be traced by kprobe.multi. If any one of the functions fails
> to be traced, it will result in the failure of all functions. The best
> approach is to filter out the functions that cannot be traced to ensure
> proper tracking of the functions.
> 
> Use available_filter_functions check first, if failed, fallback to
> kallsyms.
> 
> Here is the test eBPF program [1].
> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
> 
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
>  v1: 0.27s user 5.09s system 99% cpu 5.392 total
>  v2: 0.37s user 1.54s system 98% cpu 1.947 total
>  v3: 0.10s user 0.98s system 97% cpu 1.107 total
> 
>  I saw that reading available_filter_functions takes 0.98s and kallsyms only
>  takes 0.12s.  There is a big difference in performance between them.
> 
>  tools/lib/bpf/libbpf.c          | 80 ++++++++++++++++++++++++++++++---
>  tools/lib/bpf/libbpf_internal.h |  4 +-
>  2 files changed, 77 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ad1ec893b41b..f3e3c92bdf8a 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
>  struct kprobe_multi_resolve {
>  	const char *pattern;
>  	unsigned long *addrs;
> +	const char **syms;
>  	size_t cap;
>  	size_t cnt;
>  };
>  
>  static int
> -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> -			const char *sym_name, void *ctx)
> +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> +				 const char *sym_name, void *ctx)
>  {
>  	struct kprobe_multi_resolve *res = ctx;
>  	int err;
> @@ -10440,6 +10441,69 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>  	return 0;
>  }
>  
> +static int resolve_kprobe_multi_cb(const char *sym_name, void *ctx)
> +{
> +	struct kprobe_multi_resolve *res = ctx;
> +	int err;
> +
> +	if (!glob_match(sym_name, res->pattern))
> +		return 0;
> +
> +	err = libbpf_ensure_mem((void **) &res->syms, &res->cap, sizeof(const char *),
> +				res->cnt + 1);
> +	if (err)
> +		return err;
> +
> +	res->syms[res->cnt++] = strdup(sym_name);

we need to check the strdup return value

> +	return 0;
> +}
> +
> +int libbpf_available_filter_functions_parse(available_filter_functions_cb_t cb,
> +					    void *ctx)

might be too long, maybe we could just use 'ftrace' instead, so we'd have:

   libbpf_ftrace_parse
   libbpf_kallsyms_parse

that might be too terse, but AFAIK we don't parse any other ftrace file

> +{
> +	char sym_name[256];
> +	FILE *f;
> +	int ret, err = 0;
> +
> +	f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> +	if (!f) {
> +		pr_warn("failed to open available_filter_functions, fallback to /proc/kallsyms.\n");
> +		goto fallback;
> +	}
> +
> +	while (true) {
> +		ret = fscanf(f, "%s%*[^\n]\n", sym_name);
> +		if (ret == EOF && feof(f))
> +			break;
> +		if (ret != 1) {
> +			pr_warn("failed to read available_filter_functions entry: %d\n",
> +				ret);
> +			break;
> +		}
> +
> +		err = cb(sym_name, ctx);
> +		if (err)
> +			break;
> +	}
> +
> +	fclose(f);
> +	return err;
> +
> +fallback:
> +	return libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb, ctx);

let's do the kallsyms fallback in bpf_program__attach_kprobe_multi_opts,
something like:

	err = libbpf_available_filter_functions_parse(resolve_kprobe_multi_cb, &res);
	if (err)
		err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb, &res);
	if (err)
		goto error;


> +}
> +
> +static void kprobe_multi_resolve_resource_free(struct kprobe_multi_resolve *res)

s/kprobe_multi_resolve_resource_free/kprobe_multi_resolve_free/

> +{
> +	if (res->syms) {
> +		while (res->cnt)
> +			free((char *)res->syms[--res->cnt]);
> +		free(res->syms);
> +	} else {
> +		free(res->addrs);
> +	}
> +}
> +
>  struct bpf_link *
>  bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>  				      const char *pattern,
> @@ -10476,14 +10540,18 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>  		return libbpf_err_ptr(-EINVAL);
>  
>  	if (pattern) {
> -		err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
> +		err = libbpf_available_filter_functions_parse(resolve_kprobe_multi_cb,
> +							      &res);
>  		if (err)
>  			goto error;
>  		if (!res.cnt) {
>  			err = -ENOENT;
>  			goto error;
>  		}
> -		addrs = res.addrs;
> +		if (res.syms)
> +			syms = res.syms;
> +		else
> +			addrs = res.addrs;

this could be just:

		syms = res.syms;
		addrs = res.addrs;

once (cnt > 0) then we have either syms or addrs defined
and the other is NULL

>  		cnt = res.cnt;
>  	}
>  
> @@ -10511,12 +10579,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>  		goto error;
>  	}
>  	link->fd = link_fd;
> -	free(res.addrs);
> +	kprobe_multi_resolve_resource_free(&res);
>  	return link;
>  
>  error:
>  	free(link);
> -	free(res.addrs);
> +	kprobe_multi_resolve_resource_free(&res);
>  	return libbpf_err_ptr(err);
>  }
>  
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index e4d05662a96c..fdf6b464481f 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -481,8 +481,10 @@ __s32 btf__find_by_name_kind_own(const struct btf *btf, const char *type_name,
>  
>  typedef int (*kallsyms_cb_t)(unsigned long long sym_addr, char sym_type,
>  			     const char *sym_name, void *ctx);
> -
>  int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *arg);
> +typedef int (*available_filter_functions_cb_t)(const char *sym_name, void *ctx);
> +int libbpf_available_filter_functions_parse(available_filter_functions_cb_t cb,
> +					    void *arg);

let's kee it static, we don't have any other callers

thanks,
jirka

[PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

When using regular expression matching with "kprobe multi", it scans all
the functions under "/proc/kallsyms" that can be matched. However, not all
of them can be traced by kprobe.multi. If any one of the functions fails
to be traced, it will result in the failure of all functions. The best
approach is to filter out the functions that cannot be traced to ensure
proper tracking of the functions.

Use available_filter_functions check first, if failed, fallback to
kallsyms.

Here is the test eBPF program [1].
[1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867

Suggested-by: Jiri Olsa <olsajiri@gmail.com>
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 83 insertions(+), 9 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ad1ec893b41b..3dd72d69cdf7 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
 struct kprobe_multi_resolve {
 	const char *pattern;
 	unsigned long *addrs;
+	const char **syms;
 	size_t cap;
 	size_t cnt;
 };
 
 static int
-resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
-			const char *sym_name, void *ctx)
+kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
+				 const char *sym_name, void *ctx)
 {
 	struct kprobe_multi_resolve *res = ctx;
 	int err;
@@ -10431,8 +10432,8 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
 	if (!glob_match(sym_name, res->pattern))
 		return 0;
 
-	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
-				res->cnt + 1);
+	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap,
+				sizeof(unsigned long), res->cnt + 1);
 	if (err)
 		return err;
 
@@ -10440,6 +10441,73 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
 	return 0;
 }
 
+static int ftrace_resolve_kprobe_multi_cb(const char *sym_name, void *ctx)
+{
+	struct kprobe_multi_resolve *res = ctx;
+	int err;
+	char *name;
+
+	if (!glob_match(sym_name, res->pattern))
+		return 0;
+
+	err = libbpf_ensure_mem((void **) &res->syms, &res->cap,
+				sizeof(const char *), res->cnt + 1);
+	if (err)
+		return err;
+
+	name = strdup(sym_name);
+	if (!name)
+		return errno;
+
+	res->syms[res->cnt++] = name;
+	return 0;
+}
+
+typedef int (*available_filter_functions_cb_t)(const char *sym_name, void *ctx);
+
+static int
+libbpf_ftrace_parse(available_filter_functions_cb_t cb, void *ctx)
+{
+	char sym_name[256];
+	FILE *f;
+	int ret, err = 0;
+
+	f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
+	if (!f) {
+		pr_warn("failed to open available_filter_functions, fallback to /proc/kallsyms.\n");
+		return -EINVAL;
+	}
+
+	while (true) {
+		ret = fscanf(f, "%s%*[^\n]\n", sym_name);
+		if (ret == EOF && feof(f))
+			break;
+		if (ret != 1) {
+			pr_warn("failed to read available_filter_functions entry: %d\n",
+				ret);
+			break;
+		}
+
+		err = cb(sym_name, ctx);
+		if (err)
+			break;
+	}
+
+	fclose(f);
+	return err;
+}
+
+static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res)
+{
+	if (res->syms) {
+		while (res->cnt)
+			free((char *)res->syms[--res->cnt]);
+		free(res->syms);
+	} else {
+		free(res->addrs);
+	}
+}
+
 struct bpf_link *
 bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 				      const char *pattern,
@@ -10476,13 +10544,19 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 		return libbpf_err_ptr(-EINVAL);
 
 	if (pattern) {
-		err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
-		if (err)
-			goto error;
+		err = libbpf_ftrace_parse(ftrace_resolve_kprobe_multi_cb, &res);
+		if (err) {
+			/* fallback to kallsyms */
+			err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
+						    &res);
+			if (err)
+				goto error;
+		}
 		if (!res.cnt) {
 			err = -ENOENT;
 			goto error;
 		}
+		syms = res.syms;
 		addrs = res.addrs;
 		cnt = res.cnt;
 	}
@@ -10511,12 +10585,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 		goto error;
 	}
 	link->fd = link_fd;
-	free(res.addrs);
+	kprobe_multi_resolve_free(&res);
 	return link;
 
 error:
 	free(link);
-	free(res.addrs);
+	kprobe_multi_resolve_free(&res);
 	return libbpf_err_ptr(err);
 }
 
-- 
2.25.1

Re: [PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Andrii Nakryiko]

On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@linux.dev> wrote:
>
> From: Jackie Liu <liuyun01@kylinos.cn>
>
> When using regular expression matching with "kprobe multi", it scans all
> the functions under "/proc/kallsyms" that can be matched. However, not all
> of them can be traced by kprobe.multi. If any one of the functions fails
> to be traced, it will result in the failure of all functions. The best
> approach is to filter out the functions that cannot be traced to ensure
> proper tracking of the functions.
>
> Use available_filter_functions check first, if failed, fallback to
> kallsyms.
>
> Here is the test eBPF program [1].
> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
>
> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
>  tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 83 insertions(+), 9 deletions(-)
>

Question to you and Jiri: what happens when multi-kprobe's syms has
duplicates? Will the program be attached multiple times? If yes, then
it sounds like a problem? Both available_filters and kallsyms can have
duplicate function names in them, right?

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ad1ec893b41b..3dd72d69cdf7 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
>  struct kprobe_multi_resolve {
>         const char *pattern;
>         unsigned long *addrs;
> +       const char **syms;
>         size_t cap;
>         size_t cnt;
>  };
>
>  static int
> -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> -                       const char *sym_name, void *ctx)
> +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> +                                const char *sym_name, void *ctx)
>  {
>         struct kprobe_multi_resolve *res = ctx;
>         int err;
> @@ -10431,8 +10432,8 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>         if (!glob_match(sym_name, res->pattern))
>                 return 0;
>
> -       err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
> -                               res->cnt + 1);
> +       err = libbpf_ensure_mem((void **) &res->addrs, &res->cap,
> +                               sizeof(unsigned long), res->cnt + 1);
>         if (err)
>                 return err;
>
> @@ -10440,6 +10441,73 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>         return 0;
>  }
>
> +static int ftrace_resolve_kprobe_multi_cb(const char *sym_name, void *ctx)
> +{
> +       struct kprobe_multi_resolve *res = ctx;
> +       int err;
> +       char *name;
> +
> +       if (!glob_match(sym_name, res->pattern))
> +               return 0;
> +
> +       err = libbpf_ensure_mem((void **) &res->syms, &res->cap,
> +                               sizeof(const char *), res->cnt + 1);
> +       if (err)
> +               return err;
> +
> +       name = strdup(sym_name);
> +       if (!name)
> +               return errno;

-errno

> +
> +       res->syms[res->cnt++] = name;
> +       return 0;
> +}
> +
> +typedef int (*available_filter_functions_cb_t)(const char *sym_name, void *ctx);

quite mouthful, maybe just "available_kprobe_cb_t"? "filters"
terminology isn't common within libbpf and BPF tracing in general

> +
> +static int
> +libbpf_ftrace_parse(available_filter_functions_cb_t cb, void *ctx)

let's call it "libbpf_available_kprobes_parse" ?

> +{
> +       char sym_name[256];
> +       FILE *f;
> +       int ret, err = 0;
> +
> +       f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");

we need to check between DEBUGFS and TRACEFS, let's do something like
tracefs_kprobe_events()

> +       if (!f) {
> +               pr_warn("failed to open available_filter_functions, fallback to /proc/kallsyms.\n");
> +               return -EINVAL;

preserve errno, just like libbpf_kallsyms_parse

> +       }
> +
> +       while (true) {
> +               ret = fscanf(f, "%s%*[^\n]\n", sym_name);

%255s, similar to libbpf_kallsyms_probe. You have precedent code that
does parsing like this, please stick to the same approaches

> +               if (ret == EOF && feof(f))
> +                       break;
> +               if (ret != 1) {
> +                       pr_warn("failed to read available_filter_functions entry: %d\n",

s/available_filter_functions/kprobe/

> +                               ret);

err = -EINVAL

> +                       break;
> +               }
> +
> +               err = cb(sym_name, ctx);
> +               if (err)
> +                       break;
> +       }
> +
> +       fclose(f);
> +       return err;
> +}
> +
> +static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res)
> +{
> +       if (res->syms) {
> +               while (res->cnt)
> +                       free((char *)res->syms[--res->cnt]);
> +               free(res->syms);
> +       } else {
> +               free(res->addrs);

there is no need to assume that res->addrs will be null, let's free it
unconditionally. free() handles NULL just fine

> +       }
> +}
> +
>  struct bpf_link *
>  bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>                                       const char *pattern,
> @@ -10476,13 +10544,19 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>                 return libbpf_err_ptr(-EINVAL);
>
>         if (pattern) {
> -               err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
> -               if (err)
> -                       goto error;
> +               err = libbpf_ftrace_parse(ftrace_resolve_kprobe_multi_cb, &res);
> +               if (err) {
> +                       /* fallback to kallsyms */
> +                       err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
> +                                                   &res);
> +                       if (err)
> +                               goto error;
> +               }
>                 if (!res.cnt) {
>                         err = -ENOENT;
>                         goto error;
>                 }
> +               syms = res.syms;
>                 addrs = res.addrs;
>                 cnt = res.cnt;
>         }
> @@ -10511,12 +10585,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>                 goto error;
>         }
>         link->fd = link_fd;
> -       free(res.addrs);
> +       kprobe_multi_resolve_free(&res);
>         return link;
>
>  error:
>         free(link);
> -       free(res.addrs);
> +       kprobe_multi_resolve_free(&res);
>         return libbpf_err_ptr(err);
>  }
>
> --
> 2.25.1
>
>

Re: [PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Jackie Liu]

Hi Andrii.

在 2023/5/26 04:43, Andrii Nakryiko 写道:
> On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@linux.dev> wrote:
>>
>> From: Jackie Liu <liuyun01@kylinos.cn>
>>
>> When using regular expression matching with "kprobe multi", it scans all
>> the functions under "/proc/kallsyms" that can be matched. However, not all
>> of them can be traced by kprobe.multi. If any one of the functions fails
>> to be traced, it will result in the failure of all functions. The best
>> approach is to filter out the functions that cannot be traced to ensure
>> proper tracking of the functions.
>>
>> Use available_filter_functions check first, if failed, fallback to
>> kallsyms.
>>
>> Here is the test eBPF program [1].
>> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
>>
>> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>> ---
>>   tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 83 insertions(+), 9 deletions(-)
>>
> 
> Question to you and Jiri: what happens when multi-kprobe's syms has
> duplicates? Will the program be attached multiple times? If yes, then
> it sounds like a problem? Both available_filters and kallsyms can have
> duplicate function names in them, right?

If I understand correctly, there should be no problem with repeated
function registration, because the bottom layer is done through fprobe
registration addrs, kprobe.multi itself does not do this work, but
fprobe is based on ftrace, it will register addr by makes a hash,
that is, if it is the same address, it should be filtered out.

The main problem here is not the problem of repeated registration of
functions, but some functions are not allowed to hook. For example, when
I track vfs_*, vfs_set_acl_prepare_kgid and vfs_set_acl_prepare_kuid are
not allowed to hook. These exist under kallsyms, but
available_filter_functions does not, I have observed for a while,
matching through available_filter_functions can effectively prevent this
from happening.

> 
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index ad1ec893b41b..3dd72d69cdf7 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
>>   struct kprobe_multi_resolve {
>>          const char *pattern;
>>          unsigned long *addrs;
>> +       const char **syms;
>>          size_t cap;
>>          size_t cnt;
>>   };
>>
>>   static int
>> -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>> -                       const char *sym_name, void *ctx)
>> +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>> +                                const char *sym_name, void *ctx)
>>   {
>>          struct kprobe_multi_resolve *res = ctx;
>>          int err;
>> @@ -10431,8 +10432,8 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>>          if (!glob_match(sym_name, res->pattern))
>>                  return 0;
>>
>> -       err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
>> -                               res->cnt + 1);
>> +       err = libbpf_ensure_mem((void **) &res->addrs, &res->cap,
>> +                               sizeof(unsigned long), res->cnt + 1);
>>          if (err)
>>                  return err;
>>
>> @@ -10440,6 +10441,73 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>>          return 0;
>>   }
>>
>> +static int ftrace_resolve_kprobe_multi_cb(const char *sym_name, void *ctx)
>> +{
>> +       struct kprobe_multi_resolve *res = ctx;
>> +       int err;
>> +       char *name;
>> +
>> +       if (!glob_match(sym_name, res->pattern))
>> +               return 0;
>> +
>> +       err = libbpf_ensure_mem((void **) &res->syms, &res->cap,
>> +                               sizeof(const char *), res->cnt + 1);
>> +       if (err)
>> +               return err;
>> +
>> +       name = strdup(sym_name);
>> +       if (!name)
>> +               return errno;
> 
> -errno
> 
>> +
>> +       res->syms[res->cnt++] = name;
>> +       return 0;
>> +}
>> +
>> +typedef int (*available_filter_functions_cb_t)(const char *sym_name, void *ctx);
> 
> quite mouthful, maybe just "available_kprobe_cb_t"? "filters"
> terminology isn't common within libbpf and BPF tracing in general
> 
>> +
>> +static int
>> +libbpf_ftrace_parse(available_filter_functions_cb_t cb, void *ctx)
> 
> let's call it "libbpf_available_kprobes_parse" ?
> 
>> +{
>> +       char sym_name[256];
>> +       FILE *f;
>> +       int ret, err = 0;
>> +
>> +       f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> 
> we need to check between DEBUGFS and TRACEFS, let's do something like
> tracefs_kprobe_events()

Got.

> 
>> +       if (!f) {
>> +               pr_warn("failed to open available_filter_functions, fallback to /proc/kallsyms.\n");
>> +               return -EINVAL;
> 
> preserve errno, just like libbpf_kallsyms_parse
> 
>> +       }
>> +
>> +       while (true) {
>> +               ret = fscanf(f, "%s%*[^\n]\n", sym_name);
> 
> %255s, similar to libbpf_kallsyms_probe. You have precedent code that
> does parsing like this, please stick to the same approaches
> 
>> +               if (ret == EOF && feof(f))
>> +                       break;
>> +               if (ret != 1) {
>> +                       pr_warn("failed to read available_filter_functions entry: %d\n",
> 
> s/available_filter_functions/kprobe/
> 
>> +                               ret);
> 
> err = -EINVAL
> 
>> +                       break;
>> +               }
>> +
>> +               err = cb(sym_name, ctx);
>> +               if (err)
>> +                       break;
>> +       }
>> +
>> +       fclose(f);
>> +       return err;
>> +}
>> +
>> +static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res)
>> +{
>> +       if (res->syms) {
>> +               while (res->cnt)
>> +                       free((char *)res->syms[--res->cnt]);
>> +               free(res->syms);
>> +       } else {
>> +               free(res->addrs);
> 
> there is no need to assume that res->addrs will be null, let's free it
> unconditionally. free() handles NULL just fine

Yes.

-- 
Jackie Liu

> 
>> +       }
>> +}
>> +
>>   struct bpf_link *
>>   bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>>                                        const char *pattern,
>> @@ -10476,13 +10544,19 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>>                  return libbpf_err_ptr(-EINVAL);
>>
>>          if (pattern) {
>> -               err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
>> -               if (err)
>> -                       goto error;
>> +               err = libbpf_ftrace_parse(ftrace_resolve_kprobe_multi_cb, &res);
>> +               if (err) {
>> +                       /* fallback to kallsyms */
>> +                       err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
>> +                                                   &res);
>> +                       if (err)
>> +                               goto error;
>> +               }
>>                  if (!res.cnt) {
>>                          err = -ENOENT;
>>                          goto error;
>>                  }
>> +               syms = res.syms;
>>                  addrs = res.addrs;
>>                  cnt = res.cnt;
>>          }
>> @@ -10511,12 +10585,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>>                  goto error;
>>          }
>>          link->fd = link_fd;
>> -       free(res.addrs);
>> +       kprobe_multi_resolve_free(&res);
>>          return link;
>>
>>   error:
>>          free(link);
>> -       free(res.addrs);
>> +       kprobe_multi_resolve_free(&res);
>>          return libbpf_err_ptr(err);
>>   }
>>
>> --
>> 2.25.1
>>
>>

Re: [PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Jiri Olsa]

On Fri, May 26, 2023 at 09:38:15AM +0800, Jackie Liu wrote:
> Hi Andrii.
> 
> 在 2023/5/26 04:43, Andrii Nakryiko 写道:
> > On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@linux.dev> wrote:
> > > 
> > > From: Jackie Liu <liuyun01@kylinos.cn>
> > > 
> > > When using regular expression matching with "kprobe multi", it scans all
> > > the functions under "/proc/kallsyms" that can be matched. However, not all
> > > of them can be traced by kprobe.multi. If any one of the functions fails
> > > to be traced, it will result in the failure of all functions. The best
> > > approach is to filter out the functions that cannot be traced to ensure
> > > proper tracking of the functions.
> > > 
> > > Use available_filter_functions check first, if failed, fallback to
> > > kallsyms.
> > > 
> > > Here is the test eBPF program [1].
> > > [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
> > > 
> > > Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> > > Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> > > ---
> > >   tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
> > >   1 file changed, 83 insertions(+), 9 deletions(-)
> > > 
> > 
> > Question to you and Jiri: what happens when multi-kprobe's syms has
> > duplicates? Will the program be attached multiple times? If yes, then
> > it sounds like a problem? Both available_filters and kallsyms can have
> > duplicate function names in them, right?
> 
> If I understand correctly, there should be no problem with repeated
> function registration, because the bottom layer is done through fprobe
> registration addrs, kprobe.multi itself does not do this work, but
> fprobe is based on ftrace, it will register addr by makes a hash,
> that is, if it is the same address, it should be filtered out.

it won't get through the kprobe_multi symbols resolve code, because we
check that the number of resolved addresses matches the number of
provided symbols

also found test bug (hunk#2) when checking on that (hunk#1) ;-)

jirka


---
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index 2173c4bb555e..e78362354bd3 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -124,7 +124,7 @@ static void test_link_api_syms(void)
 		"bpf_fentry_test5",
 		"bpf_fentry_test6",
 		"bpf_fentry_test7",
-		"bpf_fentry_test8",
+		"bpf_fentry_test7",
 	};
 
 	opts.kprobe_multi.syms = syms;
@@ -477,9 +477,9 @@ void test_kprobe_multi_test(void)
 	if (test__start_subtest("skel_api"))
 		test_skel_api();
 	if (test__start_subtest("link_api_addrs"))
-		test_link_api_syms();
-	if (test__start_subtest("link_api_syms"))
 		test_link_api_addrs();
+	if (test__start_subtest("link_api_syms"))
+		test_link_api_syms();
 	if (test__start_subtest("attach_api_pattern"))
 		test_attach_api_pattern();
 	if (test__start_subtest("attach_api_addrs"))

Re: [PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Andrii Nakryiko]

On Thu, May 25, 2023 at 6:38 PM Jackie Liu <liu.yun@linux.dev> wrote:
>
> Hi Andrii.
>
> 在 2023/5/26 04:43, Andrii Nakryiko 写道:
> > On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@linux.dev> wrote:
> >>
> >> From: Jackie Liu <liuyun01@kylinos.cn>
> >>
> >> When using regular expression matching with "kprobe multi", it scans all
> >> the functions under "/proc/kallsyms" that can be matched. However, not all
> >> of them can be traced by kprobe.multi. If any one of the functions fails
> >> to be traced, it will result in the failure of all functions. The best
> >> approach is to filter out the functions that cannot be traced to ensure
> >> proper tracking of the functions.
> >>
> >> Use available_filter_functions check first, if failed, fallback to
> >> kallsyms.
> >>
> >> Here is the test eBPF program [1].
> >> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
> >>
> >> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> >> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> >> ---
> >>   tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
> >>   1 file changed, 83 insertions(+), 9 deletions(-)
> >>
> >
> > Question to you and Jiri: what happens when multi-kprobe's syms has
> > duplicates? Will the program be attached multiple times? If yes, then
> > it sounds like a problem? Both available_filters and kallsyms can have
> > duplicate function names in them, right?
>
> If I understand correctly, there should be no problem with repeated
> function registration, because the bottom layer is done through fprobe
> registration addrs, kprobe.multi itself does not do this work, but
> fprobe is based on ftrace, it will register addr by makes a hash,
> that is, if it is the same address, it should be filtered out.
>

Looking at kernel code, it seems kernel will actually return error if
user specifies multiple duplicated names. Because kernel will
bsearch() to the first instance, and never resolve the second
duplicated instance. And then will assume that not all symbols are
resolved.

So, it worries me that we'll switch from kallsyms to available_filters
by default, because that introduces new failure modes.

Either way, let's add a selftest that uses a duplicate function name
and see what happens?

> The main problem here is not the problem of repeated registration of
> functions, but some functions are not allowed to hook. For example, when
> I track vfs_*, vfs_set_acl_prepare_kgid and vfs_set_acl_prepare_kuid are
> not allowed to hook. These exist under kallsyms, but
> available_filter_functions does not, I have observed for a while,
> matching through available_filter_functions can effectively prevent this
> from happening.

Yeah, I understand that. My point above is that a)
available_filter_functions contains duplicates and b) doesn't contain
addresses. So we are forced to rely on kernel string -> addr
resolution, which doesn't seem to handle duplicate entries well (let's
test).

So it's a regression to switch to that without taking any other precautions.

>
> >
> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >> index ad1ec893b41b..3dd72d69cdf7 100644
> >> --- a/tools/lib/bpf/libbpf.c
> >> +++ b/tools/lib/bpf/libbpf.c
> >> @@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
> >>   struct kprobe_multi_resolve {
> >>          const char *pattern;
> >>          unsigned long *addrs;
> >> +       const char **syms;
> >>          size_t cap;
> >>          size_t cnt;
> >>   };
> >>

[...]

Re: [PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Jackie Liu]

Hi Andrii.

在 2023/6/3 01:27, Andrii Nakryiko 写道:
> On Thu, May 25, 2023 at 6:38 PM Jackie Liu <liu.yun@linux.dev> wrote:
>>
>> Hi Andrii.
>>
>> 在 2023/5/26 04:43, Andrii Nakryiko 写道:
>>> On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@linux.dev> wrote:
>>>>
>>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>>
>>>> When using regular expression matching with "kprobe multi", it scans all
>>>> the functions under "/proc/kallsyms" that can be matched. However, not all
>>>> of them can be traced by kprobe.multi. If any one of the functions fails
>>>> to be traced, it will result in the failure of all functions. The best
>>>> approach is to filter out the functions that cannot be traced to ensure
>>>> proper tracking of the functions.
>>>>
>>>> Use available_filter_functions check first, if failed, fallback to
>>>> kallsyms.
>>>>
>>>> Here is the test eBPF program [1].
>>>> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
>>>>
>>>> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
>>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>>> ---
>>>>    tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
>>>>    1 file changed, 83 insertions(+), 9 deletions(-)
>>>>
>>>
>>> Question to you and Jiri: what happens when multi-kprobe's syms has
>>> duplicates? Will the program be attached multiple times? If yes, then
>>> it sounds like a problem? Both available_filters and kallsyms can have
>>> duplicate function names in them, right?

I don't have any idea, I tested it on my own device, and they don't have
duplicate functions.

╭─jackieliu@jackieliu-PC ~/gitee/ketones/src
╰─➤ sudo cat /sys/kernel/debug/tracing/available_filter_functions | awk 
-F' ' '{print $1}' | wc -l 

81882
╭─jackieliu@jackieliu-PC ~/gitee/ketones/src
╰─➤ sudo cat /sys/kernel/debug/tracing/available_filter_functions | awk 
-F' ' '{print $1}' | uniq | wc -l 

81882

>>
>> If I understand correctly, there should be no problem with repeated
>> function registration, because the bottom layer is done through fprobe
>> registration addrs, kprobe.multi itself does not do this work, but
>> fprobe is based on ftrace, it will register addr by makes a hash,
>> that is, if it is the same address, it should be filtered out.
>>
> 
> Looking at kernel code, it seems kernel will actually return error if
> user specifies multiple duplicated names. Because kernel will
> bsearch() to the first instance, and never resolve the second
> duplicated instance. And then will assume that not all symbols are
> resolved.

I wrote a test program myself, but it cannot be attached normally, and
an error will be reported.

const char *sysms[] = {
     "vfs_read",
     "vfs_write",
     "vfs_read",
};

when attach_kprobe_multi, -3 returned.

> 
> So, it worries me that we'll switch from kallsyms to available_filters
> by default, because that introduces new failure modes.

In fact, this is not a new problem introduced by switching from kallsyms
to available_filters. If kallsyms also has duplicate functions, then
this problem will also exist before.

> 
> Either way, let's add a selftest that uses a duplicate function name
> and see what happens?

Hi Jiri, Do you mind write a self-test program for duplicate function? I
saw that it has been written before.
for some reason, I failed to compile kselftest/bpf successfully on
fedora38 and Ubuntu2004. :<


> 
>> The main problem here is not the problem of repeated registration of
>> functions, but some functions are not allowed to hook. For example, when
>> I track vfs_*, vfs_set_acl_prepare_kgid and vfs_set_acl_prepare_kuid are
>> not allowed to hook. These exist under kallsyms, but
>> available_filter_functions does not, I have observed for a while,
>> matching through available_filter_functions can effectively prevent this
>> from happening.
> 
> Yeah, I understand that. My point above is that a)
> available_filter_functions contains duplicates and b) doesn't contain
> addresses. So we are forced to rely on kernel string -> addr
> resolution, which doesn't seem to handle duplicate entries well (let's
> test).

Yes, the test for repeated functions reports errors. If there is an
interface similar to available_filter_functions, which contains the
function name and function address, and ensures that it is not 
duplicate, then it is a good speedup for eBPF program, because using
'strdup' to record the function name consumes a certain amount of
startup time.

> 
> So it's a regression to switch to that without taking any other precautions.
> 

Yes, agree.

-- 
BR, Jackie Liu
>>
>>>
>>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>>> index ad1ec893b41b..3dd72d69cdf7 100644
>>>> --- a/tools/lib/bpf/libbpf.c
>>>> +++ b/tools/lib/bpf/libbpf.c
>>>> @@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
>>>>    struct kprobe_multi_resolve {
>>>>           const char *pattern;
>>>>           unsigned long *addrs;
>>>> +       const char **syms;
>>>>           size_t cap;
>>>>           size_t cnt;
>>>>    };
>>>>
> 
> [...]

Re: [PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Andrii Nakryiko]

On Tue, Jun 6, 2023 at 11:01 PM Jackie Liu <liu.yun@linux.dev> wrote:
>
> Hi Andrii.
>
> 在 2023/6/3 01:27, Andrii Nakryiko 写道:
> > On Thu, May 25, 2023 at 6:38 PM Jackie Liu <liu.yun@linux.dev> wrote:
> >>
> >> Hi Andrii.
> >>
> >> 在 2023/5/26 04:43, Andrii Nakryiko 写道:
> >>> On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@linux.dev> wrote:
> >>>>
> >>>> From: Jackie Liu <liuyun01@kylinos.cn>
> >>>>
> >>>> When using regular expression matching with "kprobe multi", it scans all
> >>>> the functions under "/proc/kallsyms" that can be matched. However, not all
> >>>> of them can be traced by kprobe.multi. If any one of the functions fails
> >>>> to be traced, it will result in the failure of all functions. The best
> >>>> approach is to filter out the functions that cannot be traced to ensure
> >>>> proper tracking of the functions.
> >>>>
> >>>> Use available_filter_functions check first, if failed, fallback to
> >>>> kallsyms.
> >>>>
> >>>> Here is the test eBPF program [1].
> >>>> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
> >>>>
> >>>> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> >>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> >>>> ---
> >>>>    tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
> >>>>    1 file changed, 83 insertions(+), 9 deletions(-)
> >>>>
> >>>
> >>> Question to you and Jiri: what happens when multi-kprobe's syms has
> >>> duplicates? Will the program be attached multiple times? If yes, then
> >>> it sounds like a problem? Both available_filters and kallsyms can have
> >>> duplicate function names in them, right?
>
> I don't have any idea, I tested it on my own device, and they don't have
> duplicate functions.
>
> ╭─jackieliu@jackieliu-PC ~/gitee/ketones/src
> ╰─➤ sudo cat /sys/kernel/debug/tracing/available_filter_functions | awk
> -F' ' '{print $1}' | wc -l
>
> 81882
> ╭─jackieliu@jackieliu-PC ~/gitee/ketones/src
> ╰─➤ sudo cat /sys/kernel/debug/tracing/available_filter_functions | awk
> -F' ' '{print $1}' | uniq | wc -l
>
> 81882

hm... I'm pretty sure there are plenty:

$ sudo cat /sys/kernel/debug/tracing/available_filter_functions | grep
-v __ftrace_invalid_address | sort | uniq -c | sort -nr | head -n10
     14 type_show
     12 init_once
     11 modalias_show
      8 event_show
      7 name_show
      6 enabled_show
      5 version_show
      5 size_show
      5 offset_show
      5 numa_node_show


>
> >>
> >> If I understand correctly, there should be no problem with repeated
> >> function registration, because the bottom layer is done through fprobe
> >> registration addrs, kprobe.multi itself does not do this work, but
> >> fprobe is based on ftrace, it will register addr by makes a hash,
> >> that is, if it is the same address, it should be filtered out.
> >>
> >
> > Looking at kernel code, it seems kernel will actually return error if
> > user specifies multiple duplicated names. Because kernel will
> > bsearch() to the first instance, and never resolve the second
> > duplicated instance. And then will assume that not all symbols are
> > resolved.
>
> I wrote a test program myself, but it cannot be attached normally, and
> an error will be reported.
>
> const char *sysms[] = {
>      "vfs_read",
>      "vfs_write",
>      "vfs_read",
> };
>
> when attach_kprobe_multi, -3 returned.
>
> >
> > So, it worries me that we'll switch from kallsyms to available_filters
> > by default, because that introduces new failure modes.
>
> In fact, this is not a new problem introduced by switching from kallsyms
> to available_filters. If kallsyms also has duplicate functions, then
> this problem will also exist before.

It is, because currently when we parse kallsyms we remember function
addresses, which are unique. We don't rely on kernel string -> addr
resolution.

>
> >
> > Either way, let's add a selftest that uses a duplicate function name
> > and see what happens?
>
> Hi Jiri, Do you mind write a self-test program for duplicate function? I
> saw that it has been written before.
> for some reason, I failed to compile kselftest/bpf successfully on
> fedora38 and Ubuntu2004. :<
>
>
> >
> >> The main problem here is not the problem of repeated registration of
> >> functions, but some functions are not allowed to hook. For example, when
> >> I track vfs_*, vfs_set_acl_prepare_kgid and vfs_set_acl_prepare_kuid are
> >> not allowed to hook. These exist under kallsyms, but
> >> available_filter_functions does not, I have observed for a while,
> >> matching through available_filter_functions can effectively prevent this
> >> from happening.
> >
> > Yeah, I understand that. My point above is that a)
> > available_filter_functions contains duplicates and b) doesn't contain
> > addresses. So we are forced to rely on kernel string -> addr
> > resolution, which doesn't seem to handle duplicate entries well (let's
> > test).
>
> Yes, the test for repeated functions reports errors. If there is an
> interface similar to available_filter_functions, which contains the
> function name and function address, and ensures that it is not
> duplicate, then it is a good speedup for eBPF program, because using
> 'strdup' to record the function name consumes a certain amount of
> startup time.
>
> >
> > So it's a regression to switch to that without taking any other precautions.
> >
>
> Yes, agree.
>
> --
> BR, Jackie Liu
> >>
> >>>
> >>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >>>> index ad1ec893b41b..3dd72d69cdf7 100644
> >>>> --- a/tools/lib/bpf/libbpf.c
> >>>> +++ b/tools/lib/bpf/libbpf.c
> >>>> @@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
> >>>>    struct kprobe_multi_resolve {
> >>>>           const char *pattern;
> >>>>           unsigned long *addrs;
> >>>> +       const char **syms;
> >>>>           size_t cap;
> >>>>           size_t cnt;
> >>>>    };
> >>>>
> >
> > [...]

Re: [PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Jiri Olsa]

On Fri, Jun 02, 2023 at 10:27:31AM -0700, Andrii Nakryiko wrote:
> On Thu, May 25, 2023 at 6:38 PM Jackie Liu <liu.yun@linux.dev> wrote:
> >
> > Hi Andrii.
> >
> > 在 2023/5/26 04:43, Andrii Nakryiko 写道:
> > > On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@linux.dev> wrote:
> > >>
> > >> From: Jackie Liu <liuyun01@kylinos.cn>
> > >>
> > >> When using regular expression matching with "kprobe multi", it scans all
> > >> the functions under "/proc/kallsyms" that can be matched. However, not all
> > >> of them can be traced by kprobe.multi. If any one of the functions fails
> > >> to be traced, it will result in the failure of all functions. The best
> > >> approach is to filter out the functions that cannot be traced to ensure
> > >> proper tracking of the functions.
> > >>
> > >> Use available_filter_functions check first, if failed, fallback to
> > >> kallsyms.
> > >>
> > >> Here is the test eBPF program [1].
> > >> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
> > >>
> > >> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> > >> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> > >> ---
> > >>   tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
> > >>   1 file changed, 83 insertions(+), 9 deletions(-)
> > >>
> > >
> > > Question to you and Jiri: what happens when multi-kprobe's syms has
> > > duplicates? Will the program be attached multiple times? If yes, then
> > > it sounds like a problem? Both available_filters and kallsyms can have
> > > duplicate function names in them, right?
> >
> > If I understand correctly, there should be no problem with repeated
> > function registration, because the bottom layer is done through fprobe
> > registration addrs, kprobe.multi itself does not do this work, but
> > fprobe is based on ftrace, it will register addr by makes a hash,
> > that is, if it is the same address, it should be filtered out.
> >
> 
> Looking at kernel code, it seems kernel will actually return error if
> user specifies multiple duplicated names. Because kernel will
> bsearch() to the first instance, and never resolve the second
> duplicated instance. And then will assume that not all symbols are
> resolved.

right, as I wrote in here [1] it will fail

[1] https://lore.kernel.org/bpf/ZHB0xNEbjmwHv18d@krava/

> 
> So, it worries me that we'll switch from kallsyms to available_filters
> by default, because that introduces new failure modes.

we did not care about duplicate with kallsyms because we used addresses,
and I think with duplicate addresss the kprobe_multi link will probably
attach (need to check) while with duplicate symbols it won't..

perhaps we could make sure we don't pass duplicate symbols?

we do the kprobe_multi bench with symbol names read from available_filter_functions
and we filter out duplicates

jirka

> 
> Either way, let's add a selftest that uses a duplicate function name
> and see what happens?
> 
> > The main problem here is not the problem of repeated registration of
> > functions, but some functions are not allowed to hook. For example, when
> > I track vfs_*, vfs_set_acl_prepare_kgid and vfs_set_acl_prepare_kuid are
> > not allowed to hook. These exist under kallsyms, but
> > available_filter_functions does not, I have observed for a while,
> > matching through available_filter_functions can effectively prevent this
> > from happening.
> 
> Yeah, I understand that. My point above is that a)
> available_filter_functions contains duplicates and b) doesn't contain
> addresses. So we are forced to rely on kernel string -> addr
> resolution, which doesn't seem to handle duplicate entries well (let's
> test).
> 
> So it's a regression to switch to that without taking any other precautions.
> 
> >
> > >
> > >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > >> index ad1ec893b41b..3dd72d69cdf7 100644
> > >> --- a/tools/lib/bpf/libbpf.c
> > >> +++ b/tools/lib/bpf/libbpf.c
> > >> @@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
> > >>   struct kprobe_multi_resolve {
> > >>          const char *pattern;
> > >>          unsigned long *addrs;
> > >> +       const char **syms;
> > >>          size_t cap;
> > >>          size_t cnt;
> > >>   };
> > >>
> 
> [...]

Re: [PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Andrii Nakryiko]

On Wed, Jun 7, 2023 at 4:22 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Fri, Jun 02, 2023 at 10:27:31AM -0700, Andrii Nakryiko wrote:
> > On Thu, May 25, 2023 at 6:38 PM Jackie Liu <liu.yun@linux.dev> wrote:
> > >
> > > Hi Andrii.
> > >
> > > 在 2023/5/26 04:43, Andrii Nakryiko 写道:
> > > > On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@linux.dev> wrote:
> > > >>
> > > >> From: Jackie Liu <liuyun01@kylinos.cn>
> > > >>
> > > >> When using regular expression matching with "kprobe multi", it scans all
> > > >> the functions under "/proc/kallsyms" that can be matched. However, not all
> > > >> of them can be traced by kprobe.multi. If any one of the functions fails
> > > >> to be traced, it will result in the failure of all functions. The best
> > > >> approach is to filter out the functions that cannot be traced to ensure
> > > >> proper tracking of the functions.
> > > >>
> > > >> Use available_filter_functions check first, if failed, fallback to
> > > >> kallsyms.
> > > >>
> > > >> Here is the test eBPF program [1].
> > > >> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
> > > >>
> > > >> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> > > >> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> > > >> ---
> > > >>   tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
> > > >>   1 file changed, 83 insertions(+), 9 deletions(-)
> > > >>
> > > >
> > > > Question to you and Jiri: what happens when multi-kprobe's syms has
> > > > duplicates? Will the program be attached multiple times? If yes, then
> > > > it sounds like a problem? Both available_filters and kallsyms can have
> > > > duplicate function names in them, right?
> > >
> > > If I understand correctly, there should be no problem with repeated
> > > function registration, because the bottom layer is done through fprobe
> > > registration addrs, kprobe.multi itself does not do this work, but
> > > fprobe is based on ftrace, it will register addr by makes a hash,
> > > that is, if it is the same address, it should be filtered out.
> > >
> >
> > Looking at kernel code, it seems kernel will actually return error if
> > user specifies multiple duplicated names. Because kernel will
> > bsearch() to the first instance, and never resolve the second
> > duplicated instance. And then will assume that not all symbols are
> > resolved.
>
> right, as I wrote in here [1] it will fail
>
> [1] https://lore.kernel.org/bpf/ZHB0xNEbjmwHv18d@krava/
>
> >
> > So, it worries me that we'll switch from kallsyms to available_filters
> > by default, because that introduces new failure modes.
>
> we did not care about duplicate with kallsyms because we used addresses,
> and I think with duplicate addresss the kprobe_multi link will probably
> attach (need to check) while with duplicate symbols it won't..
>
> perhaps we could make sure we don't pass duplicate symbols?

I think we have to stick to kallsyms and addresses. What if I actually
want to attach to all instances of type_show? We should take into
account available_filter_functions, but still use addresses from
kallsyms.

I'd also advocate working on having an available_filter_functions
version reporting not just function names, but also its associated
address. That would actually eliminate the need for kallsyms.

I chatted with Steven Rostedt about this at the last LSF/MM/BPF
conference, and I think we both agreed that we both a) have all the
information in the kernel to implement this and b) it's a good idea to
expose all that to user space. For backwards compat reasons it will
have to be a separate file, but it's generated on the fly, so it's not
a big deal in terms of resource usage.


>
> we do the kprobe_multi bench with symbol names read from available_filter_functions
> and we filter out duplicates
>
> jirka
>
> >
> > Either way, let's add a selftest that uses a duplicate function name
> > and see what happens?
> >
> > > The main problem here is not the problem of repeated registration of
> > > functions, but some functions are not allowed to hook. For example, when
> > > I track vfs_*, vfs_set_acl_prepare_kgid and vfs_set_acl_prepare_kuid are
> > > not allowed to hook. These exist under kallsyms, but
> > > available_filter_functions does not, I have observed for a while,
> > > matching through available_filter_functions can effectively prevent this
> > > from happening.
> >
> > Yeah, I understand that. My point above is that a)
> > available_filter_functions contains duplicates and b) doesn't contain
> > addresses. So we are forced to rely on kernel string -> addr
> > resolution, which doesn't seem to handle duplicate entries well (let's
> > test).
> >
> > So it's a regression to switch to that without taking any other precautions.
> >
> > >
> > > >
> > > >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > >> index ad1ec893b41b..3dd72d69cdf7 100644
> > > >> --- a/tools/lib/bpf/libbpf.c
> > > >> +++ b/tools/lib/bpf/libbpf.c
> > > >> @@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
> > > >>   struct kprobe_multi_resolve {
> > > >>          const char *pattern;
> > > >>          unsigned long *addrs;
> > > >> +       const char **syms;
> > > >>          size_t cap;
> > > >>          size_t cnt;
> > > >>   };
> > > >>
> >
> > [...]

Re: [PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Jackie Liu]

在 2023/6/8 08:00, Andrii Nakryiko 写道:
> On Wed, Jun 7, 2023 at 4:22 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>>
>> On Fri, Jun 02, 2023 at 10:27:31AM -0700, Andrii Nakryiko wrote:
>>> On Thu, May 25, 2023 at 6:38 PM Jackie Liu <liu.yun@linux.dev> wrote:
>>>>
>>>> Hi Andrii.
>>>>
>>>> 在 2023/5/26 04:43, Andrii Nakryiko 写道:
>>>>> On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@linux.dev> wrote:
>>>>>>
>>>>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>>>>
>>>>>> When using regular expression matching with "kprobe multi", it scans all
>>>>>> the functions under "/proc/kallsyms" that can be matched. However, not all
>>>>>> of them can be traced by kprobe.multi. If any one of the functions fails
>>>>>> to be traced, it will result in the failure of all functions. The best
>>>>>> approach is to filter out the functions that cannot be traced to ensure
>>>>>> proper tracking of the functions.
>>>>>>
>>>>>> Use available_filter_functions check first, if failed, fallback to
>>>>>> kallsyms.
>>>>>>
>>>>>> Here is the test eBPF program [1].
>>>>>> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
>>>>>>
>>>>>> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
>>>>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>>>>> ---
>>>>>>    tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
>>>>>>    1 file changed, 83 insertions(+), 9 deletions(-)
>>>>>>
>>>>>
>>>>> Question to you and Jiri: what happens when multi-kprobe's syms has
>>>>> duplicates? Will the program be attached multiple times? If yes, then
>>>>> it sounds like a problem? Both available_filters and kallsyms can have
>>>>> duplicate function names in them, right?
>>>>
>>>> If I understand correctly, there should be no problem with repeated
>>>> function registration, because the bottom layer is done through fprobe
>>>> registration addrs, kprobe.multi itself does not do this work, but
>>>> fprobe is based on ftrace, it will register addr by makes a hash,
>>>> that is, if it is the same address, it should be filtered out.
>>>>
>>>
>>> Looking at kernel code, it seems kernel will actually return error if
>>> user specifies multiple duplicated names. Because kernel will
>>> bsearch() to the first instance, and never resolve the second
>>> duplicated instance. And then will assume that not all symbols are
>>> resolved.
>>
>> right, as I wrote in here [1] it will fail
>>
>> [1] https://lore.kernel.org/bpf/ZHB0xNEbjmwHv18d@krava/
>>
>>>
>>> So, it worries me that we'll switch from kallsyms to available_filters
>>> by default, because that introduces new failure modes.
>>
>> we did not care about duplicate with kallsyms because we used addresses,
>> and I think with duplicate addresss the kprobe_multi link will probably
>> attach (need to check) while with duplicate symbols it won't..
>>
>> perhaps we could make sure we don't pass duplicate symbols?
> 
> I think we have to stick to kallsyms and addresses. What if I actually
> want to attach to all instances of type_show? We should take into
> account available_filter_functions, but still use addresses from
> kallsyms.
> 
> I'd also advocate working on having an available_filter_functions
> version reporting not just function names, but also its associated
> address. That would actually eliminate the need for kallsyms.
> 
> I chatted with Steven Rostedt about this at the last LSF/MM/BPF
> conference, and I think we both agreed that we both a) have all the
> information in the kernel to implement this and b) it's a good idea to
> expose all that to user space. For backwards compat reasons it will
> have to be a separate file, but it's generated on the fly, so it's not
> a big deal in terms of resource usage.

Yes, I noticed that the latest version of the kernel has added 
touched_functions and enabled_functions, are they? I'm not sure.
Perhaps we can wait for such an interface to appear before directly
switching to that interface, and then submit this patch again.

-- 
Jackie Liu

> 
> 
>>
>> we do the kprobe_multi bench with symbol names read from available_filter_functions
>> and we filter out duplicates
>>
>> jirka
>>
>>>
>>> Either way, let's add a selftest that uses a duplicate function name
>>> and see what happens?
>>>
>>>> The main problem here is not the problem of repeated registration of
>>>> functions, but some functions are not allowed to hook. For example, when
>>>> I track vfs_*, vfs_set_acl_prepare_kgid and vfs_set_acl_prepare_kuid are
>>>> not allowed to hook. These exist under kallsyms, but
>>>> available_filter_functions does not, I have observed for a while,
>>>> matching through available_filter_functions can effectively prevent this
>>>> from happening.
>>>
>>> Yeah, I understand that. My point above is that a)
>>> available_filter_functions contains duplicates and b) doesn't contain
>>> addresses. So we are forced to rely on kernel string -> addr
>>> resolution, which doesn't seem to handle duplicate entries well (let's
>>> test).
>>>
>>> So it's a regression to switch to that without taking any other precautions.
>>>
>>>>
>>>>>
>>>>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>>>>> index ad1ec893b41b..3dd72d69cdf7 100644
>>>>>> --- a/tools/lib/bpf/libbpf.c
>>>>>> +++ b/tools/lib/bpf/libbpf.c
>>>>>> @@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
>>>>>>    struct kprobe_multi_resolve {
>>>>>>           const char *pattern;
>>>>>>           unsigned long *addrs;
>>>>>> +       const char **syms;
>>>>>>           size_t cap;
>>>>>>           size_t cnt;
>>>>>>    };
>>>>>>
>>>
>>> [...]

[PATCH v5] libbpf: kprobe.multi: Filter with available_filter_functions

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

When using regular expression matching with "kprobe multi", it scans all
the functions under "/proc/kallsyms" that can be matched. However, not all
of them can be traced by kprobe.multi. If any one of the functions fails
to be traced, it will result in the failure of all functions. The best
approach is to filter out the functions that cannot be traced to ensure
proper tracking of the functions.

Use available_filter_functions check first, if failed, fallback to
kallsyms.

Here is the test eBPF program [1].
[1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867

Suggested-by: Jiri Olsa <olsajiri@gmail.com>
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 tools/lib/bpf/libbpf.c | 101 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 92 insertions(+), 9 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ad1ec893b41b..5aac8fe76c0a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10106,6 +10106,12 @@ static const char *tracefs_uprobe_events(void)
 	return use_debugfs() ? DEBUGFS"/uprobe_events" : TRACEFS"/uprobe_events";
 }
 
+static const char *tracefs_available_filter_functions(void)
+{
+	return use_debugfs() ? DEBUGFS"/available_filter_functions" :
+			       TRACEFS"/available_filter_functions";
+}
+
 static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
 					 const char *kfunc_name, size_t offset)
 {
@@ -10417,13 +10423,14 @@ static bool glob_match(const char *str, const char *pat)
 struct kprobe_multi_resolve {
 	const char *pattern;
 	unsigned long *addrs;
+	const char **syms;
 	size_t cap;
 	size_t cnt;
 };
 
 static int
-resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
-			const char *sym_name, void *ctx)
+kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
+				 const char *sym_name, void *ctx)
 {
 	struct kprobe_multi_resolve *res = ctx;
 	int err;
@@ -10431,8 +10438,8 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
 	if (!glob_match(sym_name, res->pattern))
 		return 0;
 
-	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
-				res->cnt + 1);
+	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap,
+				sizeof(unsigned long), res->cnt + 1);
 	if (err)
 		return err;
 
@@ -10440,6 +10447,75 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
 	return 0;
 }
 
+static int ftrace_resolve_kprobe_multi_cb(const char *sym_name, void *ctx)
+{
+	struct kprobe_multi_resolve *res = ctx;
+	int err;
+	char *name;
+
+	if (!glob_match(sym_name, res->pattern))
+		return 0;
+
+	err = libbpf_ensure_mem((void **) &res->syms, &res->cap,
+				sizeof(const char *), res->cnt + 1);
+	if (err)
+		return err;
+
+	name = strdup(sym_name);
+	if (!name)
+		return -errno;
+
+	res->syms[res->cnt++] = name;
+	return 0;
+}
+
+typedef int (*available_kprobe_cb_t)(const char *sym_name, void *ctx);
+
+static int
+libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
+{
+	char sym_name[256];
+	FILE *f;
+	int ret, err = 0;
+	const char *available_path = tracefs_available_filter_functions();
+
+	f = fopen(available_path, "r");
+	if (!f) {
+		err = -errno;
+		pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
+			available_path);
+		return err;
+	}
+
+	while (true) {
+		ret = fscanf(f, "%255s%*[^\n]\n", sym_name);
+		if (ret == EOF && feof(f))
+			break;
+		if (ret != 1) {
+			pr_warn("failed to read available kprobe entry: %d\n",
+				ret);
+			err = -EINVAL;
+			break;
+		}
+
+		err = cb(sym_name, ctx);
+		if (err)
+			break;
+	}
+
+	fclose(f);
+	return err;
+}
+
+static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res)
+{
+	while (res->syms && res->cnt)
+		free((char *)res->syms[--res->cnt]);
+
+	free(res->syms);
+	free(res->addrs);
+}
+
 struct bpf_link *
 bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 				      const char *pattern,
@@ -10476,13 +10552,20 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 		return libbpf_err_ptr(-EINVAL);
 
 	if (pattern) {
-		err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
-		if (err)
-			goto error;
+		err = libbpf_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
+						     &res);
+		if (err) {
+			/* fallback to kallsyms */
+			err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
+						    &res);
+			if (err)
+				goto error;
+		}
 		if (!res.cnt) {
 			err = -ENOENT;
 			goto error;
 		}
+		syms = res.syms;
 		addrs = res.addrs;
 		cnt = res.cnt;
 	}
@@ -10511,12 +10594,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 		goto error;
 	}
 	link->fd = link_fd;
-	free(res.addrs);
+	kprobe_multi_resolve_free(&res);
 	return link;
 
 error:
 	free(link);
-	free(res.addrs);
+	kprobe_multi_resolve_free(&res);
 	return libbpf_err_ptr(err);
 }
 
-- 
2.25.1

Re: [PATCH v5] libbpf: kprobe.multi: Filter with available_filter_functions

[Jiri Olsa]

On Fri, May 26, 2023 at 10:10:47AM +0800, Jackie Liu wrote:

SNIP

> -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> -			const char *sym_name, void *ctx)
> +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> +				 const char *sym_name, void *ctx)
>  {
>  	struct kprobe_multi_resolve *res = ctx;
>  	int err;
> @@ -10431,8 +10438,8 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>  	if (!glob_match(sym_name, res->pattern))
>  		return 0;
>  
> -	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
> -				res->cnt + 1);
> +	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap,
> +				sizeof(unsigned long), res->cnt + 1);

hum, looks like this is just formatting change, AFAICS we don't need that 

>  	if (err)
>  		return err;
>  
> @@ -10440,6 +10447,75 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>  	return 0;
>  }
>  

SNIP

> +
> +static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res)
> +{
> +	while (res->syms && res->cnt)
> +		free((char *)res->syms[--res->cnt]);
> +
> +	free(res->syms);
> +	free(res->addrs);

we should set cnt and cap to zero for the fallback sake

> +}
> +
>  struct bpf_link *
>  bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>  				      const char *pattern,
> @@ -10476,13 +10552,20 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>  		return libbpf_err_ptr(-EINVAL);
>  
>  	if (pattern) {
> -		err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
> -		if (err)
> -			goto error;
> +		err = libbpf_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
> +						     &res);
> +		if (err) {
> +			/* fallback to kallsyms */

we need to call kprobe_multi_resolve_free in here and set
cnt/cap to zero in kprobe_multi_resolve_free

jirka

> +			err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
> +						    &res);
> +			if (err)
> +				goto error;
> +		}
>  		if (!res.cnt) {
>  			err = -ENOENT;
>  			goto error;
>  		}
> +		syms = res.syms;
>  		addrs = res.addrs;
>  		cnt = res.cnt;
>  	}
> @@ -10511,12 +10594,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>  		goto error;
>  	}
>  	link->fd = link_fd;
> -	free(res.addrs);
> +	kprobe_multi_resolve_free(&res);
>  	return link;
>  
>  error:
>  	free(link);
> -	free(res.addrs);
> +	kprobe_multi_resolve_free(&res);
>  	return libbpf_err_ptr(err);
>  }
>  
> -- 
> 2.25.1
> 

Re: [PATCH v5] libbpf: kprobe.multi: Filter with available_filter_functions

[Jackie Liu]

在 2023/5/26 17:53, Jiri Olsa 写道:
> On Fri, May 26, 2023 at 10:10:47AM +0800, Jackie Liu wrote:
> 
> SNIP
> 
>> -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>> -			const char *sym_name, void *ctx)
>> +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>> +				 const char *sym_name, void *ctx)
>>   {
>>   	struct kprobe_multi_resolve *res = ctx;
>>   	int err;
>> @@ -10431,8 +10438,8 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>>   	if (!glob_match(sym_name, res->pattern))
>>   		return 0;
>>   
>> -	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
>> -				res->cnt + 1);
>> +	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap,
>> +				sizeof(unsigned long), res->cnt + 1);
> 
> hum, looks like this is just formatting change, AFAICS we don't need that
> 
>>   	if (err)
>>   		return err;
>>   
>> @@ -10440,6 +10447,75 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>>   	return 0;
>>   }
>>   
> 
> SNIP
> 
>> +
>> +static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res)
>> +{
>> +	while (res->syms && res->cnt)
>> +		free((char *)res->syms[--res->cnt]);
>> +
>> +	free(res->syms);
>> +	free(res->addrs);
> 
> we should set cnt and cap to zero for the fallback sake

It is necessary to set cap to 0, cnt is already 0, if syms exists.

> 
>> +}
>> +
>>   struct bpf_link *
>>   bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>>   				      const char *pattern,
>> @@ -10476,13 +10552,20 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>>   		return libbpf_err_ptr(-EINVAL);
>>   
>>   	if (pattern) {
>> -		err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
>> -		if (err)
>> -			goto error;
>> +		err = libbpf_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
>> +						     &res);
>> +		if (err) {
>> +			/* fallback to kallsyms */
> 
> we need to call kprobe_multi_resolve_free in here and set
> cnt/cap to zero in kprobe_multi_resolve_free

Yes.

> 
> jirka
> 
>> +			err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
>> +						    &res);
>> +			if (err)
>> +				goto error;
>> +		}
>>   		if (!res.cnt) {
>>   			err = -ENOENT;
>>   			goto error;
>>   		}
>> +		syms = res.syms;
>>   		addrs = res.addrs;
>>   		cnt = res.cnt;
>>   	}
>> @@ -10511,12 +10594,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>>   		goto error;
>>   	}
>>   	link->fd = link_fd;
>> -	free(res.addrs);
>> +	kprobe_multi_resolve_free(&res);
>>   	return link;
>>   
>>   error:
>>   	free(link);
>> -	free(res.addrs);
>> +	kprobe_multi_resolve_free(&res);
>>   	return libbpf_err_ptr(err);
>>   }
>>   
>> -- 
>> 2.25.1
>>

[PATCH v2] libbpf: kprobe.multi: Filter with available_filter_functions

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

When using regular expression matching with "kprobe multi", it scans all
the functions under "/proc/kallsyms" that can be matched. However, not all
of them can be traced by kprobe.multi. If any one of the functions fails
to be traced, it will result in the failure of all functions. The best
approach is to filter out the functions that cannot be traced to ensure
proper tracking of the functions.

Check available_filter_functions first, speed up for function check than
/proc/kallsyms. since each function needs to check kallsyms and
available_filter_functions, its startup time will increase. The function
implementation is referenced from BCC's kprobe_exists().

Here is the test eBPF program [1].
[1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867

Suggested-by: Jiri Olsa <olsajiri@gmail.com>
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 v1->v2: speed up startup time.
	 Before: 0.27s user 5.09s system 99% cpu 5.392 total
	 After : 0.37s user 1.54s system 98% cpu 1.947 total

 tools/lib/bpf/libbpf.c          | 100 +++++++++++++++++++++++++++++++-
 tools/lib/bpf/libbpf_internal.h |   4 +-
 2 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ad1ec893b41b..0380d171c1cd 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10422,8 +10422,8 @@ struct kprobe_multi_resolve {
 };
 
 static int
-resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
-			const char *sym_name, void *ctx)
+kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
+				 const char *sym_name, void *ctx)
 {
 	struct kprobe_multi_resolve *res = ctx;
 	int err;
@@ -10440,6 +10440,99 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
 	return 0;
 }
 
+static int
+resolve_function_addrs(const char *name, unsigned long long *sym_addr)
+{
+	char sym_name[500];
+	int ret, err = 0;
+	FILE *f;
+
+	f = fopen("/proc/kallsyms", "r");
+	if (!f) {
+		err = -errno;
+		pr_warn("failed to open /proc/kallsyms: %d\n", err);
+		return err;
+	}
+
+	while (true) {
+		ret = fscanf(f, "%llx %*c %499s%*[^\n]\n",
+			     sym_addr, sym_name);
+		if (ret == EOF && feof(f)) {
+			pr_warn("not found syms in /proc/kallsyms\n");
+			err = -ENOENT;
+			break;
+		}
+		if (ret != 2) {
+			pr_warn("failed to read kallsyms entry: %d\n", ret);
+			err = -EINVAL;
+			break;
+		}
+
+		if (strcmp(name, sym_name) == 0)
+			return 0;
+	}
+
+	return err;
+}
+
+static int resolve_kprobe_multi_cb(const char *sym_name, void *ctx)
+{
+	unsigned long long sym_addr;
+	struct kprobe_multi_resolve *res = ctx;
+	int err;
+
+	if (!glob_match(sym_name, res->pattern))
+		return 0;
+
+	err = resolve_function_addrs(sym_name, &sym_addr);
+	if (err)
+		return err;
+
+	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
+				res->cnt + 1);
+	if (err)
+		return err;
+
+	res->addrs[res->cnt++] = (unsigned long) sym_addr;
+	return 0;
+}
+
+int libbpf_available_filter_functions_parse(available_filter_functions_cb_t cb,
+					    void *ctx)
+{
+	char sym_name[256];
+	FILE *f;
+	int ret, err = 0;
+
+	f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
+	if (!f) {
+		pr_warn("failed to open /sys/kernel/debug/tracing/available_filter_functions, ");
+		pr_warn("fallback to /proc/kallsyms.\n");
+		goto fallback;
+	}
+
+	while (true) {
+		ret = fscanf(f, "%s%*[^\n]\n", sym_name);
+		if (ret == EOF && feof(f))
+			break;
+		if (ret != 1) {
+			pr_warn("failed to read available_filter_functions entry: %d\n",
+				ret);
+			break;
+		}
+
+		err = cb(sym_name, ctx);
+		if (err)
+			return err;
+	}
+
+	fclose(f);
+	return err;
+
+fallback:
+	return libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb, ctx);
+}
+
 struct bpf_link *
 bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 				      const char *pattern,
@@ -10476,7 +10569,8 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 		return libbpf_err_ptr(-EINVAL);
 
 	if (pattern) {
-		err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
+		err = libbpf_available_filter_functions_parse(resolve_kprobe_multi_cb,
+							      &res);
 		if (err)
 			goto error;
 		if (!res.cnt) {
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index e4d05662a96c..fdf6b464481f 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -481,8 +481,10 @@ __s32 btf__find_by_name_kind_own(const struct btf *btf, const char *type_name,
 
 typedef int (*kallsyms_cb_t)(unsigned long long sym_addr, char sym_type,
 			     const char *sym_name, void *ctx);
-
 int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *arg);
+typedef int (*available_filter_functions_cb_t)(const char *sym_name, void *ctx);
+int libbpf_available_filter_functions_parse(available_filter_functions_cb_t cb,
+					    void *arg);
 
 /* handle direct returned errors */
 static inline int libbpf_err(int ret)
-- 
2.25.1

[PATCH v5] libbpf: kprobe.multi: Filter with available_filter_functions

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

When using regular expression matching with "kprobe multi", it scans all
the functions under "/proc/kallsyms" that can be matched. However, not all
of them can be traced by kprobe.multi. If any one of the functions fails
to be traced, it will result in the failure of all functions. The best
approach is to filter out the functions that cannot be traced to ensure
proper tracking of the functions.

Use available_filter_functions check first, if failed, fallback to
kallsyms.

Here is the test eBPF program [1].
[1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867

Suggested-by: Jiri Olsa <olsajiri@gmail.com>
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 tools/lib/bpf/libbpf.c | 100 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 93 insertions(+), 7 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ad1ec893b41b..0914b7e98e30 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10106,6 +10106,12 @@ static const char *tracefs_uprobe_events(void)
 	return use_debugfs() ? DEBUGFS"/uprobe_events" : TRACEFS"/uprobe_events";
 }
 
+static const char *tracefs_available_filter_functions(void)
+{
+	return use_debugfs() ? DEBUGFS"/available_filter_functions" :
+			       TRACEFS"/available_filter_functions";
+}
+
 static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
 					 const char *kfunc_name, size_t offset)
 {
@@ -10417,13 +10423,14 @@ static bool glob_match(const char *str, const char *pat)
 struct kprobe_multi_resolve {
 	const char *pattern;
 	unsigned long *addrs;
+	const char **syms;
 	size_t cap;
 	size_t cnt;
 };
 
 static int
-resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
-			const char *sym_name, void *ctx)
+kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
+				 const char *sym_name, void *ctx)
 {
 	struct kprobe_multi_resolve *res = ctx;
 	int err;
@@ -10440,6 +10447,77 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
 	return 0;
 }
 
+static int ftrace_resolve_kprobe_multi_cb(const char *sym_name, void *ctx)
+{
+	struct kprobe_multi_resolve *res = ctx;
+	int err;
+	char *name;
+
+	if (!glob_match(sym_name, res->pattern))
+		return 0;
+
+	err = libbpf_ensure_mem((void **) &res->syms, &res->cap,
+				sizeof(const char *), res->cnt + 1);
+	if (err)
+		return err;
+
+	name = strdup(sym_name);
+	if (!name)
+		return -errno;
+
+	res->syms[res->cnt++] = name;
+	return 0;
+}
+
+typedef int (*available_kprobe_cb_t)(const char *sym_name, void *ctx);
+
+static int
+libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
+{
+	char sym_name[256];
+	FILE *f;
+	int ret, err = 0;
+	const char *available_path = tracefs_available_filter_functions();
+
+	f = fopen(available_path, "r");
+	if (!f) {
+		err = -errno;
+		pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
+			available_path);
+		return err;
+	}
+
+	while (true) {
+		ret = fscanf(f, "%255s%*[^\n]\n", sym_name);
+		if (ret == EOF && feof(f))
+			break;
+		if (ret != 1) {
+			pr_warn("failed to read available kprobe entry: %d\n",
+				ret);
+			err = -EINVAL;
+			break;
+		}
+
+		err = cb(sym_name, ctx);
+		if (err)
+			break;
+	}
+
+	fclose(f);
+	return err;
+}
+
+static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res)
+{
+	while (res->syms && res->cnt)
+		free((char *)res->syms[--res->cnt]);
+
+	free(res->syms);
+	free(res->addrs);
+	/* reset cap to zero, when fallback */
+	res->cap = 0;
+}
+
 struct bpf_link *
 bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 				      const char *pattern,
@@ -10476,13 +10554,21 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 		return libbpf_err_ptr(-EINVAL);
 
 	if (pattern) {
-		err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
-		if (err)
-			goto error;
+		err = libbpf_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
+						     &res);
+		if (err) {
+			/* fallback to kallsyms */
+			kprobe_multi_resolve_free(&res);
+			err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
+						    &res);
+			if (err)
+				goto error;
+		}
 		if (!res.cnt) {
 			err = -ENOENT;
 			goto error;
 		}
+		syms = res.syms;
 		addrs = res.addrs;
 		cnt = res.cnt;
 	}
@@ -10511,12 +10597,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 		goto error;
 	}
 	link->fd = link_fd;
-	free(res.addrs);
+	kprobe_multi_resolve_free(&res);
 	return link;
 
 error:
 	free(link);
-	free(res.addrs);
+	kprobe_multi_resolve_free(&res);
 	return libbpf_err_ptr(err);
 }
 
-- 
2.25.1