Date: Mon, 3 Sep 2012 15:46:42 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Svatopluk Kraus <onwahe@gmail.com> Cc: freebsd-current@freebsd.org Subject: Re: [patch] mmap() MAP_TEXT implementation (to use for shared libraries) Message-ID: <20120903124642.GI33100@deviant.kiev.zoral.com.ua> In-Reply-To: <CAFHCsPX6HrCXHA%2BS31Dz9QP8eiwbo21Vzju4K4paohciu2vPTw@mail.gmail.com> References: <CAFHCsPX6HrCXHA%2BS31Dz9QP8eiwbo21Vzju4K4paohciu2vPTw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--q+qpHfi2NROeSQHU
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Mon, Sep 03, 2012 at 12:35:08PM +0200, Svatopluk Kraus wrote:
> Hi,
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> Svata
>=20
>=20
> Index: libexec/rtld-elf/map_object.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- libexec/rtld-elf/map_object.c (revision 239770)
> +++ libexec/rtld-elf/map_object.c (working copy)
> @@ -199,7 +199,8 @@
> data_prot =3D convert_prot(segs[i]->p_flags);
> data_flags =3D 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) =3D=3D (caddr_t) -1)=
{
> + data_flags | MAP_PREFAULT_READ | MAP_TEXT, fd, data_offset) =3D=3D
> + (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
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- sys/vm/vm_mmap.c (revision 239770)
> +++ sys/vm/vm_mmap.c (working copy)
> @@ -1258,10 +1258,13 @@
> struct mount *mp;
> struct ucred *cred;
> int error, flags, locktype, vfslocked;
> + int writeable_shared;
>=20
> mp =3D vp->v_mount;
> cred =3D td->td_ucred;
> - if ((*maxprotp & VM_PROT_WRITE) && (*flagsp & MAP_SHARED))
> + flags =3D *flagsp;
> + writeable_shared =3D ((*maxprotp & VM_PROT_WRITE) && (flags & MAP_SHARE=
D));
> + if (writeable_shared || ((flags & MAP_TEXT) !=3D 0))
> locktype =3D LK_EXCLUSIVE;
> else
> locktype =3D LK_SHARED;
> @@ -1271,7 +1274,6 @@
> return (error);
> }
> foff =3D *foffp;
> - flags =3D *flagsp;
> obj =3D vp->v_object;
> if (vp->v_type =3D=3D VREG) {
> /*
> @@ -1294,7 +1296,7 @@
> return (error);
> }
> }
> - if (locktype =3D=3D LK_EXCLUSIVE) {
> + if (writeable_shared) {
> *writecounted =3D TRUE;
> vnode_pager_update_writecount(obj, 0, objsize);
> }
> @@ -1337,6 +1339,14 @@
> error =3D ENOMEM;
> goto done;
> }
> + /*
> + * If MAP_TEXT is announced, set VV_TEXT so no one can write
> + * to the executable.
> + */
> + if ((flags & MAP_TEXT) !=3D 0) {
> + ASSERT_VOP_ELOCKED(vp, "vv_text");
> + vp->v_vflag |=3D VV_TEXT;
> + }
I do not think we want to set VV_TEXT for device vnodes.
> *objp =3D obj;
> *flagsp =3D flags;
>=20
> Index: sys/sys/mman.h
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- sys/sys/mman.h (revision 239770)
> +++ sys/sys/mman.h (working copy)
> @@ -91,6 +91,7 @@
> */
> #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 */
>=20
> #if __POSIX_VISIBLE >=3D 199309
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
--q+qpHfi2NROeSQHU
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (FreeBSD)
iEYEARECAAYFAlBEprIACgkQC3+MBN1Mb4h5yACggbaWE+xAW3fsF8IXMohsHxtW
pZwAmwb4i4Mgt/RHBLdM56PNcE3Qsxdk
=a2Zn
-----END PGP SIGNATURE-----
--q+qpHfi2NROeSQHU--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120903124642.GI33100>