Skip to content

Commit fef8f64

Browse files
captain5050acmel
authored andcommitted
perf symbol: Fix use-after-free in filename__read_build_id
The same buf is used for the program headers and reading notes. As the notes memory may be reallocated then this corrupts the memory pointed to by the phdr. Using the same buffer is in any case a logic error. Rather than deal with the duplicated code, introduce an elf32 boolean and a union for either the elf32 or elf64 headers that are in use. Let the program headers have their own memory and grow the buffer for notes as necessary. Before `perf list -j` compiled with asan would crash with: ``` ==4176189==ERROR: AddressSanitizer: heap-use-after-free on address 0x5160000070b8 at pc 0x555d3b15075b bp 0x7ffebb5a8090 sp 0x7ffebb5a8088 READ of size 8 at 0x5160000070b8 thread T0 #0 0x555d3b15075a in filename__read_build_id tools/perf/util/symbol-minimal.c:212:25 #1 0x555d3ae43aff in filename__sprintf_build_id tools/perf/util/build-id.c:110:8 ... 0x5160000070b8 is located 312 bytes inside of 560-byte region [0x516000006f80,0x5160000071b0) freed by thread T0 here: #0 0x555d3ab21840 in realloc (perf+0x264840) (BuildId: 12dff2f6629f738e5012abdf0e90055518e70b5e) #1 0x555d3b1506e7 in filename__read_build_id tools/perf/util/symbol-minimal.c:206:11 ... previously allocated by thread T0 here: #0 0x555d3ab21423 in malloc (perf+0x264423) (BuildId: 12dff2f6629f738e5012abdf0e90055518e70b5e) #1 0x555d3b1503a2 in filename__read_build_id tools/perf/util/symbol-minimal.c:182:9 ... ``` Note: this bug is long standing and not introduced by the other asan fix in commit fa9c497 ("perf symbol-minimal: Fix double free in filename__read_build_id"). Fixes: b691f64 ("perf symbols: Implement poor man's ELF parser") Signed-off-by: Ian Rogers <[email protected]> Link: https://lore.kernel.org/r/[email protected] Cc: Mark Rutland <[email protected]> Cc: Gary Guo <[email protected]> Cc: Alex Gaynor <[email protected]> Cc: Boqun Feng <[email protected]> Cc: Howard Chu <[email protected]> Cc: Alice Ryhl <[email protected]> Cc: Dmitry Vyukov <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Adrian Hunter <[email protected]> Cc: Weilin Wang <[email protected]> Cc: Andreas Hindborg <[email protected]> Cc: Arnaldo Carvalho de Melo <[email protected]> Cc: Danilo Krummrich <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Namhyung Kim <[email protected]> Cc: Miguel Ojeda <[email protected]> Cc: James Clark <[email protected]> Cc: Jiapeng Chong <[email protected]> Cc: Andi Kleen <[email protected]> Cc: Alexander Shishkin <[email protected]> Cc: Kan Liang <[email protected]> Cc: Stephen Brennan <[email protected]> Cc: Benno Lossin <[email protected]> Cc: Björn Roy Baron <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Trevor Gross <[email protected]> Cc: [email protected] Cc: [email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent 2a2a7f5 commit fef8f64

File tree

1 file changed

+70
-98
lines changed

1 file changed

+70
-98
lines changed

tools/perf/util/symbol-minimal.c

Lines changed: 70 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,23 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
9090
{
9191
FILE *fp;
9292
int ret = -1;
93-
bool need_swap = false;
93+
bool need_swap = false, elf32;
9494
u8 e_ident[EI_NIDENT];
95-
size_t buf_size;
96-
void *buf;
9795
int i;
96+
union {
97+
struct {
98+
Elf32_Ehdr ehdr32;
99+
Elf32_Phdr *phdr32;
100+
};
101+
struct {
102+
Elf64_Ehdr ehdr64;
103+
Elf64_Phdr *phdr64;
104+
};
105+
} hdrs;
106+
void *phdr;
107+
size_t phdr_size;
108+
void *buf = NULL;
109+
size_t buf_size = 0;
98110

99111
fp = fopen(filename, "r");
100112
if (fp == NULL)
@@ -108,119 +120,79 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
108120
goto out;
109121

110122
need_swap = check_need_swap(e_ident[EI_DATA]);
123+
elf32 = e_ident[EI_CLASS] == ELFCLASS32;
111124

112-
/* for simplicity */
113-
fseek(fp, 0, SEEK_SET);
114-
115-
if (e_ident[EI_CLASS] == ELFCLASS32) {
116-
Elf32_Ehdr ehdr;
117-
Elf32_Phdr *phdr;
118-
119-
if (fread(&ehdr, sizeof(ehdr), 1, fp) != 1)
120-
goto out;
125+
if (fread(elf32 ? (void *)&hdrs.ehdr32 : (void *)&hdrs.ehdr64,
126+
elf32 ? sizeof(hdrs.ehdr32) : sizeof(hdrs.ehdr64),
127+
1, fp) != 1)
128+
goto out;
121129

122-
if (need_swap) {
123-
ehdr.e_phoff = bswap_32(ehdr.e_phoff);
124-
ehdr.e_phentsize = bswap_16(ehdr.e_phentsize);
125-
ehdr.e_phnum = bswap_16(ehdr.e_phnum);
130+
if (need_swap) {
131+
if (elf32) {
132+
hdrs.ehdr32.e_phoff = bswap_32(hdrs.ehdr32.e_phoff);
133+
hdrs.ehdr32.e_phentsize = bswap_16(hdrs.ehdr32.e_phentsize);
134+
hdrs.ehdr32.e_phnum = bswap_16(hdrs.ehdr32.e_phnum);
135+
} else {
136+
hdrs.ehdr64.e_phoff = bswap_64(hdrs.ehdr64.e_phoff);
137+
hdrs.ehdr64.e_phentsize = bswap_16(hdrs.ehdr64.e_phentsize);
138+
hdrs.ehdr64.e_phnum = bswap_16(hdrs.ehdr64.e_phnum);
126139
}
140+
}
141+
phdr_size = elf32 ? hdrs.ehdr32.e_phentsize * hdrs.ehdr32.e_phnum
142+
: hdrs.ehdr64.e_phentsize * hdrs.ehdr64.e_phnum;
143+
phdr = malloc(phdr_size);
144+
if (phdr == NULL)
145+
goto out;
127146

128-
buf_size = ehdr.e_phentsize * ehdr.e_phnum;
129-
buf = malloc(buf_size);
130-
if (buf == NULL)
131-
goto out;
132-
133-
fseek(fp, ehdr.e_phoff, SEEK_SET);
134-
if (fread(buf, buf_size, 1, fp) != 1)
135-
goto out_free;
136-
137-
for (i = 0, phdr = buf; i < ehdr.e_phnum; i++, phdr++) {
138-
void *tmp;
139-
long offset;
140-
141-
if (need_swap) {
142-
phdr->p_type = bswap_32(phdr->p_type);
143-
phdr->p_offset = bswap_32(phdr->p_offset);
144-
phdr->p_filesz = bswap_32(phdr->p_filesz);
145-
}
146-
147-
if (phdr->p_type != PT_NOTE)
148-
continue;
149-
150-
offset = phdr->p_offset;
151-
if (phdr->p_filesz > buf_size) {
152-
buf_size = phdr->p_filesz;
153-
tmp = realloc(buf, buf_size);
154-
if (tmp == NULL)
155-
goto out_free;
156-
buf = tmp;
157-
}
158-
fseek(fp, offset, SEEK_SET);
159-
if (fread(buf, phdr->p_filesz, 1, fp) != 1)
160-
goto out_free;
147+
fseek(fp, elf32 ? hdrs.ehdr32.e_phoff : hdrs.ehdr64.e_phoff, SEEK_SET);
148+
if (fread(phdr, phdr_size, 1, fp) != 1)
149+
goto out_free;
161150

162-
ret = read_build_id(buf, phdr->p_filesz, bid, need_swap);
163-
if (ret == 0) {
164-
ret = bid->size;
165-
break;
166-
}
167-
}
168-
} else {
169-
Elf64_Ehdr ehdr;
170-
Elf64_Phdr *phdr;
151+
if (elf32)
152+
hdrs.phdr32 = phdr;
153+
else
154+
hdrs.phdr64 = phdr;
171155

172-
if (fread(&ehdr, sizeof(ehdr), 1, fp) != 1)
173-
goto out;
156+
for (i = 0; i < elf32 ? hdrs.ehdr32.e_phnum : hdrs.ehdr64.e_phnum; i++) {
157+
size_t p_filesz;
174158

175159
if (need_swap) {
176-
ehdr.e_phoff = bswap_64(ehdr.e_phoff);
177-
ehdr.e_phentsize = bswap_16(ehdr.e_phentsize);
178-
ehdr.e_phnum = bswap_16(ehdr.e_phnum);
160+
if (elf32) {
161+
hdrs.phdr32[i].p_type = bswap_32(hdrs.phdr32[i].p_type);
162+
hdrs.phdr32[i].p_offset = bswap_32(hdrs.phdr32[i].p_offset);
163+
hdrs.phdr32[i].p_filesz = bswap_32(hdrs.phdr32[i].p_offset);
164+
} else {
165+
hdrs.phdr64[i].p_type = bswap_32(hdrs.phdr64[i].p_type);
166+
hdrs.phdr64[i].p_offset = bswap_64(hdrs.phdr64[i].p_offset);
167+
hdrs.phdr64[i].p_filesz = bswap_64(hdrs.phdr64[i].p_filesz);
168+
}
179169
}
170+
if ((elf32 ? hdrs.phdr32[i].p_type : hdrs.phdr64[i].p_type) != PT_NOTE)
171+
continue;
180172

181-
buf_size = ehdr.e_phentsize * ehdr.e_phnum;
182-
buf = malloc(buf_size);
183-
if (buf == NULL)
184-
goto out;
185-
186-
fseek(fp, ehdr.e_phoff, SEEK_SET);
187-
if (fread(buf, buf_size, 1, fp) != 1)
188-
goto out_free;
189-
190-
for (i = 0, phdr = buf; i < ehdr.e_phnum; i++, phdr++) {
173+
p_filesz = elf32 ? hdrs.phdr32[i].p_filesz : hdrs.phdr64[i].p_filesz;
174+
if (p_filesz > buf_size) {
191175
void *tmp;
192-
long offset;
193-
194-
if (need_swap) {
195-
phdr->p_type = bswap_32(phdr->p_type);
196-
phdr->p_offset = bswap_64(phdr->p_offset);
197-
phdr->p_filesz = bswap_64(phdr->p_filesz);
198-
}
199-
200-
if (phdr->p_type != PT_NOTE)
201-
continue;
202176

203-
offset = phdr->p_offset;
204-
if (phdr->p_filesz > buf_size) {
205-
buf_size = phdr->p_filesz;
206-
tmp = realloc(buf, buf_size);
207-
if (tmp == NULL)
208-
goto out_free;
209-
buf = tmp;
210-
}
211-
fseek(fp, offset, SEEK_SET);
212-
if (fread(buf, phdr->p_filesz, 1, fp) != 1)
177+
buf_size = p_filesz;
178+
tmp = realloc(buf, buf_size);
179+
if (tmp == NULL)
213180
goto out_free;
181+
buf = tmp;
182+
}
183+
fseek(fp, elf32 ? hdrs.phdr32[i].p_offset : hdrs.phdr64[i].p_offset, SEEK_SET);
184+
if (fread(buf, p_filesz, 1, fp) != 1)
185+
goto out_free;
214186

215-
ret = read_build_id(buf, phdr->p_filesz, bid, need_swap);
216-
if (ret == 0) {
217-
ret = bid->size;
218-
break;
219-
}
187+
ret = read_build_id(buf, p_filesz, bid, need_swap);
188+
if (ret == 0) {
189+
ret = bid->size;
190+
break;
220191
}
221192
}
222193
out_free:
223194
free(buf);
195+
free(phdr);
224196
out:
225197
fclose(fp);
226198
return ret;

0 commit comments

Comments
 (0)