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.
