In-Reply-To: Oleg Nesterov's message of Monday, 17 August 2009 17:31:23 +0200
<20090817153123.ga10...@redhat.com>
References: <20090811145211.ga30...@linux.vnet.ibm.com>
<20090811172310.ga14...@redhat.com>
<20090811213739.ea81a42...@magilla.sf.frob.com>
<20090812103644.ga29...@redhat.com>
<20090814230728.f039840...@magilla.sf.frob.com>
<20090817153123.ga10...@redhat.com>
X-Shopping-List:
(1) Vehement aggressors
(2) Asiatic Pencil bruisers
(3) Insufficient attention
(4) Decadent animals
I've added this patch to help module-writers diagnose problems like
the one that came up recently more quickly. This especially helps
for these cases where things can very well work fine without
supplying a callback in simple tests, but then fail later when
circumstances are more complicated (like unrelated other engines
being around).
Note I did not make it check event bits for UTRACE_STOP or
UTRACE_INTERRUPT. While it's usual practice to use these in
concern with a report_quiesce callback as with UTRACE_REPORT
et al, there are meaningful ways to use these without callbacks.
Signed-off-by: Roland McGrath <rol...@redhat.com>
---
kernel/utrace.c | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/kernel/utrace.c b/kernel/utrace.c
index fa2a719..35be909 100644
--- a/kernel/utrace.c
+++ b/kernel/utrace.c
@@ -985,6 +985,9 @@ static inline int utrace_control_dead(struct task_struct
*target,
* stopped, then there might be no callbacks until all engines let
* it resume.
*
+ * Since this is meaningless unless @report_quiesce callbacks will
+ * be made, it returns -%EINVAL if @engine lacks %UTRACE_EVENT(%QUIESCE).
+ *
* UTRACE_INTERRUPT:
*
* This is like %UTRACE_REPORT, but ensures that @target will make a
@@ -1013,12 +1016,18 @@ static inline int utrace_control_dead(struct
task_struct *target,
* @report_quiesce callback with a zero event mask, or the
* @report_signal callback with %UTRACE_SIGNAL_REPORT.
*
+ * Since this is not robust unless @report_quiesce callbacks will
+ * be made, it returns -%EINVAL if @engine lacks %UTRACE_EVENT(%QUIESCE).
+ *
* UTRACE_BLOCKSTEP:
*
* It's invalid to use this unless arch_has_block_step() returned true.
* This is like %UTRACE_SINGLESTEP, but resumes for one whole basic
* block of user instructions.
*
+ * Since this is not robust unless @report_quiesce callbacks will
+ * be made, it returns -%EINVAL if @engine lacks %UTRACE_EVENT(%QUIESCE).
+ *
* %UTRACE_BLOCKSTEP devolves to %UTRACE_SINGLESTEP when another
* tracing engine is using %UTRACE_SINGLESTEP at the same time.
*/
@@ -1033,6 +1042,18 @@ int utrace_control(struct task_struct *target,
if (unlikely(action > UTRACE_DETACH))
return -EINVAL;
+ /*
+ * This is a sanity check for a programming error in the caller.
+ * Their request can only work properly in all cases by relying on
+ * a follow-up callback, but they didn't set one up! This check
+ * doesn't do locking, but it shouldn't matter. The caller has to
+ * be synchronously sure the callback is set up to be operating the
+ * interface properly.
+ */
+ if (action >= UTRACE_REPORT && action < UTRACE_RESUME &&
+ unlikely(!(engine->flags & UTRACE_EVENT(QUIESCE))))
+ return -EINVAL;
+
utrace = get_utrace_lock(target, engine, true);
if (unlikely(IS_ERR(utrace)))
return PTR_ERR(utrace);
