Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix memory leak (start enum) #497

Merged
1 commit merged into from
Oct 13, 2015
Merged

Fix memory leak (start enum) #497

1 commit merged into from
Oct 13, 2015

Conversation

cedric-chaumont-st-dev
Copy link

Signed-off-by: Cedric Chaumont cedric.chaumont@st.com

Tested-by: Cedric Chaumont cedric.chaumont@linaro.org (STM boards)

jenswi-linaro
jenswi-linaro reviewed Oct 12, 2015
View reviewed changes
core/tee/tee_svc_storage.c
@@ -978,6 +978,7 @@ TEE_Result tee_svc_storage_start_enum(uint32_t obj_enum, uint32_t storage_id)
if (res != TEE_SUCCESS)
goto exit;
res = tee_obj_verify(sess, o);
free(o->pobj->obj_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The need for this free is a bit obscure, please add a comment explaining why it's needed here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@jforissier
Copy link
Contributor

Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey)

I think you should add 'Fixes #494' to the commit log.

@cedric-chaumont-st-dev
Copy link
Author

Update commit message.

@cedric-chaumont-st-dev cedric-chaumont-st-dev force-pushed the dev/fix_start_enum_mem_leak branch 2 times, most recently from 53e82ad to ac23c78 Compare October 13, 2015 08:36
@cedric-chaumont-st-dev
Copy link
Author

Update.

@pascal-brand38
Copy link
Contributor 

Reviewed-by: Pascal Brand <pascal.brand@linaro.org>

jforissier added a commit to jforissier/hikey_optee that referenced this pull request Oct 13, 2015
@jforissier
Use latest optee_os master merged with PR#497
e0b7abc 
PR#497 ("Fix memory leak (start enum)")
OP-TEE/optee_os#497

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
jenswi-linaro
jenswi-linaro reviewed Oct 13, 2015
View reviewed changes
core/tee/tee_svc_storage.c
@@ -980,6 +980,8 @@ TEE_Result tee_svc_storage_start_enum(uint32_t obj_enum, uint32_t storage_id)
res = tee_obj_verify(sess, o);
if (res != TEE_SUCCESS)
goto exit;
free(o->pobj->obj_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The need for this free is a bit obscure, please add a comment explaining why it's needed here.

@cedric-chaumont-st-dev
Copy link
Author

@jenswi-linaro : Add comments in the code. Is it OK with you ?

@jforissier
Copy link
Contributor

I'm not sure that 'Fix#494' in the commit log will be recognized by GitHub for linking and closing the issue automatically, see https://help.github.com/articles/closing-issues-via-commit-messages/

@jenswi-linaro
Copy link
Contributor 

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

@jforissier
Copy link
Contributor

@cedric-chaumont-st-dev you have modified the patch after I have tested it (you changed the code structure) and still you are keeping my Tested-by. You should not do this.

@cedric-chaumont-st-dev cedric-chaumont-st-dev force-pushed the dev/fix_start_enum_mem_leak branch 2 times, most recently from 555127c to 4a893a4 Compare October 13, 2015 09:21
@cedric-chaumont-st-dev
Copy link
Author

updated

@jforissier
Copy link
Contributor

Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey)

@cedric-chaumont-st
Fix memory leak (start enum)
35ade1d 
Enumeration loop added for object corruption.
Add missing free because of tee_svc_storage_set_enum
obj_id memory allocation (malloc) during enumeration loop.
Force obj_id to NULL in the enumation loop to skip freeing
at 'exit' label statement.
closes OP-TEE#494

Signed-off-by: Cedric Chaumont <cedric.chaumont@st.com>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Pascal Brand <pascal.brand@linaro.org>
Tested-by: Cedric Chaumont <cedric.chaumont@linaro.org> (STM boards)
Tested-by: Cedric Chaumont <cedric.chaumont@linaro.org> (ARM Juno board)
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey)
@cedric-chaumont-st-dev
Copy link
Author

Update commit message. Verified (STM board, ARM Juno)
48211 subtests of which 0 failed
701 test cases of which 0 failed
0 test case was skipped
TEE test application done!

@ghost ghost merged commit 35ade1d into OP-TEE:master Oct 13, 2015
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cedric-chaumont-st-dev @jforissier @pascal-brand38 @jenswi-linaro @cedric-chaumont-st