On 14.08.21 04:02, Al Viro wrote:
On Sat, Aug 14, 2021 at 01:57:31AM +0000, Al Viro wrote:
On Fri, Aug 13, 2021 at 02:58:57PM -1000, Linus Torvalds wrote:
On Fri, Aug 13, 2021 at 2:54 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
And nobody really complained when we weakened it, so maybe removing it
entirely might be acceptable.
I guess we could just try it and see... Worst comes to worst, we'll
have to put it back, but at least we'd know what crazy thing still
wants it..
Umm... I'll need to go back and look through the thread, but I'm
fairly sure that there used to be suckers that did replacement of
binary that way (try to write, count on exclusion with execve while
it's being written to) instead of using rename. Install scripts
of weird crap and stuff like that...
... and before anyone goes off - I certainly agree that using that
behaviour is not a good idea and had never been one. All I'm saying
is that there at least used to be very random (and rarely exercised)
bits of userland relying upon that behaviour.
Removing it completely is certainly more controversial than limiting it
to the main executable. I'm mostly happy as long as we get rid of that
nasty per-VMA handling, because that adds real complexity at places that
are complicated enough.
Having the remaining deny_write_access()/allow_write_access() at sane
places now (loading a new binary, exchanging exe_file) looks certainly
much cleaner and I still consider it a valuable, simple sanity feature
to have around. I don't think there is any sane use case for modifying
the main executable, and it seems to be very easy to catch.
For example, besides users that rely on this behavior, in my thinking
(see the cover letter), especially having a binary not getting changed
while we're loading it sounds like a very good idea (not saying we would
expose a way to exploit the kernel if we would allow for modifications
while in the elf parser, but also not saying we wouldn't because I
didn't check if there would be a way; at least we already allow it in
the legacy library loader before mapping the segments with
MAP_DENYWRITE). And if we decide to keep the behavior while loading the
executable, keeping it while exe_file is set isn't much added
code/complexity IMHO.
Long story short, I'd vote for keeping it in, and if we decide to rip it
out completely, do it a a separate, more careful step.
--
Thanks,
David / dhildenb
