NetBSD Problem Report #41482

From mer@kafka.verkstad.net  Sun May 24 17:46:58 2009
Return-Path: <mer@kafka.verkstad.net>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id E0D6163C2F4
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 24 May 2009 17:46:57 +0000 (UTC)
Message-Id: <20090524174654.301D5710B12@kafka.verkstad.net>
Date: Sun, 24 May 2009 19:46:54 +0200 (CEST)
From: mer@klockrike.net
Reply-To: mer@kafka.verkstad.net
To: gnats-bugs@gnats.NetBSD.org
Cc: mer@klockrike.net
Subject: False cache hits in ld.elf_so
X-Send-Pr-Version: 3.95

>Number:         41482
>Notify-List:    skrll@netbsd.org
>Category:       lib
>Synopsis:       False cache hits in ld.elf_so
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun May 24 17:50:00 +0000 2009
>Closed-Date:    Sat Jan 30 18:58:05 +0000 2010
>Last-Modified:  Sat Jan 30 18:58:05 +0000 2010
>Originator:     Michael Eriksson
>Release:        NetBSD 5.0
>Organization:

>Environment:
Vanilla NetBSD 5.0, x86_64 and i386 and probably other architectures.

>Description:

The ld.elf_so run-time linker has a single-entry cache in
_rtld_find_symdef() for faster symbol lookup. Cache matching is done
on referencing object and symbol number. The referencing object data
structures are allocated on the heap with malloc(), and if there has
been a free() of the last data structure, the same memory area may be
resused, and false cache hits will result. This may happen when an
application uses dlopen().

>How-To-Repeat:

In my case, try to use the py-imagingtk pkgsrc package on NetBSD 5.0
with in-tree xorg X11. This triggered the bug on both i386 and amd64;
I have not tried any other machines or combinations.

The bug will cause symbols to be resolved to the wrong memory address.
I was fortunate and a read-write data symbol was mapped to read-only
code text memory, which caused an easily identifiable segmentation
violation.

>Fix:

Apply the patch below, wich resets the cache in dlopen().

Index: rtld.c
===================================================================
RCS file: /cvsroot/src/libexec/ld.elf_so/rtld.c,v
retrieving revision 1.123
diff -u -r1.123 rtld.c
--- rtld.c	26 Oct 2008 07:11:54 -0000	1.123
+++ rtld.c	24 May 2009 16:04:54 -0000
@@ -780,6 +780,10 @@
 		if (*old_obj_tail != NULL) {	/* We loaded something new. */
 			assert(*old_obj_tail == obj);

+#ifdef COMBRELOC
+			/* Reset the COMBRELOC cache */
+			combreloc_last_refobj = NULL;
+#endif
 			if (_rtld_load_needed_objects(obj, mode) == -1 ||
 			    (_rtld_init_dag(obj),
 			    _rtld_relocate_objects(obj,
Index: rtld.h
===================================================================
RCS file: /cvsroot/src/libexec/ld.elf_so/rtld.h,v
retrieving revision 1.79
diff -u -r1.79 rtld.h
--- rtld.h	4 Oct 2008 09:37:12 -0000	1.79
+++ rtld.h	24 May 2009 16:04:54 -0000
@@ -268,6 +268,9 @@
 unsigned long _rtld_elf_hash(const char *);
 const Elf_Sym *_rtld_symlook_obj(const char *, unsigned long,
     const Obj_Entry *, bool);
+#ifdef COMBRELOC
+extern const Obj_Entry *combreloc_last_refobj;
+#endif
 const Elf_Sym *_rtld_find_symdef(unsigned long, const Obj_Entry *,
     const Obj_Entry **, bool);
 const Elf_Sym *_rtld_symlook_list(const char *, unsigned long,
Index: symbol.c
===================================================================
RCS file: /cvsroot/src/libexec/ld.elf_so/symbol.c,v
retrieving revision 1.47
diff -u -r1.47 symbol.c
--- symbol.c	4 Oct 2008 09:37:12 -0000	1.47
+++ symbol.c	24 May 2009 16:04:54 -0000
@@ -235,6 +235,9 @@
 	return NULL;
 }

+#ifdef COMBRELOC
+const Obj_Entry *combreloc_last_refobj;
+#endif
 /*
  * Given a symbol number in a referencing object, find the corresponding
  * definition of the symbol.  Returns a pointer to the symbol, or NULL if
@@ -261,10 +264,9 @@
 	 */
 	static unsigned long last_symnum;
 	static const Elf_Sym *last_def;
-	static const Obj_Entry *last_refobj;
 	static const Obj_Entry *last_defobj;

-	if (symnum == last_symnum && refobj == last_refobj
+	if (symnum == last_symnum && refobj == combreloc_last_refobj
 	    && in_plt == false) {
 		*defobj_out = last_defobj;
 		return last_def;
@@ -318,7 +320,7 @@
 			 * non-PLT lookup.
 			 */
 			last_symnum = symnum;
-			last_refobj = refobj;
+			combreloc_last_refobj = refobj;
 			last_def = def;
 			last_defobj = defobj;
 		}

>Release-Note:

>Audit-Trail:

From: Nick Hudson <skrll@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/41482 CVS commit: src/libexec/ld.elf_so
Date: Sun, 10 Jan 2010 07:29:47 +0000

 Module Name:	src
 Committed By:	skrll
 Date:		Sun Jan 10 07:29:47 UTC 2010

 Modified Files:
 	src/libexec/ld.elf_so: map_object.c rtld.h symbol.c

 Log Message:
 Reset the COMBRELOC cache Obj_Entry if it was freed.

 Fixes PR 41482. Done slightly differently to the patch in the PR.


 To generate a diff of this commit:
 cvs rdiff -u -r1.38 -r1.39 src/libexec/ld.elf_so/map_object.c
 cvs rdiff -u -r1.82 -r1.83 src/libexec/ld.elf_so/rtld.h
 cvs rdiff -u -r1.48 -r1.49 src/libexec/ld.elf_so/symbol.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Sun, 10 Jan 2010 07:32:01 +0000
State-Changed-Why:
Fixed in -current. Can you check your test case? I'll request pullups once confirmed.


From: Michael Eriksson <mer@klockrike.net>
To: skrll@NetBSD.org
Cc: gnats-bugs@NetBSD.org, lib-bug-people@netbsd.org,
	netbsd-bugs@netbsd.org, gnats-admin@netbsd.org
Subject: Re: lib/41482 (False cache hits in ld.elf_so)
Date: Fri, 22 Jan 2010 09:57:34 +0100

 On Sun, Jan 10, 2010 at 07:32:05 +0000, skrll@NetBSD.org wrote:
 > Synopsis: False cache hits in ld.elf_so
 > 
 > State-Changed-From-To: open->feedback
 > State-Changed-By: skrll@NetBSD.org
 > State-Changed-When: Sun, 10 Jan 2010 07:32:01 +0000
 > State-Changed-Why:
 > Fixed in -current. Can you check your test case? I'll request
 > pullups once confirmed.

 For some reason, I can't reproduce the problem now. I have upgraded to
 5.0.1 (from 5.0) since I reported it, but I can't see how that could
 hide the problem. Maybe the pkgsrc updates during the time has removed
 the sensitivity to the bug.

 Anyway, without having really confirmed your fix, I am happy with it.
 It is rather simple and looks correct to me.


 Regards,
 Michael Eriksson

State-Changed-From-To: feedback->pending-pullups
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Fri, 22 Jan 2010 11:57:37 +0000
State-Changed-Why:
Ticket #1266.


From: Soren Jacobsen <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/41482 CVS commit: [netbsd-5] src/libexec/ld.elf_so
Date: Sat, 30 Jan 2010 18:53:47 +0000

 Module Name:	src
 Committed By:	snj
 Date:		Sat Jan 30 18:53:47 UTC 2010

 Modified Files:
 	src/libexec/ld.elf_so [netbsd-5]: map_object.c rtld.h symbol.c

 Log Message:
 Pull up following revision(s) (requested by skrll in ticket #1266):
 	libexec/ld.elf_so/map_object.c: revision 1.39
 	libexec/ld.elf_so/rtld.h: revision 1.83
 	libexec/ld.elf_so/symbol.c: revision 1.49
 Reset the COMBRELOC cache Obj_Entry if it was freed.
 Fixes PR 41482. Done slightly differently to the patch in the PR.


 To generate a diff of this commit:
 cvs rdiff -u -r1.36.4.1 -r1.36.4.2 src/libexec/ld.elf_so/map_object.c
 cvs rdiff -u -r1.79.4.2 -r1.79.4.3 src/libexec/ld.elf_so/rtld.h
 cvs rdiff -u -r1.47.4.1 -r1.47.4.2 src/libexec/ld.elf_so/symbol.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: snj@NetBSD.org
State-Changed-When: Sat, 30 Jan 2010 18:58:05 +0000
State-Changed-Why:
Pulled up.


>Unformatted:

Home	PR Database Search