| Message ID | 20241205144439.127564-1-jmarchan@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | trace-cmd: Fix misc issues uncoverd by static analysis | expand |
On Thu, 5 Dec 2024 15:44:32 +0100 "Jerome Marchand" <jmarchan@redhat.com> wrote: > More issues were found by running static analysers on the code > of trace-cmd with openscanhub[1]. > > [1] https://fedoraproject.org/wiki/OpenScanHub > > Jerome Marchand (7): > trace-cmd lib: Close FDs in create_buffer_recorder_fd2 it allocation > fails > trace-cmd lib: Prevent a memory leak in tracecmd_tsync_with_guest() > trace-cmd lib: Prevent a leaked FD in > __tracecmd_create_buffer_recorder() > trace-cmd lib: Prevent memory leak in tracecmd_msg_wait_for_cmd() > trace-cmd sqlhist: Initialize name in trace_sqlhist() > trace-cmd: Fix memory leak in stop_mapping_vcpus() > trace-cmd record: Fix stdin redirection to /dev/null > I'm going to apply all your patches except the two that deal with file descriptors. The functions should not be closing file descriptors that they did not open. The real fix there is to remove where it does close the file descriptors and move the closing in the error paths of the callers. I must have gotten lazy and just let the functions do the closing when they failed, but that is just prone to bugs. -- Steve > lib/trace-cmd/trace-msg.c | 4 +++- > lib/trace-cmd/trace-recorder.c | 19 ++++++++++--------- > lib/trace-cmd/trace-timesync.c | 1 + > tracecmd/trace-record.c | 2 +- > tracecmd/trace-sqlhist.c | 2 +- > tracecmd/trace-tsync.c | 1 + > 6 files changed, 17 insertions(+), 12 deletions(-) >
On 18/12/2024 23:53, Steven Rostedt wrote: > On Thu, 5 Dec 2024 15:44:32 +0100 > "Jerome Marchand" <jmarchan@redhat.com> wrote: > >> More issues were found by running static analysers on the code >> of trace-cmd with openscanhub[1]. >> >> [1] https://fedoraproject.org/wiki/OpenScanHub >> >> Jerome Marchand (7): >> trace-cmd lib: Close FDs in create_buffer_recorder_fd2 it allocation >> fails >> trace-cmd lib: Prevent a memory leak in tracecmd_tsync_with_guest() >> trace-cmd lib: Prevent a leaked FD in >> __tracecmd_create_buffer_recorder() >> trace-cmd lib: Prevent memory leak in tracecmd_msg_wait_for_cmd() >> trace-cmd sqlhist: Initialize name in trace_sqlhist() >> trace-cmd: Fix memory leak in stop_mapping_vcpus() >> trace-cmd record: Fix stdin redirection to /dev/null >> > > I'm going to apply all your patches except the two that deal with file > descriptors. The functions should not be closing file descriptors that they > did not open. The real fix there is to remove where it does close the file > descriptors and move the closing in the error paths of the callers. > > I must have gotten lazy and just let the functions do the closing when they > failed, but that is just prone to bugs. That makes sense. I'll send updated fixes after the holidays break. Thanks, Jerome > > -- Steve > > >> lib/trace-cmd/trace-msg.c | 4 +++- >> lib/trace-cmd/trace-recorder.c | 19 ++++++++++--------- >> lib/trace-cmd/trace-timesync.c | 1 + >> tracecmd/trace-record.c | 2 +- >> tracecmd/trace-sqlhist.c | 2 +- >> tracecmd/trace-tsync.c | 1 + >> 6 files changed, 17 insertions(+), 12 deletions(-) >> >
More issues were found by running static analysers on the code of trace-cmd with openscanhub[1]. [1] https://fedoraproject.org/wiki/OpenScanHub Jerome Marchand (7): trace-cmd lib: Close FDs in create_buffer_recorder_fd2 it allocation fails trace-cmd lib: Prevent a memory leak in tracecmd_tsync_with_guest() trace-cmd lib: Prevent a leaked FD in __tracecmd_create_buffer_recorder() trace-cmd lib: Prevent memory leak in tracecmd_msg_wait_for_cmd() trace-cmd sqlhist: Initialize name in trace_sqlhist() trace-cmd: Fix memory leak in stop_mapping_vcpus() trace-cmd record: Fix stdin redirection to /dev/null lib/trace-cmd/trace-msg.c | 4 +++- lib/trace-cmd/trace-recorder.c | 19 ++++++++++--------- lib/trace-cmd/trace-timesync.c | 1 + tracecmd/trace-record.c | 2 +- tracecmd/trace-sqlhist.c | 2 +- tracecmd/trace-tsync.c | 1 + 6 files changed, 17 insertions(+), 12 deletions(-)