Both of these were pointed out on the list against an unrelated hist
patch [1], and both are the same shape as commit 6f86bdeab633 ("tracing:
Fix bad hist from corrupting named_triggers list"): a trigger that is on
the global named_triggers list gets freed without being taken off it, and
the next lookup by name reads the freed object.

Link: 
https://lore.kernel.org/linux-trace-kernel/[email protected]/
 [1]

Patch 1 is the init path. event_hist_trigger_named_init() saves the
trigger under the name before it takes the reference that can fail, and
does not undo the save.

Patch 2 is the enable path, the one case that fix did not reach.
hist_trigger_enable() removes the trigger from file->triggers on
failure, which is the only handle hist_unregister_trigger() has, so the
->free() that unlinks it never runs. Leaving it on the list lets the
existing unregister do the whole unwind, which also stops out_free from
touching a hist_data that hist_register_trigger() already destroyed.

The two are independent and can be taken separately.

Reproduced on x86_64 under KASAN_INLINE by making the two failures fire
from a debug knob, since neither is reachable without an allocation
failure or an event that refuses to register. Same kernel and initramfs
either way, one boot per case:

                                        v7.3-rc2        patched
  control, no injected failure          clean           clean
  1) nested init fails                  KASAN           clean
  2) enable fails, new named trigger    KASAN           clean
  3) enable fails, shared histogram     KASAN + panic   clean

Case 3 reports in remove_hist_vars() rather than find_named_trigger(),
from the out_free fall-through described in patch 2.

After each injected failure the same name is written again and is
accepted on the patched kernel, so the trigger really did leave the list;
on v7.3-rc2 case 2 rejects it.

selftests/ftrace test.d/trigger before and after: 45 results, identical
item by item (32 passed, 3 failed, 2 unresolved, 8 unsupported). The
failures and the unresolved results are there without the patches too.

Donggeun Yoo (2):
  tracing: hist: take the reference before publishing the named trigger
  tracing: hist: undo the registration when enabling the trigger fails

 kernel/trace/trace_events_hist.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)


base-commit: df2908090cda368b01ff43709f51890076c56157
-- 
2.53.0


Reply via email to