The 11/16/2023 00:52, Edgecombe, Rick P wrote:

On Wed, 2023-11-15 at 18:43 +0000, Mark Brown wrote:

quoted

quoted

otherwise 0 size would be fine: the child may not execute
a call instruction at all.

It seems like a special case. Where should the SSP be for the new
thread?

yes it is likely not an interesting case in practice so < 8
can be an error i think.

quoted

quoted

quoted

quoted

I think for CLONE_VM we should not require a non-zero size.
Speaking of
CLONE_VM we should probably be clear on what the expected
behavior is
for situations when a new shadow stack is not usually
allocated.
!CLONE_VM || CLONE_VFORK will use the existing shadow stack.
Should we
require shadow_stack_size be zero in this case, or just ignore
it? I'd
lean towards requiring it to be zero so userspace doesn't pass
garbage
in that we have to accommodate later. What we could possibly
need to do
around that though, I'm not sure. What do you think?

quoted

quoted

Yes, requiring it to be zero in that case makes sense I think.

quoted

i think the condition is "no specified separate stack for
the child (stack==0 || stack==sp)".

quoted

CLONE_VFORK does not imply that the existing stack will be
used (a stack for the child can be specified, i think both
glibc and musl do this in posix_spawn).

That also works as a check I think, though it requires the arch to
check
for the stack==sp case - I hadn't been aware of the posix_spawn()
usage,
the above checks Rick suggested just follow the handling for implicit
allocation we have currently.

I didn't realize it was passing its own stack either. I guess the
reason is to avoid stack overflows. But none of the specific reasons
listed in the comments seem to applicable to shadow stacks.

while CLONE_VFORK allows the child to use the parent shadow
stack (parent and child cannot execute at the same time and
the child wont return to frames created by the parent), we
want to enable precise size accounting of the shadow stack
so requesting a new shadow stack should work if new stack
is specified.

but stack==0 can force shadow_stack_size==0.

i guess the tricky case is stack!=0 && shadow_stack_size==0:
the user may want a new shadow stack with default size logic,
or (with !CLONE_VM || CLONE_VFORK) wants to use the existing
shadow stack from the parent.

What is the case for stack=sp bit of the logic?

iirc it is not documented in the clone man page what stack=0
means and of course you don't want sp==0 in the vfork child
so some targets sets stack to sp in vfork, others set it 0
and expect the kernel to do the right thing.

this likely does not apply to clone3 where the size has to be
specified so maybe stack==sp does not need special treatment.

I need to look into this more. My first thought is, passing in a stack
doesn't absolutely mean they want a new shadow stack allocated either.
We are changing one heuristic, for another.

yes.

The other thought is, the new behavior in the !CLONE_VM case doesn't
seem optimal. fork has ->stack==0. Then we would be allocating a stack
in only the child's MM and changing the SSP there, and for what reason?
So I don't think we should fully move away from taking hints from the
CLONE flags.

only stack!=0 case is tricky. stack==0 means existing shadow stack.

Maybe an alternate could just be to lose the CLONE_VFORK specific stack
sharing logic. CLONE_VM always gets a new shadow stack. I don't think
it would disturb userspace functionally, but just involves more
mapping/unmapping.

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.