From 7f52f0d4228ee23b783e907258835d2f818350d7 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 1 Dec 2021 15:56:40 -0500 Subject: [PATCH] Fix: syscall tracing: missing trigger actions Observed issue ============== There is an issue when associating multiple actions to a single system call, where the first action is effectively added, but the following actions are silently ignored. Cause ===== The is caused by event creation of the 2nd event to be skipped because lttng_syscall_filter_enable returns "-EBUSY", because the bit is already set in the bitmap. Solution ======== Fixing this double-add-trigger scenario requires changes to how the lttng_syscall_filter_enable -EBUSY errors are dealt with on event creation, but the problem runs deeper. Given that many events may be responsible for requiring a bit to be set in the bitmap of the syscall filter, we cannot simply clear the bit whenever one event is disabled Otherwise, this will cause situations where all events for a given system call are disabled (due to the syscall filter bit being cleared) whenever one single event for that system call is disabled (e.g. 2 triggers on the same system call, but with different actions). We need to keep track of all events associated with this bit in the bitmap, and only clear the bit when the reference count reaches 0. This fixes the double-add-trigger issue by enturing that no -EBUSY is returned when creating the second event, and fixes the double-add-trigger-then-remove-trigger scenario as well. Known drawbacks =============== None. Signed-off-by: Mathieu Desnoyers Change-Id: I3c1e966e9720a2e8f4438ae2c98d83ffe318e424 --- src/lttng-syscalls.c | 43 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/src/lttng-syscalls.c b/src/lttng-syscalls.c index bf1f2b73..d618bd33 100644 --- a/src/lttng-syscalls.c +++ b/src/lttng-syscalls.c @@ -130,6 +130,15 @@ struct lttng_syscall_filter { DECLARE_BITMAP(sc_exit, NR_syscalls); DECLARE_BITMAP(sc_compat_entry, NR_compat_syscalls); DECLARE_BITMAP(sc_compat_exit, NR_compat_syscalls); + + /* + * Reference counters keeping track of number of events enabled + * for each bit. + */ + u32 sc_entry_refcount_map[NR_syscalls]; + u32 sc_exit_refcount_map[NR_syscalls]; + u32 sc_compat_entry_refcount_map[NR_compat_syscalls]; + u32 sc_compat_exit_refcount_map[NR_compat_syscalls]; }; static void syscall_entry_event_unknown(struct hlist_head *unknown_action_list_head, @@ -973,6 +982,7 @@ int lttng_syscall_filter_enable( { const char *syscall_name; unsigned long *bitmap; + u32 *refcount_map; int syscall_nr; syscall_name = get_syscall_name(desc_name, abi, entryexit); @@ -995,9 +1005,11 @@ int lttng_syscall_filter_enable( switch (abi) { case LTTNG_SYSCALL_ABI_NATIVE: bitmap = filter->sc_entry; + refcount_map = filter->sc_entry_refcount_map; break; case LTTNG_SYSCALL_ABI_COMPAT: bitmap = filter->sc_compat_entry; + refcount_map = filter->sc_compat_entry_refcount_map; break; default: return -EINVAL; @@ -1007,9 +1019,11 @@ int lttng_syscall_filter_enable( switch (abi) { case LTTNG_SYSCALL_ABI_NATIVE: bitmap = filter->sc_exit; + refcount_map = filter->sc_exit_refcount_map; break; case LTTNG_SYSCALL_ABI_COMPAT: bitmap = filter->sc_compat_exit; + refcount_map = filter->sc_compat_exit_refcount_map; break; default: return -EINVAL; @@ -1018,9 +1032,10 @@ int lttng_syscall_filter_enable( default: return -EINVAL; } - if (test_bit(syscall_nr, bitmap)) - return -EEXIST; - bitmap_set(bitmap, syscall_nr, 1); + if (refcount_map[syscall_nr] == U32_MAX) + return -EOVERFLOW; + if (refcount_map[syscall_nr]++ == 0) + bitmap_set(bitmap, syscall_nr, 1); return 0; } @@ -1033,6 +1048,10 @@ int lttng_syscall_filter_enable_event(struct lttng_kernel_event_common *event) WARN_ON_ONCE(event->priv->instrumentation != LTTNG_KERNEL_ABI_SYSCALL); + /* Skip unknown syscall */ + if (syscall_id == -1U) + return 0; + ret = lttng_syscall_filter_enable(syscall_table->sc_filter, event->priv->desc->event_name, event->priv->u.syscall.abi, event->priv->u.syscall.entryexit); @@ -1083,6 +1102,7 @@ int lttng_syscall_filter_disable(struct lttng_syscall_filter *filter, { const char *syscall_name; unsigned long *bitmap; + u32 *refcount_map; int syscall_nr; syscall_name = get_syscall_name(desc_name, abi, entryexit); @@ -1105,9 +1125,11 @@ int lttng_syscall_filter_disable(struct lttng_syscall_filter *filter, switch (abi) { case LTTNG_SYSCALL_ABI_NATIVE: bitmap = filter->sc_entry; + refcount_map = filter->sc_entry_refcount_map; break; case LTTNG_SYSCALL_ABI_COMPAT: bitmap = filter->sc_compat_entry; + refcount_map = filter->sc_compat_entry_refcount_map; break; default: return -EINVAL; @@ -1117,9 +1139,11 @@ int lttng_syscall_filter_disable(struct lttng_syscall_filter *filter, switch (abi) { case LTTNG_SYSCALL_ABI_NATIVE: bitmap = filter->sc_exit; + refcount_map = filter->sc_exit_refcount_map; break; case LTTNG_SYSCALL_ABI_COMPAT: bitmap = filter->sc_compat_exit; + refcount_map = filter->sc_compat_exit_refcount_map; break; default: return -EINVAL; @@ -1128,18 +1152,23 @@ int lttng_syscall_filter_disable(struct lttng_syscall_filter *filter, default: return -EINVAL; } - if (!test_bit(syscall_nr, bitmap)) - return -EEXIST; - bitmap_clear(bitmap, syscall_nr, 1); - + if (refcount_map[syscall_nr] == 0) + return -ENOENT; + if (--refcount_map[syscall_nr] == 0) + bitmap_clear(bitmap, syscall_nr, 1); return 0; } int lttng_syscall_filter_disable_event(struct lttng_kernel_event_common *event) { struct lttng_kernel_syscall_table *syscall_table = get_syscall_table_from_event(event); + unsigned int syscall_id = event->priv->u.syscall.syscall_id; int ret; + /* Skip unknown syscall */ + if (syscall_id == -1U) + return 0; + ret = lttng_syscall_filter_disable(syscall_table->sc_filter, event->priv->desc->event_name, event->priv->u.syscall.abi, event->priv->u.syscall.entryexit); -- 2.34.1