diff mbox

[lttng-tools] Fix: sessiond: ht_match_event(): check if filter is NULL

Message ID 20160615213417.2946-1-janinesutto@gmail.com
State Rejected, archived
Headers show

Commit Message

Janine Sutto June 15, 2016, 9:34 p.m. UTC
It looks like an agent event's filter expression is NULL when
it's created with -a, for example:

    lttng enable-event -j -a

Since there's no check for this in ht_match_event(), strlen()
makes the session daemon segfault with this scenario:

    lttng create
    lttng enable-event -j -a
    lttng disable-event -j -a

Signed-off-by: Janine Sutto <janinesutto at gmail.com>
---
 src/bin/lttng-sessiond/agent.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

--
2.8.3

Comments

Mathieu Desnoyers June 18, 2016, 2:36 a.m. UTC | #1
Received: from jsutto (cable-192.222.213.99.electronicbox.net.

host mtl.efficios.com
mtl.efficios.com has address 192.222.213.99

Nice try Phil. ;-)

We'll need the actual contributor name to merge this contribution
however. I'm pretty sure there is no Mrs. Sutto at EfficiOS.

Thanks,

Mathieu

----- On Jun 15, 2016, at 5:34 PM, Janine Sutto janinesutto at gmail.com wrote:

> It looks like an agent event's filter expression is NULL when
> it's created with -a, for example:
> 
>    lttng enable-event -j -a
> 
> Since there's no check for this in ht_match_event(), strlen()
> makes the session daemon segfault with this scenario:
> 
>    lttng create
>    lttng enable-event -j -a
>    lttng disable-event -j -a
> 
> Signed-off-by: Janine Sutto <janinesutto at gmail.com>
> ---
> src/bin/lttng-sessiond/agent.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/bin/lttng-sessiond/agent.c b/src/bin/lttng-sessiond/agent.c
> index 8e1ef08..7cbbbde 100644
> --- a/src/bin/lttng-sessiond/agent.c
> +++ b/src/bin/lttng-sessiond/agent.c
> @@ -143,11 +143,18 @@ static int ht_match_event(struct cds_lfht_node *node,
> 	}
> 
> 	/* Filter expression */
> -	if (strncmp(event->filter_expression, key->filter_expression,
> -			strlen(event->filter_expression)) != 0) {
> +	if (!!event->filter_expression ^ !!key->filter_expression) {
> +		/* One has a filter expression, the other does not */
> 		goto no_match;
> 	}
> 
> +	if (event->filter_expression) {
> +		if (strncmp(event->filter_expression, key->filter_expression,
> +				strlen(event->filter_expression)) != 0) {
> +			goto no_match;
> +		}
> +	}
> +
> 	return 1;
> 
> no_match:
> --
> 2.8.3
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
Philippe Proulx June 18, 2016, 3 p.m. UTC | #2
Oh, Janine hijacked my workstation again!

Philippe Proulx

On Fri, Jun 17, 2016 at 10:36 PM, Mathieu Desnoyers <
mathieu.desnoyers at efficios.com> wrote:

> Received: from jsutto (cable-192.222.213.99.electronicbox.net.
>
> host mtl.efficios.com
> mtl.efficios.com has address 192.222.213.99
>
> Nice try Phil. ;-)
>
> We'll need the actual contributor name to merge this contribution
> however. I'm pretty sure there is no Mrs. Sutto at EfficiOS.
>
> Thanks,
>
> Mathieu
>
> ----- On Jun 15, 2016, at 5:34 PM, Janine Sutto janinesutto at gmail.com
> wrote:
>
> > It looks like an agent event's filter expression is NULL when
> > it's created with -a, for example:
> >
> >    lttng enable-event -j -a
> >
> > Since there's no check for this in ht_match_event(), strlen()
> > makes the session daemon segfault with this scenario:
> >
> >    lttng create
> >    lttng enable-event -j -a
> >    lttng disable-event -j -a
> >
> > Signed-off-by: Janine Sutto <janinesutto at gmail.com>
> > ---
> > src/bin/lttng-sessiond/agent.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/bin/lttng-sessiond/agent.c
> b/src/bin/lttng-sessiond/agent.c
> > index 8e1ef08..7cbbbde 100644
> > --- a/src/bin/lttng-sessiond/agent.c
> > +++ b/src/bin/lttng-sessiond/agent.c
> > @@ -143,11 +143,18 @@ static int ht_match_event(struct cds_lfht_node
> *node,
> >       }
> >
> >       /* Filter expression */
> > -     if (strncmp(event->filter_expression, key->filter_expression,
> > -                     strlen(event->filter_expression)) != 0) {
> > +     if (!!event->filter_expression ^ !!key->filter_expression) {
> > +             /* One has a filter expression, the other does not */
> >               goto no_match;
> >       }
> >
> > +     if (event->filter_expression) {
> > +             if (strncmp(event->filter_expression,
> key->filter_expression,
> > +                             strlen(event->filter_expression)) != 0) {
> > +                     goto no_match;
> > +             }
> > +     }
> > +
> >       return 1;
> >
> > no_match:
> > --
> > 2.8.3
> >
> > _______________________________________________
> > lttng-dev mailing list
> > lttng-dev at lists.lttng.org
> > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.lttng.org/pipermail/lttng-dev/attachments/20160618/eff44f76/attachment.html>
diff mbox

Patch

diff --git a/src/bin/lttng-sessiond/agent.c b/src/bin/lttng-sessiond/agent.c
index 8e1ef08..7cbbbde 100644
--- a/src/bin/lttng-sessiond/agent.c
+++ b/src/bin/lttng-sessiond/agent.c
@@ -143,11 +143,18 @@  static int ht_match_event(struct cds_lfht_node *node,
 	}

 	/* Filter expression */
-	if (strncmp(event->filter_expression, key->filter_expression,
-			strlen(event->filter_expression)) != 0) {
+	if (!!event->filter_expression ^ !!key->filter_expression) {
+		/* One has a filter expression, the other does not */
 		goto no_match;
 	}

+	if (event->filter_expression) {
+		if (strncmp(event->filter_expression, key->filter_expression,
+				strlen(event->filter_expression)) != 0) {
+			goto no_match;
+		}
+	}
+
 	return 1;

 no_match: