Page MenuHomePhabricator
Create Task
Maniphest T1010

nxp/crypto/caam (formerly fsl_sec): Type-punning undefined behavior leads to unaligned access in BL31 firmware and unhandled exception in EL3
Open, Needs TriagePublic
Actions

Description

Hello,

we are using atf v1.5 (from NXP's Layerscape SDK 20.12) on a custom LX2160A board (aarch64) and noticed a crash in the BL31 runtime from atf when compiling with gcc 11.3 (from Yocto kirkstone).

The problem is that plat/nxp/drivers/fsl_sec/src/jobdesc.c contains undefined behaviour in desc_add_ptr(), which gets inlined in cnstr_rng_jobdesc(). desc_add_ptr() uses u64 * pointers to write the 64bit data pointer to the u32 *desc pointer at offsets 3 and 4, and gcc 11.3 generates a 64bit store instruction (STUR) for this. But since it's offset 3 it's unaligned and causes an "unhandled exception in EL3" when U-Boot tries to get a random number from the BL31 firmware via smc_call().

Previously with gcc 10.2 from Yocto hardknott this didn't happen, but since it is undefined behaviour, I suppose we just got lucky.

I have a fix suggestion here [1] which solves the issue on our custom board. Since the problematic code still exists in atf master (at drivers/nxp/crypto/caam/src/jobdesc.c), maybe it would still be useful?

[1] https://github.com/dkl/arm-trusted-firmware/commit/2351fb9b9db9b97aab1fb3a18221aadbe886f18a

Event Timeline

dkl created this task.Dec 7 2022, 10:37 AM
dkl added a comment.Dec 7 2022, 12:51 PM

Found a fix in atf master which seems to touch on exactly this issue: https://github.com/ARM-software/arm-trusted-firmware/commit/fa7fdfabf07d91439b0869ffd8e805f0166294bf

Although I'm not sure whether that's a complete fix, since only the ptr_addr_t struct for ptr_addr_t *ptr_addr was fixed, but the issue with the unaligned phys_addr_t *last pointer remains.