Re: [PATCH v7 30/41] x86/shstk: Handle thread shadow stack

[Edgecombe, Rick P]

On Thu, 2023-03-02 at 17:34 +0000, Szabolcs Nagy wrote:
> The 02/27/2023 14:29, Rick Edgecombe wrote:
> > For shadow stack enabled vfork(), the parent and child can share
> > the same
> > shadow stack, like they can share a normal stack. Since the parent
> > is
> > suspended until the child terminates, the child will not interfere
> > with
> > the parent while executing as long as it doesn't return from the
> > vfork()
> > and overwrite up the shadow stack. The child can safely overwrite
> > down
> > the shadow stack, as the parent can just overwrite this later. So
> > CET does
> > not add any additional limitations for vfork().
> > 
> > Userspace implementing posix vfork() can actually prevent the child
> > from
> > returning from the vfork() calling function, using CET. Glibc does
> > this
> > by adjusting the shadow stack pointer in the child, so that the
> > child
> > receives a #CP if it tries to return from vfork() calling function.
> 
> this commit message implies there is protection against
> the vfork child clobbering the parent's shadow stack,
> but actually the child can INCSSP (or longjmp) and then
> clobber it.

It's true the vfork child could use INCSSP and clobber to create
problems, so it is not a strong guarantee of shadow stack integrity.
But that's not claimed either. It does "prevent the child from
returning from the vfork() calling function" as much as shadow stack
protections apply, which I think would be reasonably understood. The
vfork child could also use wrss to write the return address to the
shadow stack and actually return, or disable shadow stack and return,
as other ways to create problems.

> 
> so the glibc code just tries to catch bugs and accidents
> not a strong security mechanism. i'd skip this paragraph.

Yep. I think it's very much a "nice to have" thing and not intended for
security. The paragraph is an aside anyway, because it is specifics
about another project. I don't have any objection to dropping it if the
opportunity comes up. IIRC it was added because someone thought vfork
couldn't work with shadow stack, so people might like to have the
details of how can be done.

I wouldn't even be too bothered if the discussed glibc behavior was
dropped either. vfork() can go wrong many ways regardless of shadow
stack. Is it worth the extra special behavior? Maybe just barely...