On Mon, Aug 9, 2021 at 3:00 PM Richard Henderson <
[email protected]> wrote:
> 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/[email protected]/
>
I'll look at that as well...
Warner
> Modulo randomness,
> Reviewed-by: Richard Henderson <[email protected]>
>
>
> r~
>
