Re: [PATCH v2] kernel/fork: stop playing lockless games for exe_file replacement

From: David Hildenbrand
Date: Tue Aug 15 2023 - 03:31:03 EST


On 14.08.23 19:21, Mateusz Guzik wrote:
xchg originated in 6e399cd144d8 ("prctl: avoid using mmap_sem for
exe_file serialization"). While the commit message does not explain
*why* the change, I found the original submission [1] which ultimately
claims it cleans things up by removing dependency of exe_file on the
semaphore.

However, fe69d560b5bd ("kernel/fork: always deny write access to current
MM exe_file") added a semaphore up/down cycle to synchronize the state
of exe_file against fork, defeating the point of the original change.

This is on top of semaphore trips already present both in the replacing
function and prctl (the only consumer).

Normally replacing exe_file does not happen for busy processes, thus
write-locking is not an impediment to performance in the intended use
case. If someone keeps invoking the routine for a busy processes they
are trying to play dirty and that's another reason to avoid any
trickery.

As such I think the atomic here only adds complexity for no benefit.

Just write-lock around the replacement.

I also note that replacement races against the mapping check loop as
nothing synchronizes actual assignment with with said checks but I am
not addressing it in this patch. (Is the loop of any use to begin with?)

V2:
- fix up comments
- tweak commit message

Link: https://lore.kernel.org/linux-mm/1424979417.10344.14.camel@xxxxxxxxxxxx/ [1]
Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx>
---


Acked-by: David Hildenbrand <david@xxxxxxxxxx>

--
Cheers,

David / dhildenb