Page MenuHomePhabricator

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

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.

sprunki phase 3
sprunki phase 3 a fan-made mod inspired by the original Incredibox game. Mix fresh beats, create unique music, and enjoy exclusive content online at sprunkiphase3.com

polytrack
Play PolyTrack, the ultimate low-poly racing game featuring loops, jumps, and high-speed action. Create, share, and race on custom tracks. Join the global PolyTrack community today!

Thank you for sharing this important technical information and solution. Looking forward to seeing further progress on this level devil issue!

sprunki phase 4
Sprunki Phase 4 is the latest update in the music rhythm game, offering new beats, characters, and challenges. You can play the game online for free at sprunkiphase4.net

saddlechow added a subscriber: saddlechow.EditedTue, Dec 24, 1:57 AM

Hi,

Thanks for sharing the detailed issue and your fix suggestion. It definitely sounds like a tricky situation with the undefined behavior in desc_add_ptr() and the alignment issue. You're right that with GCC 11.3, it's more likely to catch these types of issues due to stricter handling of alignment, whereas GCC 10.2 might have been more lenient, which explains why it worked before.

I took a look at your fix on GitHub, and it seems like a solid solution to the alignment problem. If the issue is still present in the ATF master branch, it would definitely be helpful for others who are using similar setups. I think it's worth raising a pull request with this fix, or at least opening an issue in the ATF repo to make the maintainers aware of the problem. The change looks like it addresses the root cause and should prevent any future crashes on newer compilers.

Additionally, it might also be worth checking if there are any compiler flags related to alignment or optimization that could help mitigate this issue in the short term, but your fix seems like the cleanest way to handle it.

Thanks again for bringing this up and sharing your solution. I'm sure it will be useful for others facing similar problems! ragdoll hit
Best,

Sprunki Pyramixed
Play Sprunki Pyramixed online. Sprunki Pyramixed is a very special mod for the incredibox sprunki game, it's worth a try.

https://sprunkiremastered.com/
Dive into the world of Sprunky, Play sprunki remastered! Mix, match, and create unique Sprunky beats with our quirky characters. Experience music creation like never before.