Re: [patch] mmap() MAP_TEXT implementation (to use for shared libraries)

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 <jhb_at_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.

> 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.

-- 
John Baldwin