This patch narrows down the permissions and bounds of initial capabilities pointing to argument and environment variable strings (elements of argv and envp).
This is a reduced version of Téo's patch ("fs: Handle exact bounds for argv and envp"), which had to be reverted as it could not handle empty strings. This version does not handle representability at all, as a result string capabilities may overlap. To properly handle string representability, it seems unavoidable to pass additional metadata to binfmt_elf, to clarify if and where padding was added by copy_strings().
Co-developed-by: Teo Couprie Diaz teo.coupriediaz@arm.com Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- fs/binfmt_elf.c | 59 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 14 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 1b3892cdd21d..2bf664fcc3c0 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -133,6 +133,37 @@ static int padzero(unsigned long address) return 0; }
+/* + * Put a pointer to a string on the stack, return the size of the string on the + * stack or an error. + */ +static long put_str_ptr(char __user *ustr, + elf_stack_item_t __user *stack_item) +{ + size_t len; + + len = strnlen_user(ustr, MAX_ARG_STRLEN); + if (!len || len > MAX_ARG_STRLEN) + return -EINVAL; + + /* + * TODO [PCuABI] - it is possible (though unlikely) that the string + * length makes it impossible to represent the bounds of ustr exactly. + * To avoid string pointers overlapping, padding should therefore be + * introduced at the point where strings are copied into the new + * process (copy_strings() in exec.c). Because strings can be empty + * (just one zero byte), the padding cannot simply be skipped here; some + * padding metadata should instead be made available by exec.c, for + * instance in struct linux_binprm. + */ + ustr = user_ptr_set_bounds(ustr, len); + + if (elf_stack_put_user(ustr, stack_item++)) + return -EFAULT; + + return len; +} + /* Let's use some macros to make this stack manipulation a little clearer */ #ifdef CONFIG_STACK_GROWSUP #define STACK_ADD(sp, items) ((sp) + (items) * sizeof(elf_stack_item_t)) @@ -539,14 +570,16 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec, /* Populate list of argv pointers back to argv strings. */ /* In PCuABI, this derives a capability from SP pointing to arg_start */ ustr = sp + (mm->arg_start - user_ptr_addr(sp)); +#if defined(CONFIG_CHERI_PURECAP_UABI) && (ELF_COMPAT == 0) + ustr = cheri_perms_and(ustr, CHERI_PERM_GLOBAL | + CHERI_PERM_LOAD | CHERI_PERM_STORE); +#endif while (argc-- > 0) { - size_t len; - if (elf_stack_put_user(ustr, stack_item++)) - return -EFAULT; - len = strnlen_user(ustr, MAX_ARG_STRLEN); - if (!len || len > MAX_ARG_STRLEN) - return -EINVAL; - ustr += len; + long str_ret = put_str_ptr(ustr, stack_item++); + + if (str_ret < 0) + return str_ret; + ustr += str_ret; } if (elf_stack_put_user(0, stack_item++)) return -EFAULT; @@ -555,13 +588,11 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec, /* Populate list of envp pointers back to envp strings. */ mm->env_start = user_ptr_addr(ustr); while (envc-- > 0) { - size_t len; - if (elf_stack_put_user(ustr, stack_item++)) - return -EFAULT; - len = strnlen_user(ustr, MAX_ARG_STRLEN); - if (!len || len > MAX_ARG_STRLEN) - return -EINVAL; - ustr += len; + long str_ret = put_str_ptr(ustr, stack_item++); + + if (str_ret < 0) + return str_ret; + ustr += str_ret; } if (elf_stack_put_user(0, stack_item++)) return -EFAULT;