On 6/27/23 10:20, Mark Brown wrote:
> On Mon, Jun 12, 2023 at 05:10:54PM -0700, Rick Edgecombe wrote:
>
>> +static void unmap_shadow_stack(u64 base, u64 size)
>> +{
>> + while (1) {
>> + int r;
>> +
>> + r = vm_munmap(base, size);
>> +
>> + /*
>> + * vm_munmap() returns -EINTR when mmap_lock is held by
>> + * something else, and that lock should not be held for a
>> + * long time. Retry it for the case.
>> + */
>> + if (r == -EINTR) {
>> + cond_resched();
>> + continue;
>> + }
> 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?
