Hi everyone, Here is the latest revision to make the capabilities to argv and envp strings properly restricted. All my questions have been answered along the different revisions, so hopefully we are getting close to a final patch !
Notable this revision: I changed some comment wordings due to the few reworks of the logic and code removed.
Thanks in advance, Téo
# Changes from v3-v4[0] - Some comment and commit message clarifcations - Change PC-uABI to PCuABI - Do capability representation operations unconditionally - Putting the capability/pointer to arg strings on the ELF stack is now common between purecap and compat - Fix off by one error in padding traversal - Swap padding traversal while and pre-checks to a single for
# Changes from v2-v3[1] - Commit message clarified - Fixed some typing and use issues - Updated the stack alignment to use a define depending on the build type - Changed the put_str_array function to only put one string pointer, the looping over all strings is restored to be handled in create_elf_tables. - Moved the padding calculations to a separate function that returns the updated value of bprm->p rather than the padding. - Reduced the amount of code not shared between purecap/compat - Removed nonsensical check before alignment computation
# Changes from v1-v2[2]
- Rebased on top of the last release - Update to make use of the properly derived stack capability in binfmt_elf - Move the copying of argv and envp strings in binfmt_elf to a helper function - Check for padding after an arg only - Greatly simplify the change in exec by completely skipping the padding, rather than looping through it and allocating pages in exec - Add more details to the comments explaining the padding process - Rename most variables. I'm not great with names so hopefully they are OK, otherwise feel free to suggest new ones ! - Proper COMPAT handling, but with a slight loss compared to a regular kernel - Detail trade-off in comments and in commit message - Force a greater stack alignment in exec to mitigate issues during relocation - Simplify accesses and use `get_user()` rather than `copy_from_user()` - Use wrappers provided by <cheriintrin.h> rather than builtins - Get rid of the elf_stack_put_user_cap macro
Gitlab patch for review : https://git.morello-project.org/Teo-CD/linux/-/commit/5527a39f6e3cd0eb39cc43...
[0]: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/... [1]: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/... [2]: https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/... Teo Couprie Diaz (1): fs: Handle exact bounds for argv and envp
fs/binfmt_elf.c | 88 +++++++++++++++++++++++++++++++++++++++++-------- fs/exec.c | 55 +++++++++++++++++++++++++++++-- 2 files changed, 126 insertions(+), 17 deletions(-)