freebsd stable - Sep 2018 - Constraints in libmap(32).conf do not work as expected, possible bug in rtld-elf

On Tue, Sep 25, 2018 at 12:03:32AM +0200, Andreas Longwitz
wrote:
> > 
> > Can you try this instead ?
> > 
> Yes I did on a server running FreeBSD 12.0-CURRENT (GENERIC) #0 r337452
> and - after a trivial adaptation of your patch - on FreeBSD 10.4-STABLE
> #0 r337823 and everything works correct.
> 
> My simple libmap32.conf now is:
> 
> ## php52
> [/usr/local/php52/]
> /usr/local/lib          /usr/local/lib32
> /usr/local/lib/mysql    /usr/local/lib32/mysql
> 
> [libc-client4.so.9]
> libssl.so.8             libssl.so.6
> libcrypto.so.8          libcrypto.so.6
> 
> My test command "/usr/local/php52/bin/php -i" loads also all the
shared
> objects in /usr/local/php52/lib/php/20060613: gettext.so iconv.so
> imap.so mbstring.so mcrypt.so mysql.so pcre.so session.so xml.so.
> Further ldd gives correct output for every mentioned file.
Thanks.
> 
> I like to mention one thing concerning the source libmap.c. With the
> patch (yours or mine) and the libmap32.conf given above I see the
> following lmp_list when lm_fini() is called:
> 
>     lm_fini("1, $DEFAULT$" lml-Adresse 0x2826c208)
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib/mysql,
t=/usr/local/lib32/mysql")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=libcrypto.so.8, t=libcrypto.so.6")
>         lm_fini("f=libssl.so.8, t=libssl.so.6")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>      lm_fini("1, libc-client4.so.9" lml-Adresse 0x2826c168)
>         lm_fini("f=libcrypto.so.8, t=libcrypto.so.6")
>         lm_fini("f=libssl.so.8, t=libssl.so.6")
>      lm_fini("2, /usr/local/php52/" lml-Adresse 0x2826c068)
>         lm_fini("f=/usr/local/lib/mysql,
t=/usr/local/lib32/mysql")
>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
> 
> So for $DEFAULTS we have a lot of identical entries. This comes from the
> TAILQ_INSERT_HEAD statement in lm_add(). I am not sure if this can be
> accepted or a check to avoid double entries in the list is better.
Yes, this is mostly cosmetics.  It is not clear is it better to avoid
duplicates and pay the cost at insertion, or leave them and pay at the
list traversal.  I think there is slight preference to avoid dups, but
this should be not measureable.

There is a second caller of lm_add(), but there the dup should be user-caused.
> 
> One annotation to the script /etc/rc.d/ldconfig: I had expected that
> this script during boot creates clean files ld-elf(32).so.hints in
> /var/run. For 64 bit this is true, but for 32 bit not because ldconfig
> with flag -32 also has flag -m. Is this intended behaviour ?
This seems to be from the beginning when ldconfig_local32 was
introduced in r154114.

Combined patch below.

diff --git a/libexec/rtld-elf/libmap.c b/libexec/rtld-elf/libmap.c
index 592b7664eea..33c824a65af 100644
--- a/libexec/rtld-elf/libmap.c
+++ b/libexec/rtld-elf/libmap.c
@@ -353,6 +353,7 @@ lm_add(const char *p, const char *f, const char *t)
 {
 	struct lm_list *lml;
 	struct lm *lm;
+	const char *t1;
 
 	if (p == NULL)
 		p = "$DEFAULT$";
@@ -362,11 +363,14 @@ lm_add(const char *p, const char *f, const char *t)
 	if ((lml = lmp_find(p)) == NULL)
 		lml = lmp_init(xstrdup(p));
 
-	lm = xmalloc(sizeof(struct lm));
-	lm->f = xstrdup(f);
-	lm->t = xstrdup(t);
-	TAILQ_INSERT_HEAD(lml, lm, lm_link);
-	lm_count++;
+	t1 = lml_find(lml, f);
+	if (t1 == NULL || strcmp(t1, t) != 0) {
+		lm = xmalloc(sizeof(struct lm));
+		lm->f = xstrdup(f);
+		lm->t = xstrdup(t);
+		TAILQ_INSERT_HEAD(lml, lm, lm_link);
+		lm_count++;
+	}
 }
 
 char *
diff --git a/libexec/rtld-elf/rtld.c b/libexec/rtld-elf/rtld.c
index dfd0388478f..83d5e28e287 100644
--- a/libexec/rtld-elf/rtld.c
+++ b/libexec/rtld-elf/rtld.c
@@ -125,7 +125,7 @@ static void objlist_remove(Objlist *, Obj_Entry *);
 static int open_binary_fd(const char *argv0, bool search_in_path);
 static int parse_args(char* argv[], int argc, bool *use_pathp, int *fdp);
 static int parse_integer(const char *);
-static void *path_enumerate(const char *, path_enum_proc, void *);
+static void *path_enumerate(const char *, path_enum_proc, const char *, void
*);
 static void print_usage(const char *argv0);
 static void release_object(Obj_Entry *);
 static int relocate_object_dag(Obj_Entry *root, bool bind_now,
@@ -140,7 +140,8 @@ static int rtld_dirname(const char *, char *);
 static int rtld_dirname_abs(const char *, char *);
 static void *rtld_dlopen(const char *name, int fd, int mode);
 static void rtld_exit(void);
-static char *search_library_path(const char *, const char *, int *);
+static char *search_library_path(const char *, const char *, const char *,
+    int *);
 static char *search_library_pathfds(const char *, const char *, int *);
 static const void **get_program_var_addr(const char *, RtldLockState *);
 static void set_program_var(const char *, const void *);
@@ -1576,8 +1577,7 @@ gnu_hash(const char *s)
 static char *
 find_library(const char *xname, const Obj_Entry *refobj, int *fdp)
 {
-	char *pathname;
-	char *name;
+	char *name, *pathname, *refobj_path;
 	bool nodeflib, objgiven;
 
 	objgiven = refobj != NULL;
@@ -1597,6 +1597,7 @@ find_library(const char *xname, const Obj_Entry *refobj,
int *fdp)
 	}
 
 	dbg(" Searching for \"%s\"", name);
+	refobj_path = objgiven ? refobj->path : NULL;
 
 	/*
 	 * If refobj->rpath != NULL, then refobj->runpath is NULL.  Fall
@@ -1605,52 +1606,61 @@ find_library(const char *xname, const Obj_Entry *refobj,
int *fdp)
 	 * nodeflib.
 	 */
 	if (objgiven && refobj->rpath != NULL &&
ld_library_path_rpath) {
-		pathname = search_library_path(name, ld_library_path, fdp);
+		pathname = search_library_path(name, ld_library_path,
+		    refobj_path, fdp);
 		if (pathname != NULL)
 			return (pathname);
 		if (refobj != NULL) {
-			pathname = search_library_path(name, refobj->rpath, fdp);
+			pathname = search_library_path(name, refobj->rpath,
+			    refobj_path, fdp);
 			if (pathname != NULL)
 				return (pathname);
 		}
 		pathname = search_library_pathfds(name, ld_library_dirs, fdp);
 		if (pathname != NULL)
 			return (pathname);
-		pathname = search_library_path(name, gethints(false), fdp);
+		pathname = search_library_path(name, gethints(false),
+		    refobj_path, fdp);
 		if (pathname != NULL)
 			return (pathname);
-		pathname = search_library_path(name, ld_standard_library_path, fdp);
+		pathname = search_library_path(name, ld_standard_library_path,
+		    refobj_path, fdp);
 		if (pathname != NULL)
 			return (pathname);
 	} else {
 		nodeflib = objgiven ? refobj->z_nodeflib : false;
 		if (objgiven) {
-			pathname = search_library_path(name, refobj->rpath, fdp);
+			pathname = search_library_path(name, refobj->rpath,
+			    refobj->path, fdp);
 			if (pathname != NULL)
 				return (pathname);
 		}
 		if (objgiven && refobj->runpath == NULL && refobj !=
obj_main) {
-			pathname = search_library_path(name, obj_main->rpath, fdp);
+			pathname = search_library_path(name, obj_main->rpath,
+			    refobj_path, fdp);
 			if (pathname != NULL)
 				return (pathname);
 		}
-		pathname = search_library_path(name, ld_library_path, fdp);
+		pathname = search_library_path(name, ld_library_path,
+		    refobj_path, fdp);
 		if (pathname != NULL)
 			return (pathname);
 		if (objgiven) {
-			pathname = search_library_path(name, refobj->runpath, fdp);
+			pathname = search_library_path(name, refobj->runpath,
+			    refobj_path, fdp);
 			if (pathname != NULL)
 				return (pathname);
 		}
 		pathname = search_library_pathfds(name, ld_library_dirs, fdp);
 		if (pathname != NULL)
 			return (pathname);
-		pathname = search_library_path(name, gethints(nodeflib), fdp);
+		pathname = search_library_path(name, gethints(nodeflib),
+		    refobj_path, fdp);
 		if (pathname != NULL)
 			return (pathname);
 		if (objgiven && !nodeflib) {
 			pathname = search_library_path(name,
-			    ld_standard_library_path, fdp);
+			    ld_standard_library_path, refobj_path, fdp);
 			if (pathname != NULL)
 				return (pathname);
 		}
@@ -1845,8 +1855,9 @@ gethints(bool nostdlib)
 	hargs.request = RTLD_DI_SERINFOSIZE;
 	hargs.serinfo = &hmeta;
 
-	path_enumerate(ld_standard_library_path, fill_search_info, &sargs);
-	path_enumerate(hints, fill_search_info, &hargs);
+	path_enumerate(ld_standard_library_path, fill_search_info, NULL,
+	    &sargs);
+	path_enumerate(hints, fill_search_info, NULL, &hargs);
 
 	SLPinfo = xmalloc(smeta.dls_size);
 	hintinfo = xmalloc(hmeta.dls_size);
@@ -1864,8 +1875,9 @@ gethints(bool nostdlib)
 	hargs.serpath = &hintinfo->dls_serpath[0];
 	hargs.strspace = (char *)&hintinfo->dls_serpath[hmeta.dls_cnt];
 
-	path_enumerate(ld_standard_library_path, fill_search_info, &sargs);
-	path_enumerate(hints, fill_search_info, &hargs);
+	path_enumerate(ld_standard_library_path, fill_search_info, NULL,
+	    &sargs);
+	path_enumerate(hints, fill_search_info, NULL, &hargs);
 
 	/*
 	 * Now calculate the difference between two sets, by excluding
@@ -2974,7 +2986,8 @@ rtld_exit(void)
  * callback on the result.
  */
 static void *
-path_enumerate(const char *path, path_enum_proc callback, void *arg)
+path_enumerate(const char *path, path_enum_proc callback,
+    const char *refobj_path, void *arg)
 {
     const char *trans;
     if (path == NULL)
@@ -2986,7 +2999,7 @@ path_enumerate(const char *path, path_enum_proc callback,
void *arg)
 	char  *res;
 
 	len = strcspn(path, ":;");
-	trans = lm_findn(NULL, path, len);
+	trans = lm_findn(refobj_path, path, len);
 	if (trans)
 	    res = callback(trans, strlen(trans), arg);
 	else
@@ -3045,7 +3058,8 @@ try_library_path(const char *dir, size_t dirlen, void
*param)
 }
 
 static char *
-search_library_path(const char *name, const char *path, int *fdp)
+search_library_path(const char *name, const char *path,
+    const char *refobj_path, int *fdp)
 {
     char *p;
     struct try_library_args arg;
@@ -3059,7 +3073,7 @@ search_library_path(const char *name, const char *path,
int *fdp)
     arg.buflen = PATH_MAX;
     arg.fd = -1;
 
-    p = path_enumerate(path, try_library_path, &arg);
+    p = path_enumerate(path, try_library_path, refobj_path, &arg);
     *fdp = arg.fd;
 
     free(arg.buffer);
@@ -3776,12 +3790,12 @@ do_search_info(const Obj_Entry *obj, int request, struct
dl_serinfo *info)
     _info.dls_size = __offsetof(struct dl_serinfo, dls_serpath);
     _info.dls_cnt  = 0;
 
-    path_enumerate(obj->rpath, fill_search_info, &args);
-    path_enumerate(ld_library_path, fill_search_info, &args);
-    path_enumerate(obj->runpath, fill_search_info, &args);
-    path_enumerate(gethints(obj->z_nodeflib), fill_search_info, &args);
+    path_enumerate(obj->rpath, fill_search_info, NULL, &args);
+    path_enumerate(ld_library_path, fill_search_info, NULL, &args);
+    path_enumerate(obj->runpath, fill_search_info, NULL, &args);
+    path_enumerate(gethints(obj->z_nodeflib), fill_search_info, NULL,
&args);
     if (!obj->z_nodeflib)
-      path_enumerate(ld_standard_library_path, fill_search_info, &args);
+      path_enumerate(ld_standard_library_path, fill_search_info, NULL,
&args);
 
 
     if (request == RTLD_DI_SERINFOSIZE) {
@@ -3801,25 +3815,25 @@ do_search_info(const Obj_Entry *obj, int request, struct
dl_serinfo *info)
     args.strspace = (char *)&info->dls_serpath[_info.dls_cnt];
 
     args.flags = LA_SER_RUNPATH;
-    if (path_enumerate(obj->rpath, fill_search_info, &args) != NULL)
+    if (path_enumerate(obj->rpath, fill_search_info, NULL, &args) !=
NULL)
 	return (-1);
 
     args.flags = LA_SER_LIBPATH;
-    if (path_enumerate(ld_library_path, fill_search_info, &args) != NULL)
+    if (path_enumerate(ld_library_path, fill_search_info, NULL, &args) !=
NULL)
 	return (-1);
 
     args.flags = LA_SER_RUNPATH;
-    if (path_enumerate(obj->runpath, fill_search_info, &args) != NULL)
+    if (path_enumerate(obj->runpath, fill_search_info, NULL, &args) !=
NULL)
 	return (-1);
 
     args.flags = LA_SER_CONFIG;
-    if (path_enumerate(gethints(obj->z_nodeflib), fill_search_info,
&args)
+    if (path_enumerate(gethints(obj->z_nodeflib), fill_search_info, NULL,
&args)
       != NULL)
 	return (-1);
 
     args.flags = LA_SER_DEFAULT;
-    if (!obj->z_nodeflib &&
-      path_enumerate(ld_standard_library_path, fill_search_info, &args) !=
NULL)
+    if (!obj->z_nodeflib && path_enumerate(ld_standard_library_path,
+      fill_search_info, NULL, &args) != NULL)
 	return (-1);
     return (0);
 }
diff --git a/sbin/init/rc.d/ldconfig b/sbin/init/rc.d/ldconfig
index 3af9562d3bf..4dabe03e28c 100755
--- a/sbin/init/rc.d/ldconfig
+++ b/sbin/init/rc.d/ldconfig
@@ -58,7 +58,7 @@ ldconfig_start()
 			done
 			check_startmsgs &&
 			    echo '32-bit compatibility ldconfig path:' ${_LDC}
-			${ldconfig} -32 -m ${_ins} ${_LDC}
+			${ldconfig} -32 ${_ins} ${_LDC}
 			;;
 		esac

>> I like to mention one thing concerning the source libmap.c. With the
>> patch (yours or mine) and the libmap32.conf given above I see the
>> following lmp_list when lm_fini() is called:
>>
>>     lm_fini("1, $DEFAULT$" lml-Adresse 0x2826c208)
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib/mysql,
t=/usr/local/lib32/mysql")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=libcrypto.so.8, t=libcrypto.so.6")
>>         lm_fini("f=libssl.so.8, t=libssl.so.6")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>      lm_fini("1, libc-client4.so.9" lml-Adresse 0x2826c168)
>>         lm_fini("f=libcrypto.so.8, t=libcrypto.so.6")
>>         lm_fini("f=libssl.so.8, t=libssl.so.6")
>>      lm_fini("2, /usr/local/php52/" lml-Adresse 0x2826c068)
>>         lm_fini("f=/usr/local/lib/mysql,
t=/usr/local/lib32/mysql")
>>         lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>>
>> So for $DEFAULTS we have a lot of identical entries. This comes from
the
>> TAILQ_INSERT_HEAD statement in lm_add(). I am not sure if this can be
>> accepted or a check to avoid double entries in the list is better.
> Yes, this is mostly cosmetics.  It is not clear is it better to avoid
> duplicates and pay the cost at insertion, or leave them and pay at the
> list traversal.  I think there is slight preference to avoid dups, but
> this should be not measureable.
With your patch for libmap.c the output in lm_fini() now looks fine:
    lm_fini("1, $DEFAULT$" lml-Adresse 0x2826c208)
        lm_fini("f=/usr/local/lib/mysql, t=/usr/local/lib32/mysql")
        lm_fini("f=libcrypto.so.8, t=libcrypto.so.6")
        lm_fini("f=libssl.so.8, t=libssl.so.6")
        lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
     lm_fini("1, libc-client4.so.9" lml-Adresse 0x2826c168)
        lm_fini("f=libcrypto.so.8, t=libcrypto.so.6")
        lm_fini("f=libssl.so.8, t=libssl.so.6")
     lm_fini("2, /usr/local/php52/" lml-Adresse 0x2826c068)
        lm_fini("f=/usr/local/lib/mysql, t=/usr/local/lib32/mysql")
        lm_fini("f=/usr/local/lib, t=/usr/local/lib32")
>> One annotation to the script /etc/rc.d/ldconfig: I had expected that
>> this script during boot creates clean files ld-elf(32).so.hints in
>> /var/run. For 64 bit this is true, but for 32 bit not because ldconfig
>> with flag -32 also has flag -m. Is this intended behaviour ?
> 
> This seems to be from the beginning when ldconfig_local32 was
> introduced in r154114.
Ok

Andreas Longwitz