linux-kernel - Re: [PATCH] perf symbols: Fix return incorrect build_id size in elf_read_build

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] [day] [month] [year] [list]

Date:   Fri, 28 Apr 2023 22:31:11 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Adrian Hunter <adrian.hunter@...el.com>
Cc:     Yang Jihong <yangjihong1@...wei.com>, peterz@...radead.org,
        mingo@...hat.com, mark.rutland@....com,
        alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
        namhyung@...nel.org, irogers@...gle.com, leo.yan@...aro.org,
        eranian@...gle.com, linux-perf-users@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf symbols: Fix return incorrect build_id size in
 elf_read_build_id()

Em Thu, Apr 27, 2023 at 09:02:06AM +0300, Adrian Hunter escreveu:
> On 27/04/23 04:28, Yang Jihong wrote:
> > In elf_read_build_id(), if gnu build_id is found, should return the size of
> > the actually copied data. If descsz is greater thanBuild_ID_SIZE,
> > write_buildid data access may occur.
> > 
> > Fixes: be96ea8ffa78 ("perf symbols: Fix issue with binaries using 16-bytes buildids (v2)")
> > Reported-by: Will Ochowicz <Will.Ochowicz@...usplc.com>
> > Link: https://lore.kernel.org/lkml/CWLP265MB49702F7BA3D6D8F13E4B1A719C649@CWLP265MB4970.GBRP265.PROD.OUTLOOK.COM/T/
> > Tested-by: Will Ochowicz <Will.Ochowicz@...usplc.com>
> > Signed-off-by: Yang Jihong <yangjihong1@...wei.com>
> 
> As an aside, note that the build ID on the ELF file triggering the bug was
> 62363266373430613439613534633666326432323334653665623530396566343938656130663039
> which is 80 ASCII characters, which would have been a 20 byte binary number i.e.
> 
> $ echo -en "0000: 62363266373430613439613534633666\n0010: 32643232333465366562353039656634\n0020: 3938656130663039" | xxd -r | od -c -A d 
> 0000000   b   6   2   f   7   4   0   a   4   9   a   5   4   c   6   f
> 0000016   2   d   2   2   3   4   e   6   e   b   5   0   9   e   f   4
> 0000032   9   8   e   a   0   f   0   9
> 0000040
> 
> So perhaps a mistake in the creation of the build ID on that ELF file.
> 
> In any case, for the fix:
> 
> Acked-by: Adrian Hunter <adrian.hunter@...el.com>

Thanks, applied.

- Arnaldo

 
> > ---
> >  tools/perf/util/symbol-elf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index 41882ae8452e..059f88eca630 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -903,7 +903,7 @@ static int elf_read_build_id(Elf *elf, void *bf, size_t size)
> >  				size_t sz = min(size, descsz);
> >  				memcpy(bf, ptr, sz);
> >  				memset(bf + sz, 0, size - sz);
> > -				err = descsz;
> > +				err = sz;
> >  				break;
> >  			}
> >  		}
> 

-- 

- Arnaldo