diff --git a/PR14348.patch b/PR14348.patch new file mode 100644 index 0000000..e1015a9 --- /dev/null +++ b/PR14348.patch @@ -0,0 +1,412 @@ +commit 50bd2c22a62981ae8b7b379d48574671228f1446 +Author: David Smith +Date: Mon Jul 9 16:07:22 2012 -0500 + + Add code to keep track of task_work callbacks to avoid a possible crash. + + * runtime/stp_task_work.c: New file. + * runtime/stp_utrace.c (utrace_init): Move STAPCONF_TASK_WORK_ADD_EXPORTED + code to stp_task_work_init(). + (utrace_exit): Call stp_task_work_exit(). + (utrace_cleanup): Call task_work_cancel() wrapper function. + (utrace_free): Ditto. + (utrace_do_stop): Call task_work_add() wrapper function. + (utrace_control): Ditto. + (finish_report): Ditto. + (utrace_resume): Call stp_task_work_func_done(). + * runtime/task_finder2.c (__stp_tf_cancel_task_work): Call + task_work_cancel() wrapper function. + (__stp_tf_quiesce_worker): Call stp_task_work_func_done(). + (__stp_utrace_task_finder_target_quiesce): Call task_work_add() wrapper + function. + (__stp_tf_mmap_worker): Call stp_task_work_func_done(). + (__stp_utrace_task_finder_target_syscall_exit): Call task_work_add() wrapper + function. + +diff --git a/runtime/task_finder2.c b/runtime/task_finder2.c +index 19cb99e..81cb04e 100644 +--- a/runtime/task_finder2.c ++++ b/runtime/task_finder2.c +@@ -9,7 +9,6 @@ + #ifndef STAPCONF_TASK_UID + #include + #endif +-#include + #include "syscall.h" + #include "task_finder_map.c" + +@@ -170,7 +169,7 @@ static void __stp_tf_cancel_task_work(void) + list_for_each_entry(node, &__stp_tf_task_work_list, list) { + // Remove the item from the list, cancel it, then free it. + list_del(&node->list); +- task_work_cancel(node->task, node->work.func); ++ stp_task_work_cancel(node->task, node->work.func); + _stp_kfree(node); + } + spin_unlock_irqrestore(&__stp_tf_task_work_list_lock, flags); +@@ -1210,8 +1209,11 @@ __stp_tf_quiesce_worker(struct task_work *work) + + might_sleep(); + __stp_tf_free_task_work(work); +- if (atomic_read(&__stp_task_finder_state) != __STP_TF_RUNNING) ++ if (atomic_read(&__stp_task_finder_state) != __STP_TF_RUNNING) { ++ /* Remember that this task_work_func is finished. */ ++ stp_task_work_func_done(); + return; ++ } + + __stp_tf_handler_start(); + +@@ -1227,6 +1229,9 @@ __stp_tf_quiesce_worker(struct task_work *work) + } + + __stp_tf_handler_end(); ++ ++ /* Remember that this task_work_func is finished. */ ++ stp_task_work_func_done(); + return; + } + +@@ -1288,12 +1293,13 @@ __stp_utrace_task_finder_target_quiesce(u32 action, + return UTRACE_RESUME; + } + init_task_work(work, &__stp_tf_quiesce_worker, tgt); +- rc = task_work_add(tsk, work, true); +- /* task_work_add() returns -ESRCH if the task has ++ ++ rc = stp_task_work_add(tsk, work); ++ /* stp_task_work_add() returns -ESRCH if the task has + * already passed exit_task_work(). Just ignore this + * error. */ + if (rc != 0 && rc != -ESRCH) { +- printk(KERN_ERR "%s:%d - task_work_add() returned %d\n", ++ printk(KERN_ERR "%s:%d - stp_task_work_add() returned %d\n", + __FUNCTION__, __LINE__, rc); + } + } +@@ -1397,13 +1403,19 @@ __stp_tf_mmap_worker(struct task_work *work) + + might_sleep(); + __stp_tf_free_task_work(work); +- if (atomic_read(&__stp_task_finder_state) != __STP_TF_RUNNING) ++ if (atomic_read(&__stp_task_finder_state) != __STP_TF_RUNNING) { ++ /* Remember that this task_work_func is finished. */ ++ stp_task_work_func_done(); + return; ++ } + + // See if we can find saved syscall info. + entry = __stp_tf_get_map_entry(current); +- if (entry == NULL) ++ if (entry == NULL) { ++ /* Remember that this task_work_func is finished. */ ++ stp_task_work_func_done(); + return; ++ } + + __stp_tf_handler_start(); + +@@ -1426,6 +1438,9 @@ __stp_tf_mmap_worker(struct task_work *work) + __stp_tf_remove_map_entry(entry); + + __stp_tf_handler_end(); ++ ++ /* Remember that this task_work_func is finished. */ ++ stp_task_work_func_done(); + return; + } + +@@ -1492,12 +1507,12 @@ __stp_utrace_task_finder_target_syscall_exit(u32 action, + return UTRACE_RESUME; + } + init_task_work(work, &__stp_tf_mmap_worker, tgt); +- rc = task_work_add(tsk, work, true); +- /* task_work_add() returns -ESRCH if the task has ++ rc = stp_task_work_add(tsk, work); ++ /* stp_task_work_add() returns -ESRCH if the task has + * already passed exit_task_work(). Just ignore this + * error. */ + if (rc != 0 && rc != -ESRCH) { +- printk(KERN_ERR "%s:%d - task_work_add() returned %d\n", ++ printk(KERN_ERR "%s:%d - stp_task_work_add() returned %d\n", + __FUNCTION__, __LINE__, rc); + } + } +diff --git a/runtime/stp_task_work.c b/runtime/stp_task_work.c +new file mode 100644 +index 0000000..b99593e +--- /dev/null ++++ b/runtime/stp_task_work.c +@@ -0,0 +1,102 @@ ++#ifndef _STP_TASK_WORK_C ++#define _STP_TASK_WORK_C ++ ++#include ++ ++#if !defined(STAPCONF_TASK_WORK_ADD_EXPORTED) ++typedef int (*task_work_add_fn)(struct task_struct *task, ++ struct task_work *twork, bool notify); ++#define task_work_add (* (task_work_add_fn)kallsyms_task_work_add) ++typedef struct task_work *(*task_work_cancel_fn)(struct task_struct *, ++ task_work_func_t); ++#define task_work_cancel (* (task_work_cancel_fn)kallsyms_task_work_cancel) ++#endif ++ ++/* To avoid a crash when a task_work callback gets called after the ++ * module is unloaded, keep track of the number of current callbacks. */ ++static atomic_t stp_task_work_callbacks = ATOMIC_INIT(0); ++ ++/* ++ * stp_task_work_init() should be called before any other ++ * stp_task_work_* functions are called to do setup. ++ */ ++int ++stp_task_work_init(void) ++{ ++#if !defined(STAPCONF_TASK_WORK_ADD_EXPORTED) ++ /* The task_work_add()/task_work_cancel() functions aren't ++ * exported. Look up those function addresses. */ ++ kallsyms_task_work_add = (void *)kallsyms_lookup_name("task_work_add"); ++ if (kallsyms_task_work_add == NULL) { ++ _stp_error("Can't resolve task_work_add!"); ++ return -ENOENT; ++ } ++ kallsyms_task_work_cancel = (void *)kallsyms_lookup_name("task_work_cancel"); ++ if (kallsyms_task_work_cancel == NULL) { ++ _stp_error("Can't resolve task_work_cancel!"); ++ return -ENOENT; ++ } ++#endif ++ return 0; ++} ++ ++/* ++ * stap_task_work_exit() should be called when no more ++ * stp_task_work_* functions will be called (before module exit). ++ * ++ * This function makes sure that all the callbacks are finished before ++ * letting the module unload. If the module unloads before a callback ++ * is called, the kernel will try to make a function call to an ++ * invalid address. ++ */ ++void ++stp_task_work_exit(void) ++{ ++ while (atomic_read(&stp_task_work_callbacks)) ++ schedule_timeout_uninterruptible(1); ++ return; ++} ++ ++/* ++ * Our task_work_add() wrapper that remembers that we've got a pending ++ * callback. ++ */ ++int ++stp_task_work_add(struct task_struct *task, struct task_work *twork) ++{ ++ int rc; ++ ++ rc = task_work_add(task, twork, true); ++ if (rc == 0) ++ atomic_inc(&stp_task_work_callbacks); ++ return rc; ++} ++ ++/* ++ * Our task_work_cancel() wrapper that remembers that a callback has ++ * been cancelled. ++ */ ++struct task_work * ++stp_task_work_cancel(struct task_struct *task, task_work_func_t func) ++{ ++ struct task_work *twork; ++ ++ twork = task_work_cancel(task, func); ++ if (twork != NULL) ++ atomic_dec(&stp_task_work_callbacks); ++ return twork; ++} ++ ++/* ++ * stp_task_work_func_done() should be called at the very end of a ++ * task_work callback function so that we can keep up with callback ++ * accounting. ++ */ ++void ++stp_task_work_func_done(void) ++{ ++ atomic_dec(&stp_task_work_callbacks); ++} ++ ++ ++#endif /* _STP_TASK_WORK_C */ +diff --git a/runtime/stp_utrace.c b/runtime/stp_utrace.c +index d4538d9..face3f9 100644 +--- a/runtime/stp_utrace.c ++++ b/runtime/stp_utrace.c +@@ -25,7 +25,7 @@ + #include + #include + #include +-#include ++#include "stp_task_work.c" + + /* + * Per-thread structure private to utrace implementation. +@@ -105,38 +105,13 @@ static void utrace_report_exec(void *cb_data __attribute__ ((unused)), + #define __UTRACE_REGISTERED 1 + static atomic_t utrace_state = ATOMIC_INIT(__UTRACE_UNREGISTERED); + +-#if !defined(STAPCONF_TASK_WORK_ADD_EXPORTED) +-typedef int (*task_work_add_fn)(struct task_struct *task, +- struct task_work *twork, bool notify); +-#define task_work_add (* (task_work_add_fn)kallsyms_task_work_add) +-typedef struct task_work *(*task_work_cancel_fn)(struct task_struct *, +- task_work_func_t); +-#define task_work_cancel (* (task_work_cancel_fn)kallsyms_task_work_cancel) +-#endif +- + int utrace_init(void) + { + int i; + int rc = -1; + +-#if !defined(STAPCONF_TASK_WORK_ADD_EXPORTED) +- /* The task_work_add()/task_work_cancel() functions aren't +- * exported. Look up those function addresses. */ +- kallsyms_task_work_add = (void *)kallsyms_lookup_name ("task_work_add"); +- if (kallsyms_task_work_add == NULL) { +- printk(KERN_ERR "%s can't resolve task_work_add!", +- THIS_MODULE->name); +- rc = -ENOENT; ++ if (unlikely(stp_task_work_init() != 0)) + goto error; +- } +- kallsyms_task_work_cancel = (void *)kallsyms_lookup_name ("task_work_cancel"); +- if (kallsyms_task_work_cancel == NULL) { +- printk(KERN_ERR "%s can't resolve task_work_cancel!", +- THIS_MODULE->name); +- rc = -ENOENT; +- goto error; +- } +-#endif + + /* initialize the list heads */ + for (i = 0; i < TASK_UTRACE_TABLE_SIZE; i++) { +@@ -205,6 +180,8 @@ int utrace_exit(void) + kmem_cache_destroy(utrace_cachep); + if (utrace_engine_cachep) + kmem_cache_destroy(utrace_engine_cachep); ++ ++ stp_task_work_exit(); + return 0; + } + +@@ -239,7 +216,7 @@ static void utrace_cleanup(struct utrace *utrace) + } + + if (utrace->task_work_added) { +- if (task_work_cancel(utrace->task, &utrace_resume) == NULL) ++ if (stp_task_work_cancel(utrace->task, &utrace_resume) == NULL) + printk(KERN_ERR "%s:%d - task_work_cancel() failed? task %p, %d, %s\n", + __FUNCTION__, __LINE__, utrace->task, + utrace->task->tgid, +@@ -379,7 +356,7 @@ static void utrace_free(struct utrace *utrace) + #endif + + if (utrace->task_work_added) { +- if (task_work_cancel(utrace->task, &utrace_resume) == NULL) ++ if (stp_task_work_cancel(utrace->task, &utrace_resume) == NULL) + printk(KERN_ERR "%s:%d - task_work_cancel() failed? task %p, %d, %s\n", + __FUNCTION__, __LINE__, utrace->task, + utrace->task->tgid, +@@ -898,11 +875,11 @@ static bool utrace_do_stop(struct task_struct *target, struct utrace *utrace) + } else if (utrace->resume > UTRACE_REPORT) { + utrace->resume = UTRACE_REPORT; + if (! utrace->task_work_added) { +- int rc = task_work_add(target, &utrace->work, true); ++ int rc = stp_task_work_add(target, &utrace->work); + if (rc == 0) { + utrace->task_work_added = 1; + } +- /* task_work_add() returns -ESRCH if the task ++ /* stp_task_work_add() returns -ESRCH if the task + * has already passed exit_task_work(). Just + * ignore this error. */ + else if (rc != -ESRCH) { +@@ -1041,16 +1018,16 @@ static void utrace_stop(struct task_struct *task, struct utrace *utrace, + */ + utrace->resume = action; + if (! utrace->task_work_added) { +- int rc = task_work_add(task, &utrace->work, true); ++ int rc = stp_task_work_add(task, &utrace->work); + if (rc == 0) { + utrace->task_work_added = 1; + } +- /* task_work_add() returns -ESRCH if the task ++ /* stp_task_work_add() returns -ESRCH if the task + * has already passed exit_task_work(). Just + * ignore this error. */ + else if (rc != -ESRCH) { + printk(KERN_ERR +- "%s:%d - task_work_add() returned %d\n", ++ "%s:%d - stp_task_work_add() returned %d\n", + __FUNCTION__, __LINE__, rc); + } + } +@@ -1397,18 +1374,17 @@ int utrace_control(struct task_struct *target, + if (action < utrace->resume) { + utrace->resume = action; + if (! utrace->task_work_added) { +- ret = task_work_add(target, &utrace->work, +- true); ++ ret = stp_task_work_add(target, &utrace->work); + if (ret == 0) { + utrace->task_work_added = 1; + } +- /* task_work_add() returns -ESRCH if ++ /* stp_task_work_add() returns -ESRCH if + * the task has already passed + * exit_task_work(). Just ignore this + * error. */ + else if (ret != -ESRCH) { + printk(KERN_ERR +- "%s:%d - task_work_add() returned %d\n", ++ "%s:%d - stp_task_work_add() returned %d\n", + __FUNCTION__, __LINE__, ret); + } + } +@@ -1560,11 +1536,11 @@ static void finish_report(struct task_struct *task, struct utrace *utrace, + spin_lock(&utrace->lock); + utrace->resume = resume; + if (! utrace->task_work_added) { +- int rc = task_work_add(task, &utrace->work, true); ++ int rc = stp_task_work_add(task, &utrace->work); + if (rc == 0) { + utrace->task_work_added = 1; + } +- /* task_work_add() returns -ESRCH if the task ++ /* stp_task_work_add() returns -ESRCH if the task + * has already passed exit_task_work(). Just + * ignore this error. */ + else if (rc != -ESRCH) { +@@ -2132,6 +2108,9 @@ void utrace_resume(struct task_work *work) + * call. (The arch might use TIF_NOTIFY_RESUME for other + * purposes as well as calling us.) + */ ++ ++ /* Remember that this task_work_func is finished. */ ++ stp_task_work_func_done(); + return; + case UTRACE_REPORT: + if (unlikely(!(utrace->utrace_flags & UTRACE_EVENT(QUIESCE)))) +@@ -2160,6 +2139,9 @@ void utrace_resume(struct task_work *work) + * effect now (i.e. step or interrupt). + */ + finish_resume_report(task, utrace, &report); ++ ++ /* Remember that this task_work_func is finished. */ ++ stp_task_work_func_done(); + } + + #endif /* _STP_UTRACE_C */ diff --git a/bz837641-staprun-no-linux-types.patch b/bz837641-staprun-no-linux-types.patch new file mode 100644 index 0000000..ca1fa38 --- /dev/null +++ b/bz837641-staprun-no-linux-types.patch @@ -0,0 +1,38 @@ +(NB: changed paths in backport from master) + +commit 037853b4644bb6ebd68e2f1fac11c3636f551d8e +Author: Josh Stone +Date: Mon Jul 9 12:07:48 2012 -0700 + + Fix the build with glibc 2.16 + + With our elevated compiler warnings as errors, we got: + + mainloop.c: In function 'stp_main_loop': + mainloop.c:581:3: error: signed and unsigned type in conditional expression [-Werror=sign-compare] + mainloop.c:581:3: error: signed and unsigned type in conditional expression [-Werror=sign-compare] + mainloop.c:583:35: error: signed and unsigned type in conditional expression [-Werror=sign-compare] + mainloop.c:583:35: error: signed and unsigned type in conditional expression [-Werror=sign-compare] + mainloop.c:631:2: error: signed and unsigned type in conditional expression [-Werror=sign-compare] + mainloop.c:631:2: error: signed and unsigned type in conditional expression [-Werror=sign-compare] + cc1: all warnings being treated as errors + + This turns out to be a macro fight between glibc-2.16 and + kernel-headers, via including linux/types.h. We apparently don't + even need that header, so removing it lets us proceed normally. + + Those curious can watch this bug to see how the conflict is resolved: + https://bugzilla.redhat.com/show_bug.cgi?id=837641 + +diff --git a/runtime/staprun/staprun.h b/runtime/staprun/staprun.h +index 28c7116..fe42cf2 100644 +--- a/runtime/staprun/staprun.h ++++ b/runtime/staprun/staprun.h +@@ -28,7 +28,6 @@ + #include + #include + #include +-#include + #include + #include + #include diff --git a/systemtap.spec b/systemtap.spec index 8433be8..5d8ec44 100644 --- a/systemtap.spec +++ b/systemtap.spec @@ -20,7 +20,7 @@ Name: systemtap Version: 1.8 -Release: 1%{?dist} +Release: 3%{?dist} # for version, see also configure.ac @@ -90,6 +90,9 @@ BuildRequires: /usr/share/publican/Common_Content/%{publican_brand}/defaults.cfg %endif %endif +Patch2: bz837641-staprun-no-linux-types.patch +Patch3: PR14348.patch + # Install requirements Requires: systemtap-client = %{version}-%{release} Requires: systemtap-devel = %{version}-%{release} @@ -249,6 +252,10 @@ find . \( -name configure -o -name config.h.in \) -print | xargs touch cd .. %endif +# bz837641-staprun-no-linux-types.patch +%patch2 -p1 +%patch3 -p1 + %build %if %{with_bundled_elfutils} @@ -579,6 +586,12 @@ exit 0 # ------------------------------------------------------------------------ %changelog +* Wed Jul 11 2012 Frank Ch. Eigler - 1.8-3 +- PR14348 task_work_add race condition fix + +* Mon Jul 09 2012 Josh Stone +- bz837641 build fix + * Sun Jun 17 2012 Frank Ch. Eigler - 1.8-1 - Upstream release.