[PATCH 4.19.y v3 0/2] Fix stable-4.19 use-after-free bug

[PATCH 4.19.y v3 0/2] Fix stable-4.19 use-after-free bug

[George Guo]

1. About v3-0001-tracing-Remove-unnecessary-hist_data-destroy-in-d.patch:

The reason I write the changelog by myself is that no one found the bug
at that time, then later the code was removed on upstream, but
4.19-stable has the bug.

2. About v3-0002-tracing-Remove-unnecessary-var-destroy-in-onmax_d.patch

I also write the changelog by myself is that the upstream api is changed.
refs commits:
466f4528fbc6 ("tracing: Generalize hist trigger onmax and save action")
ff9d31d0d466 ("tracing: Remove unnecessary var_ref destroy in track_data_destroy()")

George Guo (2):
  tracing: Remove unnecessary hist_data destroy in
    destroy_synth_var_refs()
  tracing: Remove unnecessary var destroy in onmax_destroy()

 kernel/trace/trace_events_hist.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

-- 
2.34.1

[PATCH 4.19.y v3 1/2] tracing: Remove unnecessary hist_data destroy in destroy_synth_var_refs()

[George Guo]

From: George Guo <guodongtai@kylinos.cn>

commit 912201345f7c39e6b0ac283207be2b6641fa47b9 upstream.

The destroy_synth_var_refs() destroyed hist_data, casusing a double-free
error flagged by KASAN.

This is tested via "./ftracetest test.d/trigger/inter-event/
trigger-field-variable-support.tc"

==================================================================
BUG: KASAN: use-after-free in destroy_hist_field+0x115/0x140
Read of size 4 at addr ffff888012e95318 by task ftracetest/1858

CPU: 1 PID: 1858 Comm: ftracetest Kdump: loaded Tainted: GE 4.19.90-89 #24
Source Version: Unknown
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0
Call Trace:
 dump_stack+0xcb/0x10b
 print_address_description.cold+0x54/0x249
 kasan_report_error.cold+0x63/0xab
 ? destroy_hist_field+0x115/0x140
 __asan_report_load4_noabort+0x8d/0xa0
 ? destroy_hist_field+0x115/0x140
 destroy_hist_field+0x115/0x140
 destroy_hist_data+0x4e4/0x9a0
 event_hist_trigger_free+0x212/0x2f0
 ? update_cond_flag+0x128/0x170
 ? event_hist_trigger_func+0x2880/0x2880
 hist_unregister_trigger+0x2f2/0x4f0
 event_hist_trigger_func+0x168c/0x2880
 ? tracing_map_read_var_once+0xd0/0xd0
 ? create_key_field+0x520/0x520
 ? __mutex_lock_slowpath+0x10/0x10
 event_trigger_write+0x2f4/0x490
 ? trigger_start+0x180/0x180
 ? __fget_light+0x369/0x5d0
 ? count_memcg_event_mm+0x104/0x2b0
 ? trigger_start+0x180/0x180
 __vfs_write+0x81/0x100
 vfs_write+0x1e1/0x540
 ksys_write+0x12a/0x290
 ? __ia32_sys_read+0xb0/0xb0
 ? __close_fd+0x1d3/0x280
 do_syscall_64+0xe3/0x2d0
 entry_SYSCALL_64_after_hwframe+0x5c/0xc1
RIP: 0033:0x7efdd342ee04
Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f 80 00 00 00 00 48
8d 05 39 34 0c 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff
ff 77 54 f3 c3 66 90 41 54 55 49 89 d4 53 48 89 f5
RSP: 002b:00007ffda01f5e08 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00000000000000b4 RCX: 00007efdd342ee04
RDX: 00000000000000b4 RSI: 000055c5b41b1e90 RDI: 0000000000000001
RBP: 000055c5b41b1e90 R08: 000000000000000a R09: 0000000000000000
R10: 000000000000000a R11: 0000000000000246 R12: 00007efdd34ed5c0
R13: 00000000000000b4 R14: 00007efdd34ed7c0 R15: 00000000000000b4
==================================================================

So remove the destroy_synth_var_refs() call for that hist_data.

Fixes: c282a386a397 ("tracing: Add 'onmatch' hist trigger action support")
Signed-off-by: George Guo <guodongtai@kylinos.cn>
---
 kernel/trace/trace_events_hist.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index e004daf8cad5..7dcb96305e56 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -280,8 +280,6 @@ struct hist_trigger_data {
 	struct action_data		*actions[HIST_ACTIONS_MAX];
 	unsigned int			n_actions;
 
-	struct hist_field               *synth_var_refs[SYNTH_FIELDS_MAX];
-	unsigned int                    n_synth_var_refs;
 	struct field_var		*field_vars[SYNTH_FIELDS_MAX];
 	unsigned int			n_field_vars;
 	unsigned int			n_field_var_str;
@@ -1363,8 +1361,8 @@ static struct hist_field *find_var_ref(struct hist_trigger_data *hist_data,
 			return found;
 	}
 
-	for (i = 0; i < hist_data->n_synth_var_refs; i++) {
-		hist_field = hist_data->synth_var_refs[i];
+	for (i = 0; i < hist_data->n_var_refs; i++) {
+		hist_field = hist_data->var_refs[i];
 		found = check_field_for_var_refs(hist_data, hist_field,
 						 var_data, var_idx, 0);
 		if (found)
@@ -3707,21 +3705,6 @@ static void save_field_var(struct hist_trigger_data *hist_data,
 		hist_data->n_field_var_str++;
 }
 
-
-static void destroy_synth_var_refs(struct hist_trigger_data *hist_data)
-{
-	unsigned int i;
-
-	for (i = 0; i < hist_data->n_synth_var_refs; i++)
-		destroy_hist_field(hist_data->synth_var_refs[i], 0);
-}
-
-static void save_synth_var_ref(struct hist_trigger_data *hist_data,
-			 struct hist_field *var_ref)
-{
-	hist_data->synth_var_refs[hist_data->n_synth_var_refs++] = var_ref;
-}
-
 static int check_synth_field(struct synth_event *event,
 			     struct hist_field *hist_field,
 			     unsigned int field_pos)
@@ -3884,7 +3867,6 @@ static int onmatch_create(struct hist_trigger_data *hist_data,
 				goto err;
 			}
 
-			save_synth_var_ref(hist_data, var_ref);
 			field_pos++;
 			kfree(p);
 			continue;
@@ -4631,7 +4613,6 @@ static void destroy_hist_data(struct hist_trigger_data *hist_data)
 	destroy_actions(hist_data);
 	destroy_field_vars(hist_data);
 	destroy_field_var_hists(hist_data);
-	destroy_synth_var_refs(hist_data);
 
 	kfree(hist_data);
 }
-- 
2.34.1

[PATCH 4.19.y v3 2/2] tracing: Remove unnecessary var destroy in onmax_destroy()

[George Guo]

From: George Guo <guodongtai@kylinos.cn>

commit ff9d31d0d46672e201fc9ff59c42f1eef5f00c77 upstream.

The onmax_destroy() destroyed the onmax var, casusing a double-free error
flagged by KASAN.

This is tested via "./ftracetest test.d/trigger/inter-event/
trigger-onmatch-onmax-action-hist.tc".

==================================================================
BUG: KASAN: use-after-free in destroy_hist_field+0x1c2/0x200
Read of size 8 at addr ffff88800a4ad100 by task ftracetest/4731

CPU: 0 PID: 4731 Comm: ftracetest Kdump: loaded Tainted: GE 4.19.90-89 #77
Source Version: Unknown
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0
Call Trace:
 dump_stack+0xcb/0x10b
 print_address_description.cold+0x54/0x249
 kasan_report_error.cold+0x63/0xab
 ? destroy_hist_field+0x1c2/0x200
 ? hist_trigger_elt_data_alloc+0x5a0/0x5a0
 __asan_report_load8_noabort+0x8d/0xa0
 ? destroy_hist_field+0x1c2/0x200
 destroy_hist_field+0x1c2/0x200
 onmax_destroy+0x72/0x1e0
 ? hist_trigger_elt_data_alloc+0x5a0/0x5a0
 destroy_hist_data+0x236/0xa40
 event_hist_trigger_free+0x212/0x2f0
 ? update_cond_flag+0x128/0x170
 ? event_hist_trigger_func+0x2880/0x2880
 hist_unregister_trigger+0x2f2/0x4f0
 event_hist_trigger_func+0x168c/0x2880
 ? tracing_map_cmp_u64+0xa0/0xa0
 ? onmatch_create.constprop.0+0xf50/0xf50
 ? __mutex_lock_slowpath+0x10/0x10
 event_trigger_write+0x2f4/0x490
 ? trigger_start+0x180/0x180
 ? __fget_light+0x369/0x5d0
 ? count_memcg_event_mm+0x104/0x2b0
 ? trigger_start+0x180/0x180
 __vfs_write+0x81/0x100
 vfs_write+0x1e1/0x540
 ksys_write+0x12a/0x290
 ? __ia32_sys_read+0xb0/0xb0
 ? __close_fd+0x1d3/0x280
 do_syscall_64+0xe3/0x2d0
 entry_SYSCALL_64_after_hwframe+0x5c/0xc1
RIP: 0033:0x7fd7f4c44e04
Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f 80 00 00 00 00
48 8d 05 39 34 0c 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00
f0 ff ff 77 54 f3 c3 66 90 41 54 55 49 89 d4 53 48 89 f5
RSP: 002b:00007fff10370df8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 000000000000010f RCX: 00007fd7f4c44e04
RDX: 000000000000010f RSI: 000055fa765df650 RDI: 0000000000000001
RBP: 000055fa765df650 R08: 000000000000000a R09: 0000000000000000
R10: 000000000000000a R11: 0000000000000246 R12: 00007fd7f4d035c0
R13: 000000000000010f R14: 00007fd7f4d037c0 R15: 000000000000010f
==================================================================

So remove the onmax_destroy() destroy_hist_field() call for that var.

Fixes: 50450603ec9c ("tracing: Add 'onmax' hist trigger action support")
Signed-off-by: George Guo <guodongtai@kylinos.cn>
---
 kernel/trace/trace_events_hist.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 7dcb96305e56..58b8a2575b8c 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -337,7 +337,6 @@ struct action_data {
 			char			*fn_name;
 			unsigned int		max_var_ref_idx;
 			struct hist_field	*max_var;
-			struct hist_field	*var;
 		} onmax;
 	};
 };
@@ -3489,7 +3488,6 @@ static void onmax_destroy(struct action_data *data)
 	unsigned int i;
 
 	destroy_hist_field(data->onmax.max_var, 0);
-	destroy_hist_field(data->onmax.var, 0);
 
 	kfree(data->onmax.var_str);
 	kfree(data->onmax.fn_name);
@@ -3528,8 +3526,6 @@ static int onmax_create(struct hist_trigger_data *hist_data,
 	if (!ref_field)
 		return -ENOMEM;
 
-	data->onmax.var = ref_field;
-
 	data->fn = onmax_save;
 	data->onmax.max_var_ref_idx = var_ref_idx;
 	max_var = create_var(hist_data, file, "max", sizeof(u64), "u64");
-- 
2.34.1

Re: [PATCH 4.19.y v3 1/2] tracing: Remove unnecessary hist_data destroy in destroy_synth_var_refs()

[Greg KH]

On Tue, Apr 09, 2024 at 10:10:37AM +0800, George Guo wrote:
> From: George Guo <guodongtai@kylinos.cn>

No, you are not the author of this :(

> commit 912201345f7c39e6b0ac283207be2b6641fa47b9 upstream.



> 
> The destroy_synth_var_refs() destroyed hist_data, casusing a double-free
> error flagged by KASAN.
> 
> This is tested via "./ftracetest test.d/trigger/inter-event/
> trigger-field-variable-support.tc"
> 
> ==================================================================
> BUG: KASAN: use-after-free in destroy_hist_field+0x115/0x140
> Read of size 4 at addr ffff888012e95318 by task ftracetest/1858
> 
> CPU: 1 PID: 1858 Comm: ftracetest Kdump: loaded Tainted: GE 4.19.90-89 #24
> Source Version: Unknown
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0
> Call Trace:
>  dump_stack+0xcb/0x10b
>  print_address_description.cold+0x54/0x249
>  kasan_report_error.cold+0x63/0xab
>  ? destroy_hist_field+0x115/0x140
>  __asan_report_load4_noabort+0x8d/0xa0
>  ? destroy_hist_field+0x115/0x140
>  destroy_hist_field+0x115/0x140
>  destroy_hist_data+0x4e4/0x9a0
>  event_hist_trigger_free+0x212/0x2f0
>  ? update_cond_flag+0x128/0x170
>  ? event_hist_trigger_func+0x2880/0x2880
>  hist_unregister_trigger+0x2f2/0x4f0
>  event_hist_trigger_func+0x168c/0x2880
>  ? tracing_map_read_var_once+0xd0/0xd0
>  ? create_key_field+0x520/0x520
>  ? __mutex_lock_slowpath+0x10/0x10
>  event_trigger_write+0x2f4/0x490
>  ? trigger_start+0x180/0x180
>  ? __fget_light+0x369/0x5d0
>  ? count_memcg_event_mm+0x104/0x2b0
>  ? trigger_start+0x180/0x180
>  __vfs_write+0x81/0x100
>  vfs_write+0x1e1/0x540
>  ksys_write+0x12a/0x290
>  ? __ia32_sys_read+0xb0/0xb0
>  ? __close_fd+0x1d3/0x280
>  do_syscall_64+0xe3/0x2d0
>  entry_SYSCALL_64_after_hwframe+0x5c/0xc1
> RIP: 0033:0x7efdd342ee04
> Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f 80 00 00 00 00 48
> 8d 05 39 34 0c 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff
> ff 77 54 f3 c3 66 90 41 54 55 49 89 d4 53 48 89 f5
> RSP: 002b:00007ffda01f5e08 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 00000000000000b4 RCX: 00007efdd342ee04
> RDX: 00000000000000b4 RSI: 000055c5b41b1e90 RDI: 0000000000000001
> RBP: 000055c5b41b1e90 R08: 000000000000000a R09: 0000000000000000
> R10: 000000000000000a R11: 0000000000000246 R12: 00007efdd34ed5c0
> R13: 00000000000000b4 R14: 00007efdd34ed7c0 R15: 00000000000000b4
> ==================================================================
> 
> So remove the destroy_synth_var_refs() call for that hist_data.

That is not the commit log for the referenced commit.

> 
> Fixes: c282a386a397 ("tracing: Add 'onmatch' hist trigger action support")
> Signed-off-by: George Guo <guodongtai@kylinos.cn>

And you dropped all of the people on the original commit :(

Please fix all of this up if you wish to have these backports applied.

thanks,

greg k-h