From: Mathieu Desnoyers Date: Wed, 1 Dec 2021 20:56:40 +0000 (-0500) Subject: Fix: syscall tracing: missing trigger actions X-Git-Tag: v2.13.1~7 X-Git-Url: https://git.lttng.org/?p=lttng-modules.git;a=commitdiff_plain;h=4fabf854f6bee54286be80dd682921634f43e813 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 --- diff --git a/src/lttng-syscalls.c b/src/lttng-syscalls.c index bd836c00..202745b8 100644 --- a/src/lttng-syscalls.c +++ b/src/lttng-syscalls.c @@ -135,6 +135,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, @@ -1367,6 +1376,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); @@ -1389,9 +1399,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; @@ -1401,9 +1413,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; @@ -1412,9 +1426,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; } @@ -1428,13 +1443,16 @@ int lttng_syscall_filter_enable_event_notifier( WARN_ON_ONCE(event_notifier->priv->parent.instrumentation != LTTNG_KERNEL_ABI_SYSCALL); + /* Skip unknown syscall */ + if (syscall_id == -1U) + return 0; + ret = lttng_syscall_filter_enable(group->sc_filter, event_notifier->priv->parent.desc->event_name, event_notifier->priv->parent.u.syscall.abi, event_notifier->priv->parent.u.syscall.entryexit); - if (ret) { - goto end; - } + if (ret) + return ret; switch (event_notifier->priv->parent.u.syscall.entryexit) { case LTTNG_SYSCALL_ENTRY: @@ -1471,15 +1489,21 @@ int lttng_syscall_filter_enable_event_notifier( hlist_add_head_rcu(&event_notifier->priv->parent.u.syscall.node, dispatch_list); end: - return ret ; + return ret; } int lttng_syscall_filter_enable_event( struct lttng_kernel_channel_buffer *channel, struct lttng_kernel_event_recorder *event_recorder) { + unsigned int syscall_id = event_recorder->priv->parent.u.syscall.syscall_id; + WARN_ON_ONCE(event_recorder->priv->parent.instrumentation != LTTNG_KERNEL_ABI_SYSCALL); + /* Skip unknown syscall */ + if (syscall_id == -1U) + return 0; + return lttng_syscall_filter_enable(channel->priv->parent.sc_filter, event_recorder->priv->parent.desc->event_name, event_recorder->priv->parent.u.syscall.abi, @@ -1494,6 +1518,7 @@ int lttng_syscall_filter_disable( { const char *syscall_name; unsigned long *bitmap; + u32 *refcount_map; int syscall_nr; syscall_name = get_syscall_name(desc_name, abi, entryexit); @@ -1516,9 +1541,11 @@ int lttng_syscall_filter_disable( 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; @@ -1528,9 +1555,11 @@ int lttng_syscall_filter_disable( 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; @@ -1539,10 +1568,10 @@ int lttng_syscall_filter_disable( 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; } @@ -1550,15 +1579,21 @@ int lttng_syscall_filter_disable_event_notifier( struct lttng_kernel_event_notifier *event_notifier) { struct lttng_event_notifier_group *group = event_notifier->priv->group; + unsigned int syscall_id = event_notifier->priv->parent.u.syscall.syscall_id; int ret; WARN_ON_ONCE(event_notifier->priv->parent.instrumentation != LTTNG_KERNEL_ABI_SYSCALL); + /* Skip unknown syscall */ + if (syscall_id == -1U) + return 0; + ret = lttng_syscall_filter_disable(group->sc_filter, event_notifier->priv->parent.desc->event_name, event_notifier->priv->parent.u.syscall.abi, event_notifier->priv->parent.u.syscall.entryexit); - WARN_ON_ONCE(ret != 0); + if (ret) + return ret; hlist_del_rcu(&event_notifier->priv->parent.u.syscall.node); return 0; @@ -1568,6 +1603,12 @@ int lttng_syscall_filter_disable_event( struct lttng_kernel_channel_buffer *channel, struct lttng_kernel_event_recorder *event_recorder) { + unsigned int syscall_id = event_recorder->priv->parent.u.syscall.syscall_id; + + /* Skip unknown syscall */ + if (syscall_id == -1U) + return 0; + return lttng_syscall_filter_disable(channel->priv->parent.sc_filter, event_recorder->priv->parent.desc->event_name, event_recorder->priv->parent.u.syscall.abi,