Re: [PATCH v9 28/42] x86/shstk: Add user-mode shadow stack support

From: Edgecombe, Rick P
Date: Tue Jun 27 2023 - 20:37:52 EST


On Tue, 2023-06-27 at 16:46 -0700, Dave Hansen wrote:
> > This looks generic, not even shadow stack specific - was there any
> > discussion of making it a vm_munmap_retry() (that's not a great
> > name...)
> > or similar?  I didn't see any in old versions of the thread but I
> > might've missed something.
>
> Yeah, that looks odd.  Also odd is that none of the other users of
> vm_munmap() bother to check the return value (except for one
> passthrough
> in the nommu code).
>
> I don't think the EINTR happens during contention, though.  It's really
> there to be able break out in the face of SIGKILL.  I think that's why
> nobody handles it: the task is dying anyway and nobody cares.
>
> Rick, was this hunk here for a specific reason or were you just trying
> to be diligent in handling errors?

I'm not aware of any specific required cases. I think it is just pure
dilligence, originating from this comment:
https://lore.kernel.org/lkml/9847845a-749d-47a3-2a1d-bcc7c35f1bdd@xxxxxxxxx/#t

I didn't see a need to remove it. 

The SIGKILL certaintly sounds like something more true than the
comment, but I can't do much of dive until next week. I would think we
need to handle EINTR differently to not WARN at least.

Yea, some of it does seem like the kind of thing that could live
outside of x86 shstk.c. But I'm not sure about the WARN part. That
should probably live in the caller. I guess someday we might even find
some shadow stack specific logic that could be cross-arch.