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

[Dave Hansen]

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?