Menu
▾
▴
Re: [TrouSerS-tech] [PATCH 07/17] Fixed memory leak in ima_get_entry
|
From: Fuchs, A. <and...@si...> - 2014-04-11 13:40:56
|
Am Freitag, den 11.04.2014, 10:29 -0300 schrieb Richard:
> Em 11-04-2014 06:45, Fuchs, Andreas escreveu:
> > Disclaimer:
> > I could not complie-test or runtime-test these patches right now. This is a pure code-only review of the patches.
> >
> > I think, you missed removing one free in line 261. Otherwise, patch looks ok.
> That free is supposed to deal with the memory allocated with a calloc in
> line 253, in case the malloc - just above the error test - fails.
I know, but right now, you have two free(event) calls in the error case.
First, event is freed in line 261 and then after the "goto done" it is
freed again in line 341.
<extra>
By the way, shouldn't there be an error-test directly after the calloc
in line 253 as well ?!?
</extra>
> >
> > Am Mittwoch, den 09.04.2014, 15:41 -0300 schrieb rm...@li...:
> >> From: Richard Maciel <rm...@li...>
> >>
> >> Related to coverity CID 10311.
> >>
> >> In some error cases the memory allocated wasn't being properly released,
> >> so I grouped all the release in the end of the function and make error
> >> cases point to the label there.
> >>
> >> Signed-off-by: Richard Maciel <rm...@li...>
> >> ---
> >> src/tcs/tcs_evlog_imaem.c | 28 +++++++++++++---------------
> >> 1 file changed, 13 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/src/tcs/tcs_evlog_imaem.c b/src/tcs/tcs_evlog_imaem.c
> >> index d905381..abc5282 100644
> >> --- a/src/tcs/tcs_evlog_imaem.c
> >> +++ b/src/tcs/tcs_evlog_imaem.c
> >> @@ -244,14 +244,16 @@ ima_get_entry(FILE *handle, UINT32 pcr_index, UINT32 *num, TSS_PCR_EVENT **ppEve
> >> memcpy(&pcr_value, &page[ptr], sizeof(int));
> >>
> >> if (pcr_index == (UINT32)pcr_value) {
> >> - event = calloc(1, sizeof(TSS_PCR_EVENT));
> >> - event->ulPcrIndex = pcr_value;
> >> ptr += sizeof(int);
> >> /* This is the case where we're looking for a specific event number in a
> >> * specific PCR index. When we've reached the correct event, malloc
> >> * space for it, copy it in, then break out of the while loop */
> >> if (ppEvent && seen_indices == *num) {
> >> /* grab this entry */
> >> + event = calloc(1, sizeof(TSS_PCR_EVENT));
> >> + event->ulPcrIndex = pcr_value;
> >> + event->rgbPcrValue = NULL;
> >> + event->rgbEvent = NULL;
> >> event->ulPcrValueLength = 20;
> >> event->rgbPcrValue = malloc(event->ulPcrValueLength);
> >> if (event->rgbPcrValue == NULL) {
> >> @@ -270,26 +272,22 @@ ima_get_entry(FILE *handle, UINT32 pcr_index, UINT32 *num, TSS_PCR_EVENT **ppEve
> >> char digest[20];
> >>
> >> if (fread(&len, 1, sizeof(len), fp) != sizeof(len)) {
> >> - free(event);
> >> LogError("Failed to read event log file");
> >> result = TCSERR(TSS_E_INTERNAL_ERROR);
> >> goto done;
> >> }
> >> if (len > EVLOG_FILENAME_MAXSIZE) {
> >> - free(event);
> >> LogError("Event log file name too big! Max size is %d", EVLOG_FILENAME_MAXSIZE);
> >> result = TCSERR(TSS_E_INTERNAL_ERROR);
> >> goto done;
> >> }
> >> memset(name, 0, EVLOG_FILENAME_MAXSIZE);
> >> if (fread(name, 1, len, fp) != len) {
> >> - free(event);
> >> LogError("Failed to read event log file");
> >> result = TCSERR(TSS_E_INTERNAL_ERROR);
> >> goto done;
> >> }
> >> if (fread(digest, 1, sizeof(digest), fp) != sizeof(digest)) {
> >> - free(event);
> >> LogError("Failed to read event log file");
> >> result = TCSERR(TSS_E_INTERNAL_ERROR);
> >> goto done;
> >> @@ -297,24 +295,19 @@ ima_get_entry(FILE *handle, UINT32 pcr_index, UINT32 *num, TSS_PCR_EVENT **ppEve
> >> }
> >> /* Get the template data namelen and data */
> >> if (fread(&event->ulEventLength, 1, sizeof(int), fp) != sizeof(int)) {
> >> - free(event);
> >> LogError("Failed to read event log file");
> >> result = TCSERR(TSS_E_INTERNAL_ERROR);
> >> goto done;
> >> }
> >> event->rgbEvent = malloc(event->ulEventLength + 1);
> >> if (event->rgbEvent == NULL) {
> >> - free(event->rgbPcrValue);
> >> LogError("malloc of %u bytes failed.",
> >> event->ulEventLength);
> >> - free(event);
> >> result = TCSERR(TSS_E_OUTOFMEMORY);
> >> goto done;
> >> }
> >> memset(event->rgbEvent, 0, event->ulEventLength);
> >> if (fread(event->rgbEvent, 1, event->ulEventLength, fp) != event->ulEventLength ) {
> >> - free(event->rgbPcrValue);
> >> - free(event);
> >> LogError("Failed to read event log file");
> >> result = TCSERR(TSS_E_INTERNAL_ERROR);
> >> goto done;
> >> @@ -326,16 +319,12 @@ ima_get_entry(FILE *handle, UINT32 pcr_index, UINT32 *num, TSS_PCR_EVENT **ppEve
> >> }
> >> }
> >> if (fread(&len, 1, sizeof(len), fp) != sizeof(len)) {
> >> - free(event->rgbPcrValue);
> >> - free(event);
> >> LogError("Failed to read event log file");
> >> result = TCSERR(TSS_E_INTERNAL_ERROR);
> >> goto done;
> >> }
> >> fseek(fp, len + 20, SEEK_CUR);
> >> if (fread(&len, 1, sizeof(len), fp) != sizeof(len)) {
> >> - free(event->rgbPcrValue);
> >> - free(event);
> >> LogError("Failed to read event log file");
> >> result = TCSERR(TSS_E_INTERNAL_ERROR);
> >> goto done;
> >> @@ -344,6 +333,15 @@ ima_get_entry(FILE *handle, UINT32 pcr_index, UINT32 *num, TSS_PCR_EVENT **ppEve
> >> seen_indices++;
> >> }
> >> done:
> >> + if (result != TSS_SUCCESS) {
> >> + if (event != NULL) {
> >> + free(event->rgbPcrValue);
> >> + free(event->rgbEvent);
> >> + }
> >> + free(event);
> >> + event = NULL;
> >> + }
> >> +
> >> if (ppEvent == NULL)
> >> *num = seen_indices;
> >>
>
|