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 <mathieu.desnoyers@efficios.com>
Change-Id: I3c1e966e9720a2e8f4438ae2c98d83ffe318e424

diff --git a/src/lttng-syscalls.c b/src/lttng-syscalls.c
index bd836c001d658257e5bf529d61a3cbeabc84a168..202745b8a5f2dcaa066f16b3ad3067fbf00942e9 100644 (file)
--- 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,