On 8/7/21 11:42 AM, Warner Losh wrote:
> +static inline int setup_initial_stack(struct bsd_binprm *bprm,
> + abi_ulong *ret_addr, abi_ulong *stringp)
> +{
> + int i;
> + abi_ulong stack_hi_addr;
> + size_t execpath_len, stringspace;
> + abi_ulong destp, argvp, envp, p;
> + struct target_ps_strings ps_strs;
> + char canary[sizeof(abi_long) * 8];
> +
> + stack_hi_addr = p = target_stkbas + target_stksiz;
> +
> + /* Save some space for ps_strings. */
> + p -= sizeof(struct target_ps_strings);
> +
> +#ifdef TARGET_SZSIGCODE
> + /* Add machine depedent sigcode. */
> + p -= TARGET_SZSIGCODE;
> + if (setup_sigtramp(p, (unsigned)offsetof(struct target_sigframe, sf_uc),
> + TARGET_FREEBSD_NR_sigreturn)) {
> + errno = EFAULT;
> + return -1;
> + }
> +#endif
Hmm. The x86 version you just added always returns -EOPNOTSUPP. Therefore I conclude
that TARGET_SZSIGCODE is unset.
Perhaps a better interface would be
p = setup_sigtramp(p, ...);
if (p == 0) {
// fail
}
then you don't need to expose TARGET_SZSIGCODE or have conditional compilation here.
Perhaps for the to-do list...
I'll add it to the todo...
> + /* Add canary for SSP. */
> + arc4random_buf(canary, sizeof(canary));
You should use qemu_guest_getrandom_nofail here.
OK.
> + /*
> + * Deviate from FreeBSD stack layout: force stack to new page here
> + * so that signal trampoline is not sharing the page with user stack
> + * frames. This is actively harmful in qemu as it marks pages with
> + * code it translated as read-only, which is somewhat problematic
> + * for user trying to use the stack as intended.
> + */
A decent short-term solution.
I'll draw your attention to my vdso patch set for linux-user:
https://patchew.org/QEMU/20210706234932.356913-1-richard.henderson@linaro.org/
I'll look at that as well...
Warner
Modulo randomness,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~