On Fri, Sep 07, 2012 at 02:05:28PM -0400, John Baldwin wrote: > On Friday, September 07, 2012 12:42:18 pm Konstantin Belousov wrote: > > On Fri, Sep 07, 2012 at 12:21:37PM -0400, John Baldwin wrote: > > > On Friday, September 07, 2012 12:02:08 pm Konstantin Belousov wrote: > > > > On Fri, Sep 07, 2012 at 05:12:37PM +0200, Svatopluk Kraus wrote: > > > > > On Tue, Sep 4, 2012 at 6:00 PM, John Baldwin <j...@freebsd.org> wrote: > > > > > > On Tuesday, September 04, 2012 9:00:39 am Konstantin Belousov wrote: > > > > > >> 2. I do not see what would prevent malicious local user from > > > > > >> mmaping > > > > > >> arbitrary file readonly with MAP_TEXT, thus blocking any > > > > > >> modifications > > > > > >> to the file. Note that this is not a problem for executables, > > > > > >> because > > > > > >> kernel only sets VV_TEXT on executables if +x permission is set > > > > > >> and > > > > > >> file is valid binary which kernel is able to execute. > > > > > >> > > > > > >> E.g. you might block log writes with VV_TEXT, or other user > > > > > >> editing > > > > > >> session or whatever, having just read access to corresponding > > > > > >> files. > > > > > >> > > > > > >> Am I wrong ? > > > > > > > > > > > > Hmm, I do think 2) is a bit of a show-stopper. I do wonder why one > > > > > > needs > > > > > > MAP_TEXT at all or if you could key this off of mmap() with > > > > > > PROT_EXEC? > > > > > > Do we require +x permissions for PROT_EXEC? No, it seems we only > > > > > > require > > > > > > a file opened with FREAD. Hmm, perhaps rtld could open a separate > > > > > > fd for > > > > > > PROT_EXEC mappings that used O_EXEC and mmap()'ing an O_EXEC fd > > > > > > could enable > > > > > > VV_TEXT? That would require a file to have +x permisson for an > > > > > > mmap() to > > > > > > enable VV_TEXT. It would also make MAP_TEXT unneeded. > > > > > > > > > > It sounds good for me. I will try to patch it this way. However, do > > > > > you think that will be acceptable to set +x permission to shared > > > > > libraries in general? > > > > > > Shared libraries have +x by default already. You could make rtld fall > > > back > > > to using the O_RDONLY fd if it can't open an O_EXEC fd in which case you > > > don't > > > get the VV_TEXT protection, but rtld would be backwards compatible. > > Where is it ? On fresh stable/9 machine, I have in /lib: > > -r--r--r-- 1 root wheel 7216 Dec 3 2011 libipx.so.5 > > -r--r--r-- 1 root wheel 19800 Jun 28 18:33 libjail.so.1 > > -r--r--r-- 1 root wheel 13752 Jun 28 18:33 libkiconv.so.4 > > ... > > Hmm, I was looking in /usr/local/lib. It seems at least some ports do > install libraries with +x: > > -rwxr-xr-x 1 root wheel 210494 Apr 8 2011 > /usr/local/lib/libxml++-2.6.so.2 > -rwxr-xr-x 1 root wheel 1515416 Apr 8 2011 /usr/local/lib/libxml2.so.5 > -rwxr-xr-x 1 root wheel 44389 Apr 8 2011 > /usr/local/lib/libxmlparse.so.1 > -rwxr-xr-x 1 root wheel 100672 Apr 8 2011 /usr/local/lib/libxmltok.so.1 > -rwxr-xr-x 1 root wheel 266775 Apr 8 2011 /usr/local/lib/libxslt.so.2 > -rw-r--r-- 1 root wheel 764602 Apr 8 2011 > /usr/local/lib/libxvidcore.so.4 > -rwxr-xr-x 1 root wheel 53379 Apr 9 2011 /usr/local/lib/libzip.so.1 > > > > > Setting +x on shared libraries can be done. But setting VV_TEXT for > > > > such mappings is definitely non-standard behaviour, that could cause > > > > locking surprises for unaware system administrator. The issuw would be > > > > very hard to diagnose. > > > > > > I'm not sure it would be that obscure. It's the same as getting an error > > > that > > > a binary is busy. In that case you would resort to 'ps' to find the > > > offending > > > process. For this case (a shared library being busy), you could look at > > > the > > > output of 'procstat -af' to find which processes have executable mappings > > > of > > > the library. > > > > I much more worry about rtld reporting 'shared library is busy'. It is fine > > to not be able to write to mapped library. > > > > Rtld errors are usually quite worrying for users, and they just do not > > understand them. > > I think these would be rare? There's no good reason for anything to write to > a shared library that I can think of. install(1) does an atomic rename to > swap > in the new libraries already.
After a second thought, I do not like your proposal as well. +x is set for shebang scripts, and allowing PROT_EXEC to set VV_TEXT for them means that such scripts are subject for write denial.
pgphor0hI2qC3.pgp
Description: PGP signature
