mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] [PATCH] Add REL_COPY size change detection
@ 2020-02-26  5:24 Markus Wichmann
  2020-02-26 17:36 ` Rich Felker
  0 siblings, 1 reply; 10+ messages in thread
From: Markus Wichmann @ 2020-02-26  5:24 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]

Hi all,

I was recently reading the Oracle docs about the ELF, and I came across
their chapter about the COPY relocation. They discuraged its use, since
with those relocations, a binding exists between importing and exporting
module. If the semantics of the imported object changes, then this is an
ABI mismatch.

So I looked at the musl source code and noticed that COPY relocations
are simply processed, and an ABI mismatch is simply accepted. So, since
I am of the opinion that detectable errors should be detected, rather
than left to fester and spring a hard-to-explain bug on you, usually
five minutes before deadline, I wrote the attached patch to add
detection for at least a changed size. This won't detect all changes to
ABI regarding COPY relocation (e.g.int-->float, or in an array of
structs, a change to the struct size and to the array size cancelling
each other out), but it should find most of them.

Also, I wondered whether COPY relocations are even still in use. But on
my system (currently some Ubuntu version) I found over 15000 of the
things. Mostly for stdout and stderr, though.

Ciao,
Markus

[-- Attachment #2: 0001-Add-detection-for-changed-size-of-a-COPY-relocation.patch --]
[-- Type: text/x-diff, Size: 1631 bytes --]

From 75e98f4e4cef2eb2b867062aebc481c3b1f66498 Mon Sep 17 00:00:00 2001
From: Markus Wichmann <nullplan@gmx.net>
Date: Wed, 26 Feb 2020 06:09:14 +0100
Subject: [PATCH] Add detection for changed size of a COPY relocation.

COPY relocations create an ABI binding between importing and exporting
module. Should anything about the object in question change, that would
be an ABI change, and therefore incompatible. While the dynamic linker
is not capable of detecting all changes, it can detect most of them by
detecting a changed size between import and export. Any change is a
problem, since the source buffer will be either overread or underread.
In any case, if the semantics of the imported object changed, the ABI
contract is broken, and it is better to detect this than to silently
allow it and inexplicably crash later on.
---
 ldso/dynlink.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/ldso/dynlink.c b/ldso/dynlink.c
index afec985a..618c2cbd 100644
--- a/ldso/dynlink.c
+++ b/ldso/dynlink.c
@@ -435,6 +435,15 @@ static void do_relocs(struct dso *dso, size_t *rel, size_t rel_size, size_t stri
 			else *reloc_addr = (size_t)base + addend;
 			break;
 		case REL_COPY:
+			if (def.sym && sym->st_size != def.sym->st_size) {
+				error("Error relocating %s: %s: Size mismatch in COPY"
+						" relocation (exp %lu, got %lu)",
+						dso->name, name
+						sym->st_size + 0ul,
+						def.sym->st_size + 0ul);
+				if (runtime) longjmp(*rtld_fail, 1);
+				continue;
+			}
 			memcpy(reloc_addr, (void *)sym_val, sym->st_size);
 			break;
 		case REL_OFFSET32:
--
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-04-17 10:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-26  5:24 [musl] [PATCH] Add REL_COPY size change detection Markus Wichmann
2020-02-26 17:36 ` Rich Felker
2020-02-26 18:38   ` Florian Weimer
2020-02-26 19:00     ` Rich Felker
2020-02-26 19:53       ` Florian Weimer
2020-02-26 22:48         ` Rich Felker
2020-02-27  5:00           ` Fangrui Song
2020-04-17 10:58           ` Florian Weimer
2020-02-26 20:12   ` Markus Wichmann
2020-02-26 22:02     ` Rich Felker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).