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

On Mon, Sep 03, 2012 at 12:35:08PM +0200, Svatopluk Kraus wrote:
> Hi,
> 
>   I found out that while the running excecutables and a dynamic linker
> are protected against writing (ETXTBSY), the loaded shared libraries
> are not protected. The libraries are mapped by mmap() in dynamic
> linker (rtld) and there is no way how to set VV_TEXT flag on the
> libraries vnodes in mmap() code.
> 
>   In linux compability code \compat\linux\linux_misc.c, linux_uselib()
> sets VV_TEXT flags on a library vnode. In Solaris, MAP_TEXT flag
> exists which informs mmap() that the mapped region will be used
> primarily for executing instructions (for better MMU utilization).
> With these on mind, I propose to implement MAP_TEXT option in mmap()
> and in case that underlying object is a vnode, set VV_TEXT flag on it.
> 
>   I already have implemented it and with rtld map_object() patch it
> works fine for me (of course). The rtld patch looks easy, however I'm
> not sure about mmap patch.
> 
>   After some investigation, it looks that VV_TEXT once set on a vnode
> remains set until last reference on the vnode is left. So, I don't
> bother with VV_TEXT unset in munmap() to be consistent. The
> executables and dynamic linker are activated in kernel, so VV_TEXT is
> set before activation and cleared if something failed. Shared library
> activation is done in dynamic linker (i.e., in userland). It's done in
> steps and mmaping the library is one from them. So, I think that
> VV_TEXT can be set in mmap() just after everything is finished
> successfully.
This is right, the object reference counter is also used as
VV_TEXT counter. It is somewhat unaccurate, but in practice does
not cause issues.

> 
>   The patch itself is implemented in vm_mmap_vnode(). If I want to set
> VV_TEXT flag on a vnode, I need an exclusive lock. In current code,
> the exclusive lock flag is (mis)used as a flag for
> vnode_pager_update_writecount() call. (I hope that I didn't miss
> something.) So, the patch is bigger slightly.
> 
>   I defined the MAP_TEXT flag in extented flags sections. However, I'm
> feeling the relation to MAP_STACK flag, but not sure if and when
> reserved flags (in other flags section) can be re-used.
> 
>        Svata
> 
> 
>   Index: libexec/rtld-elf/map_object.c
> ===================================================================
> --- libexec/rtld-elf/map_object.c	(revision 239770)
> +++ libexec/rtld-elf/map_object.c	(working copy)
> _at__at_ -199,7 +199,8 _at__at_
>  	data_prot = convert_prot(segs[i]->p_flags);
>  	data_flags = convert_flags(segs[i]->p_flags) | MAP_FIXED;
>  	if (mmap(data_addr, data_vlimit - data_vaddr, data_prot,
> -	  data_flags | MAP_PREFAULT_READ, fd, data_offset) == (caddr_t) -1) {
> +	  data_flags | MAP_PREFAULT_READ | MAP_TEXT, fd, data_offset) ==
> +	    (caddr_t) -1) {
I am not sure that we shall mark all segments mappings with MAP_TEXT.
I understand the logic of the change, since we do not want data segment
to be changed under us. Still, having MAP_TEXT for non-text segments looks
strange.

>  	    _rtld_error("%s: mmap of data failed: %s", path,
>  		rtld_strerror(errno));
>  	    goto error1;
> Index: sys/vm/vm_mmap.c
> ===================================================================
> --- sys/vm/vm_mmap.c	(revision 239770)
> +++ sys/vm/vm_mmap.c	(working copy)
> _at__at_ -1258,10 +1258,13 _at__at_
>  	struct mount *mp;
>  	struct ucred *cred;
>  	int error, flags, locktype, vfslocked;
> +	int writeable_shared;
> 
>  	mp = vp->v_mount;
>  	cred = td->td_ucred;
> -	if ((*maxprotp & VM_PROT_WRITE) && (*flagsp & MAP_SHARED))
> +	flags = *flagsp;
> +	writeable_shared = ((*maxprotp & VM_PROT_WRITE) && (flags & MAP_SHARED));
> +	if (writeable_shared || ((flags & MAP_TEXT) != 0))
>  		locktype = LK_EXCLUSIVE;
>  	else
>  		locktype = LK_SHARED;
> _at__at_ -1271,7 +1274,6 _at__at_
>  		return (error);
>  	}
>  	foff = *foffp;
> -	flags = *flagsp;
>  	obj = vp->v_object;
>  	if (vp->v_type == VREG) {
>  		/*
> _at__at_ -1294,7 +1296,7 _at__at_
>  				return (error);
>  			}
>  		}
> -		if (locktype == LK_EXCLUSIVE) {
> +		if (writeable_shared) {
>  			*writecounted = TRUE;
>  			vnode_pager_update_writecount(obj, 0, objsize);
>  		}
> _at__at_ -1337,6 +1339,14 _at__at_
>  		error = ENOMEM;
>  		goto done;
>  	}
> +	/*
> +	 * If MAP_TEXT is announced, set VV_TEXT so no one can write
> +	 * to the executable.
> +	 */
> +	if ((flags & MAP_TEXT) != 0) {
> +		ASSERT_VOP_ELOCKED(vp, "vv_text");
> +		vp->v_vflag |= VV_TEXT;
> +	}
I do not think we want to set VV_TEXT for device vnodes.

>  	*objp = obj;
>  	*flagsp = flags;
> 
> Index: sys/sys/mman.h
> ===================================================================
> --- sys/sys/mman.h	(revision 239770)
> +++ sys/sys/mman.h	(working copy)
> _at__at_ -91,6 +91,7 _at__at_
>   */
>  #define	MAP_NOCORE	 0x00020000 /* dont include these pages in a coredump */
>  #define	MAP_PREFAULT_READ 0x00040000 /* prefault mapping for reading */
> +#define	MAP_TEXT	 0x00080000 /* map code segment */
>  #endif /* __BSD_VISIBLE */
> 
>  #if __POSIX_VISIBLE >= 199309
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org"