Page MenuHomeFreeBSD

Fix MAP_WIREFUTURE leakage
ClosedPublic

Authored by alc on Jun 30 2017, 3:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 20 2024, 12:36 PM
Unknown Object (File)
Dec 20 2023, 6:37 AM
Unknown Object (File)
Nov 14 2023, 1:23 AM
Unknown Object (File)
Nov 10 2023, 10:28 AM
Unknown Object (File)
Nov 8 2023, 10:03 AM
Unknown Object (File)
Nov 6 2023, 7:04 PM
Unknown Object (File)
Nov 4 2023, 10:26 PM
Unknown Object (File)
Oct 24 2023, 2:36 AM
Subscribers
None

Details

Summary

While reviewing the new stack management code, I observed that the MAP_WIREFUTURE flag is not cleared by exec_new_vmspace() when it recycles the current vmspace. Consequently, an mlockall(MCL_FUTURE) could still be in effect on the process after an {f,}execve(2). This violates the specification for mlockall(2).

Also, while I'm here, it's pointless for vm_map_stack() to check the MEMLOCK limit. It will never be asked to wire the stack. Moreover, it doesn't even implement wiring of the stack.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Jun 30 2017, 3:38 AM

With this change, vm_map_stack() becomes a check for vmemlim and call to vm_map_stack_locked(). The function has only one caller in execve(). Would it make sense to remove vm_map_stack() and call vm_map_fixed() from execve ?

In D11421#236260, @kib wrote:

With this change, vm_map_stack() becomes a check for vmemlim and call to vm_map_stack_locked(). The function has only one caller in execve(). Would it make sense to remove vm_map_stack() and call vm_map_fixed() from execve ?

I think that the answer depends on how we deal with another edge-case bug. Observe that exec_new_vmspace() unconditionally initializes the vmspace's vm_ssize based on sgrowsiz. However, this may not be the actual, current stack size as implemented by vm_map_stack_locked(). One solution to this problem would be to return the initial stack size through an "out" parameter. None of the current callers of vm_map_fixed() really need such a parameter.

This revision was automatically updated to reflect the committed changes.
In D11421#236321, @alc wrote:
In D11421#236260, @kib wrote:

With this change, vm_map_stack() becomes a check for vmemlim and call to vm_map_stack_locked(). The function has only one caller in execve(). Would it make sense to remove vm_map_stack() and call vm_map_fixed() from execve ?

I think that the answer depends on how we deal with another edge-case bug. Observe that exec_new_vmspace() unconditionally initializes the vmspace's vm_ssize based on sgrowsiz. However, this may not be the actual, current stack size as implemented by vm_map_stack_locked(). One solution to this problem would be to return the initial stack size through an "out" parameter. None of the current callers of vm_map_fixed() really need such a parameter.

I think I will leave it alone for now. In fact, it is probably more reasonable to handle it somewhat later after we rework the RLIM_STACK. If we start tracking the main thread stack mapping and relating gap(s), then vm_ssize could be naturally calculated as the size of the map entry.