Messages in this thread | | | Subject | Re: [PATCH] perf buildid: fix off-by-one in write_buildid() | From | Andrey Ryabinin <> | Date | Tue, 19 Apr 2016 18:48:43 +0300 |
| |
On 04/19/2016 04:38 PM, Arnaldo Carvalho de Melo wrote: > Em Tue, Apr 19, 2016 at 11:17:27AM +0300, Andrey Ryabinin escreveu: >> write_buildid() increments 'name_len' with intention to take into account >> trailing zero byte. However, 'name_len' was already incremented in >> machine__write_buildid_table() before. >> So this leads to out-of-bounds read in do_write(): > > Could we keep the assumptions that for a string 's' the length is > strlen(s) and that when we want to write a string _with_ its trailing > '\0' we should use strlen(s) + 1? > > I.e. I propose this patch instead, ok? >
Yup, looks good. Thanks.
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c > index 0573c2ec861d..b6ecf87bc3e3 100644 > --- a/tools/perf/util/build-id.c > +++ b/tools/perf/util/build-id.c > @@ -261,14 +261,14 @@ static int machine__write_buildid_table(struct machine *machine, int fd) > > if (dso__is_vdso(pos)) { > name = pos->short_name; > - name_len = pos->short_name_len + 1; > + name_len = pos->short_name_len; > } else if (dso__is_kcore(pos)) { > machine__mmap_name(machine, nm, sizeof(nm)); > name = nm; > - name_len = strlen(nm) + 1; > + name_len = strlen(nm); > } else { > name = pos->long_name; > - name_len = pos->long_name_len + 1; > + name_len = pos->long_name_len; > } > > in_kernel = pos->kernel || >
| |