Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f149f4ce-bd2e-b192-920f-1e2599e5b16f@intel.com>
Date:   Thu, 27 Apr 2023 09:02:06 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.10.1
Subject: Re: [PATCH] perf symbols: Fix return incorrect build_id size in
 elf_read_build_id()
Content-Language: en-US
To:     Yang Jihong <yangjihong1@huawei.com>, peterz@infradead.org,
        mingo@redhat.com, acme@kernel.org, mark.rutland@arm.com,
        alexander.shishkin@linux.intel.com, jolsa@kernel.org,
        namhyung@kernel.org, irogers@google.com, leo.yan@linaro.org,
        eranian@google.com, linux-perf-users@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230427012841.231729-1-yangjihong1@huawei.com>
From:   Adrian Hunter <adrian.hunter@intel.com>
Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki,
 Business Identity Code: 0357606 - 4, Domiciled in Helsinki
In-Reply-To: <20230427012841.231729-1-yangjihong1@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@genusplc.com>
> Link: https://lore.kernel.org/lkml/CWLP265MB49702F7BA3D6D8F13E4B1A719C649@CWLP265MB4970.GBRP265.PROD.OUTLOOK.COM/T/
> Tested-by: Will Ochowicz <Will.Ochowicz@genusplc.com>
> Signed-off-by: Yang Jihong <yangjihong1@huawei.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@intel.com>

> ---
>  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;
>  			}
>  		}