perf symbol-minimal: Fix ehdr reading in filename__read_build_id

[ Upstream commit ba0b7081f7a521d7c28b527a4f18666a148471e7 ]

The e_ident is part of the ehdr and so reading it a second time would
mean the read ehdr was displaced by 16-bytes. Switch from stdio to
open/read/lseek syscalls for similarity with the symbol-elf version of
the function and so that later changes can alter then open flags.

Fixes: fef8f648bb ("perf symbol: Fix use-after-free in filename__read_build_id")
Signed-off-by: Ian Rogers <irogers@google.com>
Link: https://lore.kernel.org/r/20250823000024.724394-2-irogers@google.com
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Ian Rogers 2025-08-22 17:00:23 -07:00 committed by Greg Kroah-Hartman
parent fbd4cf7ee4
commit 8d947540ee

View File

@ -4,7 +4,6 @@
#include <errno.h> #include <errno.h>
#include <unistd.h> #include <unistd.h>
#include <stdio.h>
#include <fcntl.h> #include <fcntl.h>
#include <string.h> #include <string.h>
#include <stdlib.h> #include <stdlib.h>
@ -88,11 +87,8 @@ int filename__read_debuglink(const char *filename __maybe_unused,
*/ */
int filename__read_build_id(const char *filename, struct build_id *bid) int filename__read_build_id(const char *filename, struct build_id *bid)
{ {
FILE *fp; int fd, ret = -1;
int ret = -1;
bool need_swap = false, elf32; bool need_swap = false, elf32;
u8 e_ident[EI_NIDENT];
int i;
union { union {
struct { struct {
Elf32_Ehdr ehdr32; Elf32_Ehdr ehdr32;
@ -103,28 +99,27 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
Elf64_Phdr *phdr64; Elf64_Phdr *phdr64;
}; };
} hdrs; } hdrs;
void *phdr; void *phdr, *buf = NULL;
size_t phdr_size; ssize_t phdr_size, ehdr_size, buf_size = 0;
void *buf = NULL;
size_t buf_size = 0;
fp = fopen(filename, "r"); fd = open(filename, O_RDONLY);
if (fp == NULL) if (fd < 0)
return -1; return -1;
if (fread(e_ident, sizeof(e_ident), 1, fp) != 1) if (read(fd, hdrs.ehdr32.e_ident, EI_NIDENT) != EI_NIDENT)
goto out; goto out;
if (memcmp(e_ident, ELFMAG, SELFMAG) || if (memcmp(hdrs.ehdr32.e_ident, ELFMAG, SELFMAG) ||
e_ident[EI_VERSION] != EV_CURRENT) hdrs.ehdr32.e_ident[EI_VERSION] != EV_CURRENT)
goto out; goto out;
need_swap = check_need_swap(e_ident[EI_DATA]); need_swap = check_need_swap(hdrs.ehdr32.e_ident[EI_DATA]);
elf32 = e_ident[EI_CLASS] == ELFCLASS32; elf32 = hdrs.ehdr32.e_ident[EI_CLASS] == ELFCLASS32;
ehdr_size = (elf32 ? sizeof(hdrs.ehdr32) : sizeof(hdrs.ehdr64)) - EI_NIDENT;
if (fread(elf32 ? (void *)&hdrs.ehdr32 : (void *)&hdrs.ehdr64, if (read(fd,
elf32 ? sizeof(hdrs.ehdr32) : sizeof(hdrs.ehdr64), (elf32 ? (void *)&hdrs.ehdr32 : (void *)&hdrs.ehdr64) + EI_NIDENT,
1, fp) != 1) ehdr_size) != ehdr_size)
goto out; goto out;
if (need_swap) { if (need_swap) {
@ -138,14 +133,18 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
hdrs.ehdr64.e_phnum = bswap_16(hdrs.ehdr64.e_phnum); hdrs.ehdr64.e_phnum = bswap_16(hdrs.ehdr64.e_phnum);
} }
} }
phdr_size = elf32 ? hdrs.ehdr32.e_phentsize * hdrs.ehdr32.e_phnum if ((elf32 && hdrs.ehdr32.e_phentsize != sizeof(Elf32_Phdr)) ||
: hdrs.ehdr64.e_phentsize * hdrs.ehdr64.e_phnum; (!elf32 && hdrs.ehdr64.e_phentsize != sizeof(Elf64_Phdr)))
goto out;
phdr_size = elf32 ? sizeof(Elf32_Phdr) * hdrs.ehdr32.e_phnum
: sizeof(Elf64_Phdr) * hdrs.ehdr64.e_phnum;
phdr = malloc(phdr_size); phdr = malloc(phdr_size);
if (phdr == NULL) if (phdr == NULL)
goto out; goto out;
fseek(fp, elf32 ? hdrs.ehdr32.e_phoff : hdrs.ehdr64.e_phoff, SEEK_SET); lseek(fd, elf32 ? hdrs.ehdr32.e_phoff : hdrs.ehdr64.e_phoff, SEEK_SET);
if (fread(phdr, phdr_size, 1, fp) != 1) if (read(fd, phdr, phdr_size) != phdr_size)
goto out_free; goto out_free;
if (elf32) if (elf32)
@ -153,8 +152,8 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
else else
hdrs.phdr64 = phdr; hdrs.phdr64 = phdr;
for (i = 0; i < elf32 ? hdrs.ehdr32.e_phnum : hdrs.ehdr64.e_phnum; i++) { for (int i = 0; i < (elf32 ? hdrs.ehdr32.e_phnum : hdrs.ehdr64.e_phnum); i++) {
size_t p_filesz; ssize_t p_filesz;
if (need_swap) { if (need_swap) {
if (elf32) { if (elf32) {
@ -180,8 +179,8 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
goto out_free; goto out_free;
buf = tmp; buf = tmp;
} }
fseek(fp, elf32 ? hdrs.phdr32[i].p_offset : hdrs.phdr64[i].p_offset, SEEK_SET); lseek(fd, elf32 ? hdrs.phdr32[i].p_offset : hdrs.phdr64[i].p_offset, SEEK_SET);
if (fread(buf, p_filesz, 1, fp) != 1) if (read(fd, buf, p_filesz) != p_filesz)
goto out_free; goto out_free;
ret = read_build_id(buf, p_filesz, bid, need_swap); ret = read_build_id(buf, p_filesz, bid, need_swap);
@ -194,7 +193,7 @@ out_free:
free(buf); free(buf);
free(phdr); free(phdr);
out: out:
fclose(fp); close(fd);
return ret; return ret;
} }