linux-kernel - Re: [PATCH] perf: fix uninitialized variable in thread__set

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: <CABPqkBQv9R1R7GPBMHf1V4BVmTyYopmdeKVCfy6-OZFiFX1CWw@mail.gmail.com>
Date:	Fri, 7 Oct 2011 12:14:18 +0200
From:	Stephane Eranian <eranian@...gle.com>
To:	linux-kernel@...r.kernel.org
Cc:	acme@...hat.com, peterz@...radead.org, mingo@...e.hu,
	dsahern@...il.com
Subject: Re: [PATCH] perf: fix uninitialized variable in thread__set_comm()

Arnaldo,

Did you also integrate this patch?


On Tue, Sep 6, 2011 at 5:00 PM, Stephane Eranian <eranian@...gle.com> wrote:
>
> In thread__set_comm(), a command string could be freed and then
> reused to point to a new command. Yet, the length was never reset
> nor recomputed. This could lead to bogus column formatting.
>
> This patch below fixes the problem by having thread__set_comm()
> immediately compute the string length in thread->comm_len. It
> gets rid of thread__comm_len() which is not needed anymore.
> It was doing the lazy eval of strlen() which could cause the
> issue.
>
> Signed-off-by: Stephane Eranian <eranian@...gle.com>
> ---
>
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 677e1da..b883821 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -61,7 +61,7 @@ static void hists__calc_col_len(struct hists *self, struct hist_entry *h)
>                                           unresolved_col_width);
>        }
>
> -       len = thread__comm_len(h->thread);
> +       len = h->thread->comm_len;
>        if (hists__new_col_len(self, HISTC_COMM, len))
>                hists__set_col_len(self, HISTC_THREAD, len + 6);
>
> @@ -731,7 +731,7 @@ static size_t hist_entry__fprintf_callchain(struct hist_entry *self,
>                struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
>                                                         typeof(*se), list);
>                left_margin = hists__col_len(hists, se->se_width_idx);
> -               left_margin -= thread__comm_len(self->thread);
> +               left_margin -= self->thread->comm_len;
>        }
>
>        return hist_entry_callchain__fprintf(fp, self, session_total,
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index d5d3b22..3ce2a80 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -35,26 +35,18 @@ int thread__set_comm(struct thread *self, const char *comm)
>
>        if (self->comm)
>                free(self->comm);
> +
>        self->comm = strdup(comm);
> +
>        err = self->comm == NULL ? -ENOMEM : 0;
>        if (!err) {
>                self->comm_set = true;
> +               self->comm_len = strlen(self->comm);
>                map_groups__flush(&self->mg);
>        }
>        return err;
>  }
>
> -int thread__comm_len(struct thread *self)
> -{
> -       if (!self->comm_len) {
> -               if (!self->comm)
> -                       return 0;
> -               self->comm_len = strlen(self->comm);
> -       }
> -
> -       return self->comm_len;
> -}
> -
>  static size_t thread__fprintf(struct thread *self, FILE *fp)
>  {
>        return fprintf(fp, "Thread %d %s\n", self->pid, self->comm) +
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index e5f2401..701ea09 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -23,7 +23,6 @@ struct perf_session;
>  void thread__delete(struct thread *self);
>
>  int thread__set_comm(struct thread *self, const char *comm);
> -int thread__comm_len(struct thread *self);
>  struct thread *perf_session__findnew(struct perf_session *self, pid_t pid);
>  void thread__insert_map(struct thread *self, struct map *map);
>  int thread__fork(struct thread *self, struct thread *parent);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/