Date: Mon, 12 Dec 2011 21:20:21 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: Joerg Sonnenberger <joerg@britannica.bec.de> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r228435 - in head/libexec/rtld-elf: . amd64 arm i386 ia64 mips powerpc powerpc64 sparc64 Message-ID: <20111212192021.GQ50300@deviant.kiev.zoral.com.ua> In-Reply-To: <20111212172132.GN50300@deviant.kiev.zoral.com.ua> References: <201112121103.pBCB3FuT097580@svn.freebsd.org> <20111212171709.GA22002@britannica.bec.de> <20111212172132.GN50300@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
--XZPmG3kZLpEx7yD+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Mon, Dec 12, 2011 at 07:21:32PM +0200, Kostik Belousov wrote:
> On Mon, Dec 12, 2011 at 06:17:09PM +0100, Joerg Sonnenberger wrote:
> > On Mon, Dec 12, 2011 at 11:03:15AM +0000, Konstantin Belousov wrote:
> > > To allow use of external references from the dispatch function, res=
olution
> > > of the R_MACHINE_IRESOLVE relocations in PLT is postponed until GOT=
entries
> > > for PLT are prepared, and normal resolution of the GOT entries is f=
inished.
> > > Similar to how it is done by GNU, IRELATIVE relocations are resolve=
d in
> > > advance, instead of normal lazy handling for PLT.
> >=20
> > Are you sure that you didn't introduce major locking issues with this?
> What do you mean, exactly ?
>=20
> The dispatcher function is called under the bind lock, yes.
To describe it in more details, _rtld_bind() read-locks the bind lock,
and possible plt resolution from the dispatcher would also acquire bind
lock in read mode, which is the supported operation. plt is explicitely
designed to allow safe multithreaded updates, so the shared lock do not
cause problems.
What does not work with rtld locks is read lock acquisition after the
write lock. Even more, it works for single-threaded locks, but fails
for thr_rtld.c. After thinking more about your question, I see how
this exact sequence can happen. If we dlopened the shared object that
contains IRELATIVE or jump slot which target is STT_GNU_IFUNC, then
possible recursive plt resolve from the dispatcher would cause it. Note
that I do not even consider implementing support for dl*(3) calls
from a dispatcher.
The solution is to postpone the resolution for irelative/ifunc right
before initializers are called, and drop bind lock around calls to
dispatcher.
The following patch is the implementation of the idea. It is somewhat
bigger then I hoped because I decided to use initlist to iterate over
the objects instead of the ->next, due to drop of the bind lock in
iteration.
diff --git a/libexec/rtld-elf/amd64/reloc.c b/libexec/rtld-elf/amd64/reloc.c
index 5ae8493..3b00987 100644
--- a/libexec/rtld-elf/amd64/reloc.c
+++ b/libexec/rtld-elf/amd64/reloc.c
@@ -413,6 +413,8 @@ reloc_iresolve(Obj_Entry *obj, RtldLockState *lockstate)
const Elf_Rela *relalim;
const Elf_Rela *rela;
=20
+ if (!obj->irelative)
+ return (0);
relalim =3D (const Elf_Rela *)((char *)obj->pltrela + obj->pltrelasize=
);
for (rela =3D obj->pltrela; rela < relalim; rela++) {
Elf_Addr *where, target, *ptr;
@@ -424,11 +426,14 @@ reloc_iresolve(Obj_Entry *obj, RtldLockState *locksta=
te)
case R_X86_64_IRELATIVE:
ptr =3D (Elf_Addr *)(obj->relocbase + rela->r_addend);
where =3D (Elf_Addr *)(obj->relocbase + rela->r_offset);
+ lock_release(rtld_bind_lock, lockstate);
target =3D ((Elf_Addr (*)(void))ptr)();
+ wlock_acquire(rtld_bind_lock, lockstate);
*where =3D target;
break;
}
}
+ obj->irelative =3D false;
return (0);
}
=20
@@ -455,13 +460,15 @@ reloc_gnu_ifunc(Obj_Entry *obj, RtldLockState *lockst=
ate)
return (-1);
if (ELF_ST_TYPE(def->st_info) !=3D STT_GNU_IFUNC)
continue;
+ lock_release(rtld_bind_lock, lockstate);
target =3D (Elf_Addr)rtld_resolve_ifunc(defobj, def);
+ wlock_acquire(rtld_bind_lock, lockstate);
reloc_jmpslot(where, target, defobj, obj, (const Elf_Rel *)rela);
break;
}
}
obj->gnu_ifunc =3D false;
- return 0;
+ return (0);
}
=20
void
diff --git a/libexec/rtld-elf/i386/reloc.c b/libexec/rtld-elf/i386/reloc.c
index 5f11106..30292ca 100644
--- a/libexec/rtld-elf/i386/reloc.c
+++ b/libexec/rtld-elf/i386/reloc.c
@@ -371,16 +371,21 @@ reloc_iresolve(Obj_Entry *obj, RtldLockState *locksta=
te)
const Elf_Rel *rel;
Elf_Addr *where, target;
=20
+ if (!obj->irelative)
+ return (0);
rellim =3D (const Elf_Rel *)((char *)obj->pltrel + obj->pltrelsize);
for (rel =3D obj->pltrel; rel < rellim; rel++) {
switch (ELF_R_TYPE(rel->r_info)) {
case R_386_IRELATIVE:
where =3D (Elf_Addr *)(obj->relocbase + rel->r_offset);
+ lock_release(rtld_bind_lock, lockstate);
target =3D ((Elf_Addr (*)(void))(*where))();
+ wlock_acquire(rtld_bind_lock, lockstate);
*where =3D target;
break;
}
}
+ obj->irelative =3D false;
return (0);
}
=20
@@ -407,7 +412,9 @@ reloc_gnu_ifunc(Obj_Entry *obj, RtldLockState *lockstat=
e)
return (-1);
if (ELF_ST_TYPE(def->st_info) !=3D STT_GNU_IFUNC)
continue;
+ lock_release(rtld_bind_lock, lockstate);
target =3D (Elf_Addr)rtld_resolve_ifunc(defobj, def);
+ wlock_acquire(rtld_bind_lock, lockstate);
reloc_jmpslot(where, target, defobj, obj, rel);
break;
}
diff --git a/libexec/rtld-elf/rtld.c b/libexec/rtld-elf/rtld.c
index 17cd67c..f91ebf7 100644
--- a/libexec/rtld-elf/rtld.c
+++ b/libexec/rtld-elf/rtld.c
@@ -116,6 +116,8 @@ static void objlist_push_tail(Objlist *, Obj_Entry *);
static void objlist_remove(Objlist *, Obj_Entry *);
static void *path_enumerate(const char *, path_enum_proc, void *);
static int relocate_objects(Obj_Entry *, bool, Obj_Entry *, RtldLockState =
*);
+static int resolve_objects_ifunc(Obj_Entry *first, bool bind_now,
+ RtldLockState *lockstate);
static int rtld_dirname(const char *, char *);
static int rtld_dirname_abs(const char *, char *);
static void rtld_exit(void);
@@ -513,6 +515,10 @@ _rtld(Elf_Addr *sp, func_ptr_type *exit_proc, Obj_Entr=
y **objp)
ld_bind_now !=3D NULL && *ld_bind_now !=3D '\0', &obj_rtld, NULL) =
=3D=3D -1)
die();
=20
+ if (resolve_objects_ifunc(obj_main,
+ ld_bind_now !=3D NULL && *ld_bind_now !=3D '\0', NULL) =3D=3D -1)
+ die();
+
dbg("doing copy relocations");
if (do_copy_relocations(obj_main) =3D=3D -1)
die();
@@ -1978,17 +1984,25 @@ relocate_objects(Obj_Entry *first, bool bind_now, O=
bj_Entry *rtldobj,
obj->version =3D RTLD_VERSION;
}
=20
- /*
- * The handling of R_MACHINE_IRELATIVE relocations and jumpslots
- * referencing STT_GNU_IFUNC symbols is postponed till the other
- * relocations are done. The indirect functions specified as
- * ifunc are allowed to call other symbols, so we need to have
- * objects relocated before asking for resolution from indirects.
- *
- * The R_MACHINE_IRELATIVE slots are resolved in greedy fashion,
- * instead of the usual lazy handling of PLT slots. It is
- * consistent with how GNU does it.
- */
+ return (0);
+}
+
+/*
+ * The handling of R_MACHINE_IRELATIVE relocations and jumpslots
+ * referencing STT_GNU_IFUNC symbols is postponed till the other
+ * relocations are done. The indirect functions specified as
+ * ifunc are allowed to call other symbols, so we need to have
+ * objects relocated before asking for resolution from indirects.
+ *
+ * The R_MACHINE_IRELATIVE slots are resolved in greedy fashion,
+ * instead of the usual lazy handling of PLT slots. It is
+ * consistent with how GNU does it.
+ */
+static int
+resolve_objects_ifunc(Obj_Entry *first, bool bind_now, RtldLockState *lock=
state)
+{
+ Obj_Entry *obj;
+
for (obj =3D first; obj !=3D NULL; obj =3D obj->next) {
if (obj->irelative && reloc_iresolve(obj, lockstate) =3D=3D -1)
return (-1);
@@ -1996,7 +2010,24 @@ relocate_objects(Obj_Entry *first, bool bind_now, Ob=
j_Entry *rtldobj,
reloc_gnu_ifunc(obj, lockstate) =3D=3D -1)
return (-1);
}
- return 0;
+ return (0);
+}
+
+static int
+initlist_objects_ifunc(Objlist *list, bool bind_now, RtldLockState *lockst=
ate)
+{
+ Objlist_Entry *elm;
+ Obj_Entry *obj;
+
+ STAILQ_FOREACH(elm, list, link) {
+ obj =3D elm->obj;
+ if (obj->irelative && reloc_iresolve(obj, lockstate) =3D=3D -1)
+ return (-1);
+ if ((obj->bind_now || bind_now) && obj->gnu_ifunc &&
+ reloc_gnu_ifunc(obj, lockstate) =3D=3D -1)
+ return (-1);
+ }
+ return (0);
}
=20
/*
@@ -2276,6 +2307,17 @@ dlopen_object(const char *name, Obj_Entry *refobj, i=
nt lo_flags, int mode)
=20
map_stacks_exec(&lockstate);
=20
+ if (initlist_objects_ifunc(&initlist, (mode & RTLD_MODEMASK) =3D=3D RT=
LD_NOW,
+ &lockstate) =3D=3D -1) {
+ objlist_clear(&initlist);
+ obj->dl_refcount--;
+ unref_dag(obj);
+ if (obj->refcount =3D=3D 0)
+ unload_object(obj);
+ lock_release(rtld_bind_lock, &lockstate);
+ return (NULL);
+ }
+
/* Call the init functions. */
objlist_call_init(&initlist, &lockstate);
objlist_clear(&initlist);
--XZPmG3kZLpEx7yD+
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (FreeBSD)
iEYEARECAAYFAk7mU/QACgkQC3+MBN1Mb4jnWQCaAuioLax20R5khgTmP++MHIxa
CxAAnj0V3IRqEqSVlaHGkQh0LyFzQScX
=ktbR
-----END PGP SIGNATURE-----
--XZPmG3kZLpEx7yD+--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111212192021.GQ50300>