tracing: Fixes for v7.2

tracing fixes for v7.2:

- Fix NULL pointer dereference when matching unloaded module wildcard event

The set_event can take events for modules that have not been loaded yet. This is done by writing '<event>:mod:<module>'.

If '<event>' is not added, then it means to add all events in <module>. This wildcard is represented by a NULL pointer. If one were to try to remove the same module item with a named event it would cause a NULL pointer dereference when comparing the NULL with the name in strcmp().

echo ':mod:kvm' > /sys/kernel/tracing/set_event echo '!kvm_ack_irq:mod:kvm' >> /sys/kernel/tracing/set_event

The above will do a strcmp("kvm_ack_irq", NULL) and crash the kernel.

Test for NULL (wildcard) before doing the strcmp().

- Fix event data field race in loading two modules at the same time

When a module loads, its trace events get registered. The fields of the events are also dynamically created and added to the events fields list. It also will call a function that will look at all the events for updates that need to be done. If two modules load at the same time, the one that scans all events and their fields may read the one being added as the scan doesn't take the event_mutex. This may cause a data race.

Have the scan take the event_mutex to prevent the race.

git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/fixes

Head SHA1: c3730b8373bb5059d735509b9e6a00d7eb337d7c

Hui Su (1):
tracing: Fix NULL pointer dereference in module event cache removal

Michael Wu (1):
tracing: Fix race between update_event_fields and, event_define_fields

----

 kernel/trace/trace_events.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

From: Hui Su <[email protected]>

A module-only event filter such as ":mod:foo" is cached with a NULL event_mod->match when foo has not been loaded. If a later write tries to remove a specific match from the same module, remove_cache_mod() passes the NULL cached match to strcmp(), causing a NULL pointer dereference.

The issue can be reproduced from userspace:

echo ':mod:trace_events_kunit_missing' > /sys/kernel/tracing/set_event echo '!foo_bar:mod:trace_events_kunit_missing' >> /sys/kernel/tracing/set_event

The second write must be a concatenation (">>") to not include O_TRUNC as that would cause ftrace_clear_events() to clear the cached modules lines.

The crash was reproduced on x86_64 QEMU while KUnit workers contended on the event tracing path:

  BUG: kernel NULL pointer dereference, address: 0000000000000000
  #PF: supervisor read access in kernel mode
  RIP: 0010:strcmp+0x10/0x30
  Call Trace:
   __ftrace_set_clr_event_nolock+0x373/0x4a0
   ftrace_set_clr_event+0xf0/0x180
   ftrace_event_write+0xdf/0x110
   vfs_write+0xf6/0x440
   ksys_write+0x68/0xe0
   do_syscall_64+0xf9/0x540
   entry_SYSCALL_64_after_hwframe+0x77/0x7f

Check event_mod->match before comparing it, consistent with the existing NULL checks for the cached system and event fields. The mismatched removal continues to return -EINVAL; a broad cached module filter is removed with "!:mod:<module>".

---
 kernel/trace/trace_events.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index c01b10b99f67..032f741ba616 100644
@@ -945,7 +945,7 @@ static int remove_cache_mod(struct trace_array *tr, const char *mod,
 		if (strcmp(event_mod->module, mod) != 0)
 			continue;
 
-		if (match && strcmp(event_mod->match, match) != 0)
+		if (match && (!event_mod->match || strcmp(event_mod->match, match) != 0))
 			continue;
 
 		if (system &&

2.53.0

From: Michael Wu <[email protected]>

The following sequence may leads race between event_define_fields() and update_event_fields():

 CPU0 (loads module A)                      CPU1 (loads module B)
 ===============================            ===============================
 load_module(A)                             load_module(B)
   notifier_call_chain                        notifier_call_chain
     trace_module_notify                        trace_module_notify
       mutex_lock(&event_mutex)                   trace_event_update_all()
         trace_module_add_events(A)                 down_write(&trace_event_sem)
            __register_event(call_A)
              __add_event_to_tracers(call_A)
                event_define_fields(call_A)
                  for each f:                         list_for_each_entry(field,
                    list_add(&f->link,                                    &class->fields, link)
                             &class->fields)            field = class->fields->next;

Where access to the class->fields is not protected by the event_mutex in trace_event_update_all().

This produces the following panic:

   Unable to handle kernel access ... at virtual address 0000000000000018
   pc : update_event_fields+0xf8/0x368
   Call trace:
    update_event_fields+0xf8/0x368
    trace_event_update_all+0x7c/0x2b4
    trace_module_notify+0x4c/0x1dc
    notifier_call_chain+0x84/0x168
    blocking_notifier_call_chain_robust+0x64/0xd4
    load_module+0x10c8/0x123c
    __arm64_sys_finit_module+0x230/0x31c

Fix by taking event_mutex in trace_event_update_all() before trace_event_sem.

---
 kernel/trace/trace_events.c | 2 ++
 1 file changed, 2 insertions(+)

index 032f741ba616..3650d84d4f16 100644
@@ -3566,6 +3566,7 @@ void trace_event_update_all(struct trace_eval_map **map, int len)
 	int last_i;
 	int i;
 
+	mutex_lock(&event_mutex);
 	down_write(&trace_event_sem);
 	list_for_each_entry_safe(call, p, &ftrace_events, list) {
 		/* events are usually grouped together with systems */

@@ -3604,6 +3605,7 @@ void trace_event_update_all(struct trace_eval_map **map, int len)
 		cond_resched();
 	}
 	up_write(&trace_event_sem);
+	mutex_unlock(&event_mutex);
 }
 
 static bool event_in_systems(struct trace_event_call *call,

2.53.0