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 <[email protected]>
Link: https://lore.kernel.org/lkml/CWLP265MB49702F7BA3D6D8F13E4B1A719C649@CWLP265MB4970.GBRP265.PROD.OUTLOOK.COM/T/
Tested-by: Will Ochowicz <[email protected]>
Signed-off-by: Yang Jihong <[email protected]>
---
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;
}
}
--
2.30.GIT
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 <[email protected]>
> Link: https://lore.kernel.org/lkml/CWLP265MB49702F7BA3D6D8F13E4B1A719C649@CWLP265MB4970.GBRP265.PROD.OUTLOOK.COM/T/
> Tested-by: Will Ochowicz <[email protected]>
> Signed-off-by: Yang Jihong <[email protected]>
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 <[email protected]>
> ---
> 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;
> }
> }
Hello,
On 2023/4/27 14:02, Adrian Hunter wrote:
> 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 <[email protected]>
>> Link: https://lore.kernel.org/lkml/CWLP265MB49702F7BA3D6D8F13E4B1A719C649@CWLP265MB4970.GBRP265.PROD.OUTLOOK.COM/T/
>> Tested-by: Will Ochowicz <[email protected]>
>> Signed-off-by: Yang Jihong <[email protected]>
>
> 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.
>
Yes, it may be an error, or it may be that the build-id was specified
when the elf file was created.
I tried ld can specify a build-id larger than 160-bit hexadecimal digits:
$ ld -o test test.o
--build-id=0x62363266373430613439613534633666326432323334653665623530396566343938656130663039
$ readelf -n test
Displaying notes found in: .note.gnu.build-id
Owner Data size Description
GNU 0x00000028 NT_GNU_BUILD_ID (unique build
ID bitstring)
Build ID:
62363266373430613439613534633666326432323334653665623530396566343938656130663039
All in all, it feels like a rare scene :)
> In any case, for the fix:
>
> Acked-by: Adrian Hunter <[email protected]>
>
Thanks for the acked-by.
Thanks,
Yang
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 <[email protected]>
> > Link: https://lore.kernel.org/lkml/CWLP265MB49702F7BA3D6D8F13E4B1A719C649@CWLP265MB4970.GBRP265.PROD.OUTLOOK.COM/T/
> > Tested-by: Will Ochowicz <[email protected]>
> > Signed-off-by: Yang Jihong <[email protected]>
>
> 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 <[email protected]>
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