linux-kernel - Re: [PATCH] building libtraceevent with clang

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170213162455.GB6473@kernel.org>
Date:   Mon, 13 Feb 2017 13:24:55 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Namhyung Kim <namhyung@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] building libtraceevent with clang

Em Fri, Feb 10, 2017 at 02:14:08PM -0500, Steven Rostedt escreveu:
> On Fri, 10 Feb 2017 14:03:17 -0300
> Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
> > +	case OLD_RINGBUF_TYPE_TIME_EXTEND:
> > +		extend = read_4(kbuf, ptr);
> > +		extend <<= TS_SHIFT;
> > +		extend += delta;
> > +		delta = extend;
> > +		ptr += 4;
> > +		/* Fall through */
 
> No, actually, the length should be zeroed here.
 
> 	length = 0;
 
> As the ptr is placed passed the metadata. At the end of this case
> statement, the ptr will be after the metadata. The length is
> to represent the length of the data that is part of the event. The
> index is the ptr - start_data + length. As there's no data to this type
> of event, it should be zero.
 
> When I add length = 0, I get more events from my old 2.6.30 trace.dat
> file :-/  I'll have to update my tests.
 
> Care to send another patch?

I have this now in my tree, and since the patch is basically yours, I
just made myself the reporter and you the author, please ack.


commit 1fb82b6cba967afde11957099a9ef440cf8c2248
Author: Steven Rostedt <rostedt@...dmis.org>
Date:   Mon Feb 13 13:18:29 2017 -0300

    tools lib traceevent: Initialize lenght on OLD_RING_BUFFER_TYPE_TIME_STAMP
    
    A undefined value was being used for the OLD_RING_BUFFER_TYPE_TIME_STAMP
    case entry, as the 'lenght' variable was not being initialized, fix it.
    
    Caught by the reporter when building tools/perf/ using clang, which emmitted
    this warning:
    
      kbuffer-parse.c:312:7: warning: variable 'length' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
              case OLD_RINGBUF_TYPE_TIME_EXTEND:
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      kbuffer-parse.c:339:29: note: uninitialized use occurs here
              kbuf->next = kbuf->index + length;
                                       ^~~~~~
      kbuffer-parse.c:297:21: note: initialize the variable 'length' to silence this warning
              unsigned int length;
                                 ^
                                  = 0
    
    Reported-by: Arnaldo Carvalho de Melo <acme@...hat.com>
    Cc: Adrian Hunter <adrian.hunter@...el.com>
    Cc: David Ahern <dsahern@...il.com>
    Cc: Jiri Olsa <jolsa@...nel.org>
    Cc: Namhyung Kim <namhyung@...nel.org>
    Cc: Wang Nan <wangnan0@...wei.com>
    Link: http://lkml.kernel.org/r/20170210141408.5eeb6e91@gandalf.local.home
    Signed-off-by: Steven Rostedt <rostedt@...dmis.org>

diff --git a/tools/lib/traceevent/kbuffer-parse.c b/tools/lib/traceevent/kbuffer-parse.c
index 65984f1c2974..2009cb7d9675 100644
--- a/tools/lib/traceevent/kbuffer-parse.c
+++ b/tools/lib/traceevent/kbuffer-parse.c
@@ -315,6 +315,7 @@ static unsigned int old_update_pointers(struct kbuffer *kbuf)
 		extend += delta;
 		delta = extend;
 		ptr += 4;
+		lenght = 0;
 		break;
 
 	case OLD_RINGBUF_TYPE_TIME_STAMP: