On 9/2/22 17:07, Kevin Brodsky wrote:
On 02/09/2022 11:45, Sherwin da Cruz wrote:
In the transitional ABI specific registers are initialised for purecap
This is true in PCuABI in general, the transitional ABI just doesn't specify them so much. Since you don't mention the transitional ABI in patch 2, I think it's fine not to mention it here either (the TODO is clear enough).
applications to make use of. This is set in start_thread(). The macro START_THREAD is specialised to allow returning the value of argc to be placed in x0.
Signed-off-by: Sherwin da Cruz sherwin.dacruz@arm.com
arch/arm64/include/asm/elf.h | 3 +++ arch/arm64/include/asm/processor.h | 30 ++++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h index 9eb86b4cb691..a666e1ccfee1 100644 --- a/arch/arm64/include/asm/elf.h +++ b/arch/arm64/include/asm/elf.h @@ -164,6 +164,9 @@ typedef unsigned long elf_greg_t; typedef elf_greg_t elf_gregset_t[ELF_NGREG]; typedef struct user_fpsimd_state elf_fpregset_t; +#define START_THREAD(elf_ex, regs, elf_entry, bprm) \
You have a mix of tab and spaces aligning , make sure to use just tabs.
- start_thread(regs, elf_entry, bprm->p, bprm->argc, bprm->envc)
- #define SET_PERSONALITY_AARCH64() \ ({ \ current->personality &= ~READ_IMPLIES_EXEC; \
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h index 5bdecb185f63..88de88ba506e 100644 --- a/arch/arm64/include/asm/processor.h +++ b/arch/arm64/include/asm/processor.h @@ -238,6 +238,31 @@ void tls_preserve_current_state(void); .fpsimd_cpu = NR_CPUS, \ } +static inline int init_gp_regs(struct pt_regs *regs, unsigned long sp,
int argc, int envc) {
{ is always at the start of a line when defining a function (you can use checkpatch.pl to find about this kind of style guide issues). Also, there are various styles used to indent hanging parameters in the kernel, but other functions in this file align them on the opening parenthesis so you should do the same here.
- int retval = 0;
+#ifdef CONFIG_CHERI_PURECAP_UABI
- unsigned long argv, envp, auxv;
- /*
* TODO [PCuABI]: When argv/envp/auxv is moved off the stack,
update the registers
* x1-x3 with the new pointer values, and c1-c3 with new
capabilities to them.
Alright so there are two things here really: 1. when the initial args are moved, as you mention, and 2. restricting bounds / permissions. 1. is about x1-x3, and 2. is about c1-c3. The two are not interdependent - you don't have to do both at the same time. Let's make it clear here.
I think I'll specify also that currently c1-c3 is taken from csp, so changing c1-c3 would be needed if csp has already been restricted to the bounds of the stack and the the data has been moved elsewhere.
Sherwin
*
* In ret_to_user c regs are first loaded then merged with x regs
if their values
* are different. Hence we load capabilities in c regs and the
value in x regs.
*/
- retval = argc; /* Placed in x0 */
- argv = sp + 1 * sizeof(user_uintptr_t); /* Increment past argc on
stack */
- envp = argv + (argc + 1) * sizeof(user_uintptr_t); /* Go past arg
vals + NULL */
- auxv = envp + (envc + 1) * sizeof(user_uintptr_t); /* Go past env
vals + NULL */
- regs->cregs[1] = regs->cregs[2] = regs->cregs[3] = regs->csp;
- regs->regs[1] = argv;
- regs->regs[2] = envp;
- regs->regs[3] = auxv;
+#endif /* CONFIG_CHERI_PURECAP_UABI */
- return retval;
+}
- static inline void start_thread_common(struct pt_regs *regs,
unsigned long pc) { memset(regs, 0, sizeof(*regs)); @@ -248,8 +273,8 @@ static inline void start_thread_common(struct pt_regs *regs, unsigned long pc) regs->pmr_save = GIC_PRIO_IRQON; } -static inline void start_thread(struct pt_regs *regs, unsigned long pc,
unsigned long sp)
+static inline int start_thread(struct pt_regs *regs, unsigned long pc,
{ start_thread_common(regs, pc); regs->pstate = PSR_MODE_EL0t;unsigned long sp, int argc, int envc)
@@ -258,6 +283,7 @@ static inline void start_thread(struct pt_regs *regs, unsigned long pc, if (system_supports_morello()) morello_thread_start(regs, pc);
Nit: would look nicer with an empty line here.
Kevin
- return init_gp_regs(regs, sp, argc, envc); } #ifdef CONFIG_COMPAT
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.