public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] plug preempt leak in _stp_runtime_entryfn_put/get_context
@ 2016-05-02 15:48 Mateusz Guzik
  2016-05-02 17:30 ` David Smith
  0 siblings, 1 reply; 2+ messages in thread
From: Mateusz Guzik @ 2016-05-02 15:48 UTC (permalink / raw)
  To: systemtap; +Cc: dsmith

If _stp_runtime_entryfn_get_context returns a context, preemption
counter is always incremented. On the other hand
_stp_runtime_entryfn_put_context only decrements the counter if the
passed context matches the one currently set on the cpu.

The context can be set to NULL by _stp_runtime_contexts_free, making the
comparison false and in effect leading to a leak, e.g.:
timer: _stp_ctl_work_callback+0x0/0x1e0[stap_af8544c7eb51251ef8c
 377abff659b05_25070] preempt leak: 00000101 -> 00000102

Signed-off-by: Mateusz Guzik <mguzik@redhat.com>
---
 runtime/linux/runtime_context.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/runtime/linux/runtime_context.h b/runtime/linux/runtime_context.h
index c9ffe18..9d325da 100644
--- a/runtime/linux/runtime_context.h
+++ b/runtime/linux/runtime_context.h
@@ -80,11 +80,12 @@ static struct context * _stp_runtime_entryfn_get_context(void)
 
 static inline void _stp_runtime_entryfn_put_context(struct context *c)
 {
-	if (c && c == _stp_runtime_get_context()) {
-		atomic_dec(&c->busy);
+	if (c) {
+		if (c == _stp_runtime_get_context())
+			atomic_dec(&c->busy);
+		/* else, warn about bad state? */
 		preempt_enable_no_resched();
 	}
-	/* else, warn about bad state? */
 	return;
 }
 
-- 
2.5.5

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] plug preempt leak in _stp_runtime_entryfn_put/get_context
  2016-05-02 15:48 [PATCH] plug preempt leak in _stp_runtime_entryfn_put/get_context Mateusz Guzik
@ 2016-05-02 17:30 ` David Smith
  0 siblings, 0 replies; 2+ messages in thread
From: David Smith @ 2016-05-02 17:30 UTC (permalink / raw)
  To: Mateusz Guzik, systemtap

On 05/02/2016 10:48 AM, Mateusz Guzik wrote:
> If _stp_runtime_entryfn_get_context returns a context, preemption
> counter is always incremented. On the other hand
> _stp_runtime_entryfn_put_context only decrements the counter if the
> passed context matches the one currently set on the cpu.
> 
> The context can be set to NULL by _stp_runtime_contexts_free, making the
> comparison false and in effect leading to a leak, e.g.:
> timer: _stp_ctl_work_callback+0x0/0x1e0[stap_af8544c7eb51251ef8c
>  377abff659b05_25070] preempt leak: 00000101 -> 00000102
> 
> Signed-off-by: Mateusz Guzik <mguzik@redhat.com>

I checked this in. Thanks for the patch!

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-05-02 17:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-02 15:48 [PATCH] plug preempt leak in _stp_runtime_entryfn_put/get_context Mateusz Guzik
2016-05-02 17:30 ` David Smith

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).