The behavior of create_buffer_recorder_fd2() wrt closing the file
descriptors is inconsistent. They aren't close if the function fails
early when allocating recorder, but they are closed in
tracecmd_free_recorder() if it fails later.
This cause use-after-free access when the caller tries to close the
FDs afterwards.
Always close the FDs in create_buffer_recorder_fd2() when it fails and
stop the caller to close them themselves.
Signed-off-by: Jerome Marchand <jmarchan@xxxxxxxxxx>
---
lib/trace-cmd/trace-recorder.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index 44f245d5..0413e529 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -114,8 +114,12 @@ create_buffer_recorder_fd2(int fd, int fd2, int cpu, unsigned flags,
bool nonblock = false;
recorder = malloc(sizeof(*recorder));
- if (!recorder)
+ if (!recorder) {
+ close(fd);
+ if (fd2 != -1)
+ close(fd2);
return NULL;
+ }
recorder->flags = flags;
@@ -204,12 +208,8 @@ __tracecmd_create_buffer_recorder(const char *file, int cpu, unsigned flags,
}
recorder = create_buffer_recorder_fd2(fd, fd2, cpu, flags, instance, maxkb, tfd);
- if (!recorder) {
- close(fd);
+ if (!recorder)
unlink(file);
- if (fd2 != -1)
- close(fd2);
- }
if (fd2 != -1) {
/* Unlink file2, we need to add everything to file at the end */
@@ -257,10 +257,9 @@ tracecmd_create_buffer_recorder_maxkb(const char *file, int cpu, unsigned flags,
free(file2);
return recorder;
- err2:
- close(fd2);
err:
close(fd);
+ err2:
unlink(file);
goto out;
}
--
2.47.0
|