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

CVE-2017-5715 AArch64 #2072

Merged
merged 5 commits into from
Jan 16, 2018
Merged

CVE-2017-5715 AArch64 #2072

merged 5 commits into from
Jan 16, 2018

Conversation

jenswi-linaro
Copy link
Contributor

No description provided.

@jenswi-linaro
Copy link
Contributor Author

There's some strace build error for the Hikey auto builder:

Makefile:249: recipe for target 'strace' failed
make: *** [strace] Error 1
make: *** Waiting for unfinished jobs....

Copy link
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

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

Nice and clean, IMO! I have only two minor comments. Anyway:

Reviewed-by: Jerome Forissier <[email protected]>
Tested-by: Jerome Forissier <[email protected]> (HiKey960)

(Note that HiKey960 has 4x Cortex-A73 and 4x Cortex-A53 so it is a good test for this code).

@@ -507,6 +529,117 @@ el0_serror_a32:
b el0_serror_a32
check_vector_size el0_serror_a32

#if defined(CFG_CORE_WORKAROUND_SPECTRE_BP_SEC)
.macro apply_workaround
Copy link
Contributor

Choose a reason for hiding this comment

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

apply_workaround is a bit too vague IMO. s/apply_workaround/ic_iallu_isb/? Or if you prefer s/apply_workaround/icache_invalidate/?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ARM-TF is mentioning a low overhead SMC interface for to invalidate the branch predictor, maybe that could be used later.
I guess s/apply_workaround/invalidate_branch_predictor/ would be good enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good

@@ -379,31 +379,31 @@ END_FUNC thread_unwind_user_mode
.endm

.section .text.thread_vect_table
.align 11
.align 11, 0
Copy link
Contributor

Choose a reason for hiding this comment

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

How could we make it more obvious that 0 is an invalid instruction? #define INV_INSN 0 maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

Commit subject: s/arm32/arm64

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix

@jenswi-linaro
Copy link
Contributor Author

Comments addressed, tags applied.

Moves MIDR definitions from arm32.h to arm.h

Reviewed-by: Jerome Forissier <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
Renames the labels in the exception vector to use consistent lower case
names.

Reviewed-by: Jerome Forissier <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
Pads exception vector with an illegal instruction to improve robustness.

Reviewed-by: Jerome Forissier <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
With CFG_CORE_UNMAP_CORE_AT_EL0=y the exception vector is updated to use
the minimal kernel mapping during user space execution. With this patch
vbar is updated relative to previous value in vbar to allow different
exception vectors for different cpu types.

Reviewed-by: Jerome Forissier <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
If build with CFG_CORE_WORKAROUND_SPECTRE_BP_SEC=y invalidate branch
predictor on all secure world exceptions originating in secure EL0
(secure user space).

Fixes CVE-2017-5715

Reviewed-by: Jerome Forissier <[email protected]>
Tested-by: Jerome Forissier <[email protected]> (HiKey960)
Signed-off-by: Jens Wiklander <[email protected]>
@jenswi-linaro
Copy link
Contributor Author

Rebased on master

@jforissier jforissier merged commit fecdfb7 into OP-TEE:master Jan 16, 2018
@jenswi-linaro jenswi-linaro deleted the cve-a64 branch January 16, 2018 14:30
jforissier added a commit to jforissier/optee_website that referenced this pull request Apr 19, 2018
- The Spectre v1 investigation found no vulnerability
- Mention the recent arm64 branch invalidation update included in
OP-TEE 3.1.0 (OP-TEE/optee_os#2229). The
previous security fix (OP-TEE/optee_os#2072)
was not effective as a mitigation against Spectre v2.

Signed-off-by: Jerome Forissier <[email protected]>
jforissier added a commit to jforissier/optee_website that referenced this pull request Apr 19, 2018
- The Spectre v1 investigation found no vulnerability
- Mention the recent arm64 branch invalidation update included in
OP-TEE 3.1.0 (OP-TEE/optee_os#2229). The
previous security fix (OP-TEE/optee_os#2072)
was not effective as a mitigation against Spectre v2.

Signed-off-by: Jerome Forissier <[email protected]>
Acked-by: Joakim Bech <[email protected]>
jforissier added a commit to jforissier/optee_website that referenced this pull request Apr 20, 2018
- The Spectre v1 investigation found no vulnerability
- Mention the recent arm64 branch invalidation update included in
OP-TEE 3.1.0 (OP-TEE/optee_os#2229). The
previous security fix (OP-TEE/optee_os#2072)
was not effective as a mitigation against Spectre v2.

Signed-off-by: Jerome Forissier <[email protected]>
Acked-by: Joakim Bech <[email protected]>
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.

2 participants