Re: [PATCH v1 0/7] Remove in-tree usage of MAP_DENYWRITE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux