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.

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

core: fix data abort during ftrace #6378

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

jenswi-linaro
Copy link
Contributor

With commit c10e3fa ("core: fix race in handling TA panic") the resources of a panicked TAs is released as early as possible, including the user space mapped ftrace buffer. However, the pointer to the ftrace buffer is stored in the ts_session for quick and easy access. The ftrace buffer is always retrieved with get_fbuf() that already have a few other checks to see if the buffer is currently available. So add a check to see that the TA hasn't panicked also.

Fixes: c10e3fa ("core: fix race in handling TA panic")

@jenswi-linaro
Copy link
Contributor Author

Force push to please checkpatch.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>

@jforissier
Copy link
Contributor

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

With commit c10e3fa ("core: fix race in handling TA panic") the
resources of a panicked TAs are released as early as possible, including
the user space mapped ftrace buffer. However, the pointer to the ftrace
buffer is stored in the ts_session for quick and easy access. The ftrace
buffer is always retrieved with get_fbuf() that already have a few other
checks to see if the buffer is currently available. So add a check to
see that the TA hasn't panicked also.

Fixes: c10e3fa ("core: fix race in handling TA panic")
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (vexpress-qemu_armv8a)
@jenswi-linaro
Copy link
Contributor Author

Tags applied. Did a s/is released/are released/ in the commit message.

@b49020
Copy link
Contributor

b49020 commented Oct 13, 2023

Does this change disallows ftrace dump for user TAs which have panicked?

@jforissier
Copy link
Contributor

Does this change disallows ftrace dump for user TAs which have panicked?

No. I could obtain a dump for e.g., xtest regression_1007 which is a panic test.

@jforissier jforissier merged commit 0a75d40 into OP-TEE:master Oct 13, 2023
8 checks passed
jforissier added a commit to jforissier/optee_release_tests that referenced this pull request Oct 13, 2023
Enable test 11 again since the issue waw fixed in [1].

Link: OP-TEE/optee_os#6378 [1].
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
@jenswi-linaro jenswi-linaro deleted the fix_ftrace branch October 14, 2023 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants