Re: [PATCH v2 bpf-next 02/10] lib/buildid: take into account e_phoff when fetching program headers

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, Jul 24, 2024 at 03:52:02PM -0700, Andrii Nakryiko wrote:
> Current code assumption is that program (segment) headers are following
> ELF header immediately. This is a common case, but is not guaranteed. So
> take into account e_phoff field of the ELF header when accessing program
> headers.
> 
> Reported-by: Alexey Dobriyan <adobriyan@gmail.com>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

looks like this one never got in right?
  https://lore.kernel.org/bpf/CAEf4BzaAKAwO=-=0qZQfkHhBodN0MQUHpL-RY7tCHdcFidjv-Q@mail.gmail.com/

I couldn't find the place where you remove that check ;-)

jirka

> ---
>  lib/buildid.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/buildid.c b/lib/buildid.c
> index 1442a2483a8b..ce48ffab4111 100644
> --- a/lib/buildid.c
> +++ b/lib/buildid.c
> @@ -206,7 +206,7 @@ static int get_build_id_32(struct freader *r, unsigned char *build_id, __u32 *si
>  {
>  	const Elf32_Ehdr *ehdr;
>  	const Elf32_Phdr *phdr;
> -	__u32 phnum, i;
> +	__u32 phnum, phoff, i;
>  
>  	ehdr = freader_fetch(r, 0, sizeof(Elf32_Ehdr));
>  	if (!ehdr)
> @@ -214,13 +214,14 @@ static int get_build_id_32(struct freader *r, unsigned char *build_id, __u32 *si
>  
>  	/* subsequent freader_fetch() calls invalidate pointers, so remember locally */
>  	phnum = ehdr->e_phnum;
> +	phoff = READ_ONCE(ehdr->e_phoff);
>  
>  	/* only supports phdr that fits in one page */
>  	if (phnum > (PAGE_SIZE - sizeof(Elf32_Ehdr)) / sizeof(Elf32_Phdr))
>  		return -EINVAL;
>  
>  	for (i = 0; i < phnum; ++i) {
> -		phdr = freader_fetch(r, i * sizeof(Elf32_Phdr), sizeof(Elf32_Phdr));
> +		phdr = freader_fetch(r, phoff + i * sizeof(Elf32_Phdr), sizeof(Elf32_Phdr));
>  		if (!phdr)
>  			return r->err;
>  
> @@ -237,6 +238,7 @@ static int get_build_id_64(struct freader *r, unsigned char *build_id, __u32 *si
>  	const Elf64_Ehdr *ehdr;
>  	const Elf64_Phdr *phdr;
>  	__u32 phnum, i;
> +	__u64 phoff;
>  
>  	ehdr = freader_fetch(r, 0, sizeof(Elf64_Ehdr));
>  	if (!ehdr)
> @@ -244,13 +246,14 @@ static int get_build_id_64(struct freader *r, unsigned char *build_id, __u32 *si
>  
>  	/* subsequent freader_fetch() calls invalidate pointers, so remember locally */
>  	phnum = ehdr->e_phnum;
> +	phoff = READ_ONCE(ehdr->e_phoff);
>  
>  	/* only supports phdr that fits in one page */
>  	if (phnum > (PAGE_SIZE - sizeof(Elf64_Ehdr)) / sizeof(Elf64_Phdr))
>  		return -EINVAL;
>  
>  	for (i = 0; i < phnum; ++i) {
> -		phdr = freader_fetch(r, i * sizeof(Elf64_Phdr), sizeof(Elf64_Phdr));
> +		phdr = freader_fetch(r, phoff + i * sizeof(Elf64_Phdr), sizeof(Elf64_Phdr));
>  		if (!phdr)
>  			return r->err;
>  
> -- 
> 2.43.0
> 
>