Skip to content

Commit c6b53dc

Browse files
rpedgecohansendc
authored andcommitted
x86/shstk: Don't retry vm_munmap() on -EINTR
The existing comment around handling vm_munmap() failure when freeing a shadow stack is wrong. It asserts that vm_munmap() returns -EINTR when the mmap lock is only being held for a short time, and so the caller should retry. Based on this wrong understanding, unmap_shadow_stack() will loop retrying vm_munmap(). What -EINTR actually means in this case is that the process is going away (see ae79878), and the whole MM will be torn down soon. In order to facilitate this, the task should not linger in the kernel, but actually do the opposite. So don't loop in this scenario, just abandon the operation and let exit_mmap() clean it up. Also, update the comment to reflect the actual meaning of the error code. Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com> Link: https://lore.kernel.org/all/20230706233858.446232-1-rick.p.edgecombe%40intel.com
1 parent 54acee6 commit c6b53dc

File tree

1 file changed

+17
-21
lines changed

1 file changed

+17
-21
lines changed

arch/x86/kernel/shstk.c

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -134,28 +134,24 @@ static unsigned long adjust_shstk_size(unsigned long size)
134134

135135
static void unmap_shadow_stack(u64 base, u64 size)
136136
{
137-
while (1) {
138-
int r;
139-
140-
r = vm_munmap(base, size);
141-
142-
/*
143-
* vm_munmap() returns -EINTR when mmap_lock is held by
144-
* something else, and that lock should not be held for a
145-
* long time. Retry it for the case.
146-
*/
147-
if (r == -EINTR) {
148-
cond_resched();
149-
continue;
150-
}
137+
int r;
151138

152-
/*
153-
* For all other types of vm_munmap() failure, either the
154-
* system is out of memory or there is bug.
155-
*/
156-
WARN_ON_ONCE(r);
157-
break;
158-
}
139+
r = vm_munmap(base, size);
140+
141+
/*
142+
* mmap_write_lock_killable() failed with -EINTR. This means
143+
* the process is about to die and have it's MM cleaned up.
144+
* This task shouldn't ever make it back to userspace. In this
145+
* case it is ok to leak a shadow stack, so just exit out.
146+
*/
147+
if (r == -EINTR)
148+
return;
149+
150+
/*
151+
* For all other types of vm_munmap() failure, either the
152+
* system is out of memory or there is bug.
153+
*/
154+
WARN_ON_ONCE(r);
159155
}
160156

161157
static int shstk_setup(void)

0 commit comments

Comments
 (0)