| Message ID | 20240605134054.2626953-30-jmarchan@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | trace-cmd: fix misc issues found by static analysis | expand |
On Wed, 5 Jun 2024 15:40:44 +0200 "Jerome Marchand" <jmarchan@redhat.com> wrote: > Because of the again label, msg_handle can be already allocated if we > exit after we got a negative socket file descriptor. Free it there. > > Fixes a RESOURCE_LEAK error (CWE-772) > > Signed-off-by: Jerome Marchand <jmarchan@redhat.com> > --- > tracecmd/trace-record.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > index b4cbd438..770e775b 100644 > --- a/tracecmd/trace-record.c > +++ b/tracecmd/trace-record.c > @@ -3903,6 +3903,7 @@ static struct tracecmd_msg_handle *setup_network(struct buffer_instance *instanc > > if (sfd < 0) { > free(thost); > + free(msg_handle); As msg_handle is created with: msg_handle = tracecmd_msg_handle_alloc(sfd, 0); This should be: trace_msg_handle_close(msg_handle); But may require: close(sfd); + msg_handle->fd = -1; free(host); host = NULL; goto again; I'll skip this patch. -- Steve > return NULL; > } >
On 18/07/2024 02:25, Steven Rostedt wrote: > On Wed, 5 Jun 2024 15:40:44 +0200 > "Jerome Marchand" <jmarchan@redhat.com> wrote: > >> Because of the again label, msg_handle can be already allocated if we >> exit after we got a negative socket file descriptor. Free it there. >> >> Fixes a RESOURCE_LEAK error (CWE-772) >> >> Signed-off-by: Jerome Marchand <jmarchan@redhat.com> >> --- >> tracecmd/trace-record.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c >> index b4cbd438..770e775b 100644 >> --- a/tracecmd/trace-record.c >> +++ b/tracecmd/trace-record.c >> @@ -3903,6 +3903,7 @@ static struct tracecmd_msg_handle *setup_network(struct buffer_instance *instanc >> >> if (sfd < 0) { >> free(thost); >> + free(msg_handle); > > As msg_handle is created with: > > msg_handle = tracecmd_msg_handle_alloc(sfd, 0); > > This should be: > > trace_msg_handle_close(msg_handle); > > But may require: > > close(sfd); > + msg_handle->fd = -1; > free(host); > host = NULL; > goto again; > > I'll skip this patch. I'll send a fixed version. Jerome > > -- Steve > > >> return NULL; >> } >>
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index b4cbd438..770e775b 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -3903,6 +3903,7 @@ static struct tracecmd_msg_handle *setup_network(struct buffer_instance *instanc if (sfd < 0) { free(thost); + free(msg_handle); return NULL; }
Because of the again label, msg_handle can be already allocated if we exit after we got a negative socket file descriptor. Free it there. Fixes a RESOURCE_LEAK error (CWE-772) Signed-off-by: Jerome Marchand <jmarchan@redhat.com> --- tracecmd/trace-record.c | 1 + 1 file changed, 1 insertion(+)