[PATCH] perf script: perf script tests fails with segfault

[PATCH] perf script: perf script tests fails with segfault

[Aditya Bodkhe]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=yes, Size: 2507 bytes --]

perf script: pert script tests fails with segmentation fault as below:

1. Run perf test -vvv 'perf script tests'

	 92: perf script tests:
	--- start ---
	test child forked, pid 103769
	DB test
	[ perf record: Woken up 1 times to write data ]
	[ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
	/usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
	--- Cleaning up ---
	---- end(-1) ----
 	92: perf script tests                                               : FAILED!

Backtrace pointed to :
	#0  0x0000000010247dd0 in maps.machine ()
	#1  0x00000000101d178c in db_export.sample ()
	#2  0x00000000103412c8 in python_process_event ()
	#3  0x000000001004eb28 in process_sample_event ()
	#4  0x000000001024fcd0 in machines.deliver_event ()
	#5  0x000000001025005c in perf_session.deliver_event ()
	#6  0x00000000102568b0 in __ordered_events__flush.part.0 ()
	#7  0x0000000010251618 in perf_session.process_events ()
	#8  0x0000000010053620 in cmd_script ()
	#9  0x00000000100b5a28 in run_builtin ()
	#10 0x00000000100b5f94 in handle_internal_command ()
	#11 0x0000000010011114 in main ()

Further investigation reveals that this occurs in the `perf script tests`,
because it uses `db_test.py` script. This script sets `perf_db_export_mode = True`.

With `perf_db_export_mode` enabled, if a sample originates from a hypervisor,
perf doesn't set maps for “[H]” sample in the code. Consequently, `al->maps` remains NULL
when `maps__machine(al->maps)` is called from `db_export__sample`.

To prevent this NULL pointer dereference, add a check for `al->maps == NULL`
before calling `maps__machine()`. If `al->maps` is NULL, return `-1` to avoid
the segmentation fault.

Reported-by: Disha Goel <Disha.Goel@ibm.com>
Signed-off-by: Aditya Bodkhe <aditya.b1@linux.ibm.com>
---
 tools/perf/util/db-export.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
index 50f916374d87..f355878a8c82 100644
--- a/tools/perf/util/db-export.c
+++ b/tools/perf/util/db-export.c
@@ -365,6 +365,11 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
        if (err)
                return err;

+       if (!al->maps) {
+          err = -1;
+          goto out_put;
+       }
+
        machine = maps__machine(al->maps);
        err = db_export__machine(dbe, machine);
        if (err)
-- 
2.43.0

Re: [PATCH] perf script: perf script tests fails with segfault

[Namhyung Kim]

CC-ing Adrian,

On Thu, Mar 20, 2025 at 02:45:51PM +0530, Aditya Bodkhe wrote:
> perf script: pert script tests fails with segmentation fault as below:
> 
> 1. Run perf test -vvv 'perf script tests'
> 
> 	 92: perf script tests:
> 	--- start ---
> 	test child forked, pid 103769
> 	DB test
> 	[ perf record: Woken up 1 times to write data ]
> 	[ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
> 	/usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
> 	--- Cleaning up ---
> 	---- end(-1) ----
>  	92: perf script tests                                               : FAILED!
> 
> Backtrace pointed to :
> 	#0  0x0000000010247dd0 in maps.machine ()
> 	#1  0x00000000101d178c in db_export.sample ()
> 	#2  0x00000000103412c8 in python_process_event ()
> 	#3  0x000000001004eb28 in process_sample_event ()
> 	#4  0x000000001024fcd0 in machines.deliver_event ()
> 	#5  0x000000001025005c in perf_session.deliver_event ()
> 	#6  0x00000000102568b0 in __ordered_events__flush.part.0 ()
> 	#7  0x0000000010251618 in perf_session.process_events ()
> 	#8  0x0000000010053620 in cmd_script ()
> 	#9  0x00000000100b5a28 in run_builtin ()
> 	#10 0x00000000100b5f94 in handle_internal_command ()
> 	#11 0x0000000010011114 in main ()
> 
> Further investigation reveals that this occurs in the `perf script tests`,
> because it uses `db_test.py` script. This script sets `perf_db_export_mode = True`.
> 
> With `perf_db_export_mode` enabled, if a sample originates from a hypervisor,
> perf doesn't set maps for “[H]” sample in the code. Consequently, `al->maps` remains NULL
> when `maps__machine(al->maps)` is called from `db_export__sample`.
> 
> To prevent this NULL pointer dereference, add a check for `al->maps == NULL`
> before calling `maps__machine()`. If `al->maps` is NULL, return `-1` to avoid
> the segmentation fault.
> 
> Reported-by: Disha Goel <Disha.Goel@ibm.com>
> Signed-off-by: Aditya Bodkhe <aditya.b1@linux.ibm.com>
> ---
>  tools/perf/util/db-export.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
> index 50f916374d87..f355878a8c82 100644
> --- a/tools/perf/util/db-export.c
> +++ b/tools/perf/util/db-export.c
> @@ -365,6 +365,11 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
>         if (err)
>                 return err;
> 
> +       if (!al->maps) {
> +          err = -1;
> +          goto out_put;
> +       }

Maybe better to check it before db_export__evsel().  Also it seems it
should not goto out_put as it doesn't get the main_thread yet.

Thanks,
Namhyung

> +
>         machine = maps__machine(al->maps);
>         err = db_export__machine(dbe, machine);
>         if (err)
> -- 
> 2.43.0
> 

Re: [PATCH] perf script: perf script tests fails with segfault

[Adrian Hunter]

On 21/03/25 08:04, Namhyung Kim wrote:
> CC-ing Adrian,

Thanks!

> 
> On Thu, Mar 20, 2025 at 02:45:51PM +0530, Aditya Bodkhe wrote:
>> perf script: pert script tests fails with segmentation fault as below:
>>
>> 1. Run perf test -vvv 'perf script tests'
>>
>> 	 92: perf script tests:
>> 	--- start ---
>> 	test child forked, pid 103769
>> 	DB test
>> 	[ perf record: Woken up 1 times to write data ]
>> 	[ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
>> 	/usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
>> 	--- Cleaning up ---
>> 	---- end(-1) ----
>>  	92: perf script tests                                               : FAILED!
>>
>> Backtrace pointed to :
>> 	#0  0x0000000010247dd0 in maps.machine ()
>> 	#1  0x00000000101d178c in db_export.sample ()
>> 	#2  0x00000000103412c8 in python_process_event ()
>> 	#3  0x000000001004eb28 in process_sample_event ()
>> 	#4  0x000000001024fcd0 in machines.deliver_event ()
>> 	#5  0x000000001025005c in perf_session.deliver_event ()
>> 	#6  0x00000000102568b0 in __ordered_events__flush.part.0 ()
>> 	#7  0x0000000010251618 in perf_session.process_events ()
>> 	#8  0x0000000010053620 in cmd_script ()
>> 	#9  0x00000000100b5a28 in run_builtin ()
>> 	#10 0x00000000100b5f94 in handle_internal_command ()
>> 	#11 0x0000000010011114 in main ()
>>
>> Further investigation reveals that this occurs in the `perf script tests`,
>> because it uses `db_test.py` script. This script sets `perf_db_export_mode = True`.
>>
>> With `perf_db_export_mode` enabled, if a sample originates from a hypervisor,
>> perf doesn't set maps for “[H]” sample in the code. Consequently, `al->maps` remains NULL
>> when `maps__machine(al->maps)` is called from `db_export__sample`.
>>
>> To prevent this NULL pointer dereference, add a check for `al->maps == NULL`
>> before calling `maps__machine()`. If `al->maps` is NULL, return `-1` to avoid
>> the segmentation fault.
>>
>> Reported-by: Disha Goel <Disha.Goel@ibm.com>
>> Signed-off-by: Aditya Bodkhe <aditya.b1@linux.ibm.com>
>> ---
>>  tools/perf/util/db-export.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
>> index 50f916374d87..f355878a8c82 100644
>> --- a/tools/perf/util/db-export.c
>> +++ b/tools/perf/util/db-export.c
>> @@ -365,6 +365,11 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
>>         if (err)
>>                 return err;
>>
>> +       if (!al->maps) {
>> +          err = -1;
>> +          goto out_put;
>> +       }
> 
> Maybe better to check it before db_export__evsel().  Also it seems it
> should not goto out_put as it doesn't get the main_thread yet.

There has to be a machine to have found a thread.  I'd suggest
getting the machine from the thread, like this:

diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
index 50f916374d87..7ea6fd474c4c 100644
--- a/tools/perf/util/db-export.c
+++ b/tools/perf/util/db-export.c
@@ -181,7 +181,7 @@ static int db_ids_from_al(struct db_export *dbe, struct addr_location *al,
 	if (al->map) {
 		struct dso *dso = map__dso(al->map);
 
-		err = db_export__dso(dbe, dso, maps__machine(al->maps));
+		err = db_export__dso(dbe, dso, maps__machine(thread__maps(al->thread)));
 		if (err)
 			return err;
 		*dso_db_id = dso__db_id(dso);
@@ -358,14 +358,18 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
 	};
 	struct thread *main_thread;
 	struct comm *comm = NULL;
-	struct machine *machine;
+	struct machine *machine = NULL;
 	int err;
 
+	if (thread__maps(thread))
+		machine = maps__machine(thread__maps(thread));
+	if (!machine)
+		return -1;
+
 	err = db_export__evsel(dbe, evsel);
 	if (err)
 		return err;
 
-	machine = maps__machine(al->maps);
 	err = db_export__machine(dbe, machine);
 	if (err)
 		return err;
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 520729e78965..00f2c6c5114d 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -1306,7 +1306,7 @@ static void python_export_sample_table(struct db_export *dbe,
 
 	tuple_set_d64(t, 0, es->db_id);
 	tuple_set_d64(t, 1, es->evsel->db_id);
-	tuple_set_d64(t, 2, maps__machine(es->al->maps)->db_id);
+	tuple_set_d64(t, 2, maps__machine(thread__maps(es->al->thread))->db_id);
 	tuple_set_d64(t, 3, thread__db_id(es->al->thread));
 	tuple_set_d64(t, 4, es->comm_db_id);
 	tuple_set_d64(t, 5, es->dso_db_id);

Re: [PATCH] perf script: perf script tests fails with segfault

[Aditya Bodkhe]

Hi Adrian,

The approach you suggested works well in general, but we encountered a 
case where al->thread was NULL, which led to a segmentation fault. To 
prevent this, we need to ensure that al->thread is properly set before 
it's used in this line:

@@ -181,7 +181,7 @@ static int db_ids_from_al(struct db_export *dbe, 
struct addr_location *al,
      if (al->map) {
          struct dso *dso = map__dso(al->map);

-        err = db_export__dso(dbe, dso, maps__machine(al->maps));
+        err = db_export__dso(dbe, dso, 
maps__machine(thread__maps(al->thread)));

So, the following additional change is required to set al->thread:

--- a/tools/perf/util/db-export.c
+++ b/tools/perf/util/db-export.c
@@ -256,6 +256,7 @@ static struct call_path 
*call_path_from_sample(struct db_export *dbe,
                 al.map = map__get(node->ms.map);
                 al.maps = maps__get(thread__maps(thread));
                 al.addr = node->ip;
+               al.thread = thread__get(thread);

                 if (al.map && !al.sym)
                         al.sym = dso__find_symbol(map__dso(al.map), 
al.addr);

I will send a V2 with these changes
Sent again for replying to all

Thanks
Aditya

On 21/03/25 1:28 pm, Adrian Hunter wrote:
> On 21/03/25 08:04, Namhyung Kim wrote:
>> CC-ing Adrian,
> Thanks!
>
>> On Thu, Mar 20, 2025 at 02:45:51PM +0530, Aditya Bodkhe wrote:
>>> perf script: pert script tests fails with segmentation fault as below:
>>>
>>> 1. Run perf test -vvv 'perf script tests'
>>>
>>> 	 92: perf script tests:
>>> 	--- start ---
>>> 	test child forked, pid 103769
>>> 	DB test
>>> 	[ perf record: Woken up 1 times to write data ]
>>> 	[ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
>>> 	/usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
>>> 	--- Cleaning up ---
>>> 	---- end(-1) ----
>>>   	92: perf script tests                                               : FAILED!
>>>
>>> Backtrace pointed to :
>>> 	#0  0x0000000010247dd0 in maps.machine ()
>>> 	#1  0x00000000101d178c in db_export.sample ()
>>> 	#2  0x00000000103412c8 in python_process_event ()
>>> 	#3  0x000000001004eb28 in process_sample_event ()
>>> 	#4  0x000000001024fcd0 in machines.deliver_event ()
>>> 	#5  0x000000001025005c in perf_session.deliver_event ()
>>> 	#6  0x00000000102568b0 in __ordered_events__flush.part.0 ()
>>> 	#7  0x0000000010251618 in perf_session.process_events ()
>>> 	#8  0x0000000010053620 in cmd_script ()
>>> 	#9  0x00000000100b5a28 in run_builtin ()
>>> 	#10 0x00000000100b5f94 in handle_internal_command ()
>>> 	#11 0x0000000010011114 in main ()
>>>
>>> Further investigation reveals that this occurs in the `perf script tests`,
>>> because it uses `db_test.py` script. This script sets `perf_db_export_mode = True`.
>>>
>>> With `perf_db_export_mode` enabled, if a sample originates from a hypervisor,
>>> perf doesn't set maps for “[H]” sample in the code. Consequently, `al->maps` remains NULL
>>> when `maps__machine(al->maps)` is called from `db_export__sample`.
>>>
>>> To prevent this NULL pointer dereference, add a check for `al->maps == NULL`
>>> before calling `maps__machine()`. If `al->maps` is NULL, return `-1` to avoid
>>> the segmentation fault.
>>>
>>> Reported-by: Disha Goel <Disha.Goel@ibm.com>
>>> Signed-off-by: Aditya Bodkhe <aditya.b1@linux.ibm.com>
>>> ---
>>>   tools/perf/util/db-export.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
>>> index 50f916374d87..f355878a8c82 100644
>>> --- a/tools/perf/util/db-export.c
>>> +++ b/tools/perf/util/db-export.c
>>> @@ -365,6 +365,11 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
>>>          if (err)
>>>                  return err;
>>>
>>> +       if (!al->maps) {
>>> +          err = -1;
>>> +          goto out_put;
>>> +       }
>> Maybe better to check it before db_export__evsel().  Also it seems it
>> should not goto out_put as it doesn't get the main_thread yet.
> There has to be a machine to have found a thread.  I'd suggest
> getting the machine from the thread, like this:
>
> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
> index 50f916374d87..7ea6fd474c4c 100644
> --- a/tools/perf/util/db-export.c
> +++ b/tools/perf/util/db-export.c
> @@ -181,7 +181,7 @@ static int db_ids_from_al(struct db_export *dbe, struct addr_location *al,
>   	if (al->map) {
>   		struct dso *dso = map__dso(al->map);
>   
> -		err = db_export__dso(dbe, dso, maps__machine(al->maps));
> +		err = db_export__dso(dbe, dso, maps__machine(thread__maps(al->thread)));
>   		if (err)
>   			return err;
>   		*dso_db_id = dso__db_id(dso);
> @@ -358,14 +358,18 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
>   	};
>   	struct thread *main_thread;
>   	struct comm *comm = NULL;
> -	struct machine *machine;
> +	struct machine *machine = NULL;
>   	int err;
>   
> +	if (thread__maps(thread))
> +		machine = maps__machine(thread__maps(thread));
> +	if (!machine)
> +		return -1;
> +
>   	err = db_export__evsel(dbe, evsel);
>   	if (err)
>   		return err;
>   
> -	machine = maps__machine(al->maps);
>   	err = db_export__machine(dbe, machine);
>   	if (err)
>   		return err;
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index 520729e78965..00f2c6c5114d 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -1306,7 +1306,7 @@ static void python_export_sample_table(struct db_export *dbe,
>   
>   	tuple_set_d64(t, 0, es->db_id);
>   	tuple_set_d64(t, 1, es->evsel->db_id);
> -	tuple_set_d64(t, 2, maps__machine(es->al->maps)->db_id);
> +	tuple_set_d64(t, 2, maps__machine(thread__maps(es->al->thread))->db_id);
>   	tuple_set_d64(t, 3, thread__db_id(es->al->thread));
>   	tuple_set_d64(t, 4, es->comm_db_id);
>   	tuple_set_d64(t, 5, es->dso_db_id);
>
>
>
>