| Submitter | Phil Blundell |
|---|---|
| Date | Oct. 3, 2012, 10:24 a.m. |
| Message ID | <1349259854.32611.106.camel@phil-desktop> |
| Download | mbox | patch |
| Permalink | /patch/37669/ |
| State | New |
| Headers | show |
Comments
On Wed, Oct 03, 2012 at 11:24:12AM +0100, Phil Blundell wrote: > Signed-off-by: Phil Blundell <pb@pbcl.net> Can you add a bit longer description of possible issues with relocations in .text? So that people seeing this issue will know how dangerous it is for them? From my understanding (after reading http://www.gentoo.org/proj/en/hardened/pic-fix-guide.xml) it's mostly performance issue? Cheers, > --- > This requires qa.elf.run_objdump() so needs to be applied after the > patch which adds that function. > > meta/classes/insane.bbclass | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass > index 5848ab4..ff35ed8 100644 > --- a/meta/classes/insane.bbclass > +++ b/meta/classes/insane.bbclass > @@ -108,7 +108,7 @@ def package_qa_get_machine_dict(): > > > # Currently not being used by default "desktop" > -WARN_QA ?= "ldflags useless-rpaths rpaths unsafe-references-in-binaries unsafe-references-in-scripts staticdev libdir" > +WARN_QA ?= "ldflags useless-rpaths rpaths unsafe-references-in-binaries unsafe-references-in-scripts staticdev libdir textrel" > ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch la2 pkgconfig la perms dep-cmp" > > ALL_QA = "${WARN_QA} ${ERROR_QA}" > @@ -421,6 +421,30 @@ def package_qa_check_desktop(path, name, d, elf, messages): > for l in output: > messages.append("Desktop file issue: " + l.strip()) > > +QAPATHTEST[textrel] = "package_qa_textrel" > +def package_qa_textrel(path, name, d, elf, messages): > + """ > + Check if the binary contains relocations in .text > + """ > + > + if not elf: > + return > + > + if os.path.islink(path): > + return > + > + phdrs = elf.run_objdump("-p", d) > + sane = True > + > + import re > + textrel_re = re.compile("\s+TEXTREL\s+") > + for line in phdrs.split("\n"): > + if textrel_re.match(line): > + sane = False > + > + if not sane: > + messages.append("ELF binary '%s' has relocations in .text" % path) > + > QAPATHTEST[ldflags] = "package_qa_hash_style" > def package_qa_hash_style(path, name, d, elf, messages): > """ > -- > 1.7.10.4 > > > > > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
On Wed, 2012-10-03 at 12:31 +0200, Martin Jansa wrote: > On Wed, Oct 03, 2012 at 11:24:12AM +0100, Phil Blundell wrote: > > Signed-off-by: Phil Blundell <pb@pbcl.net> > > Can you add a bit longer description of possible issues with relocations > in .text? So that people seeing this issue will know how dangerous it is > for them? > > From my understanding (after reading > http://www.gentoo.org/proj/en/hardened/pic-fix-guide.xml) it's mostly > performance issue? Yes, that's correct. It basically falls into the same sort of category as useless-rpaths; the binary will still work, but there will be some adverse impact on performance and memory usage. Historically, the most common cause of DT_TEXTREL was accidentally linking non-PIC code into a DSO. Recent versions of the linker will flatly refuse to do this on at least some architectures, though, so hopefully this problem will just go away over time. p.
On Wed, 2012-10-03 at 11:44 +0100, Phil Blundell wrote: > On Wed, 2012-10-03 at 12:31 +0200, Martin Jansa wrote: > > On Wed, Oct 03, 2012 at 11:24:12AM +0100, Phil Blundell wrote: > > > Signed-off-by: Phil Blundell <pb@pbcl.net> > > > > Can you add a bit longer description of possible issues with relocations > > in .text? So that people seeing this issue will know how dangerous it is > > for them? > > > > From my understanding (after reading > > http://www.gentoo.org/proj/en/hardened/pic-fix-guide.xml) it's mostly > > performance issue? > > Yes, that's correct. It basically falls into the same sort of category > as useless-rpaths; the binary will still work, but there will be some > adverse impact on performance and memory usage. > > Historically, the most common cause of DT_TEXTREL was accidentally > linking non-PIC code into a DSO. Recent versions of the linker will > flatly refuse to do this on at least some architectures, though, so > hopefully this problem will just go away over time. Am I right in thinking this is also a marginal help to 'security' since if the .text segment is loaded read only, it becomes slightly harder for certain kinds of overflow attacks to work? Cheers, Richard
On Wed, 2012-10-03 at 12:19 +0100, Richard Purdie wrote: > Am I right in thinking this is also a marginal help to 'security' since > if the .text segment is loaded read only, it becomes slightly harder for > certain kinds of overflow attacks to work? Possibly a marginal help, though (for glibc at least) the dynamic linker will restore the original protection on .text once the relocations have been applied, so the window of time during which you could mount an attack based on the writeable .text region will be fairly small. But in principle you're right, for best security you don't want to have any regions which are both writeable and executable. p.
Patch
diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass index 5848ab4..ff35ed8 100644 --- a/meta/classes/insane.bbclass +++ b/meta/classes/insane.bbclass @@ -108,7 +108,7 @@ def package_qa_get_machine_dict(): # Currently not being used by default "desktop" -WARN_QA ?= "ldflags useless-rpaths rpaths unsafe-references-in-binaries unsafe-references-in-scripts staticdev libdir" +WARN_QA ?= "ldflags useless-rpaths rpaths unsafe-references-in-binaries unsafe-references-in-scripts staticdev libdir textrel" ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch la2 pkgconfig la perms dep-cmp" ALL_QA = "${WARN_QA} ${ERROR_QA}" @@ -421,6 +421,30 @@ def package_qa_check_desktop(path, name, d, elf, messages): for l in output: messages.append("Desktop file issue: " + l.strip()) +QAPATHTEST[textrel] = "package_qa_textrel" +def package_qa_textrel(path, name, d, elf, messages): + """ + Check if the binary contains relocations in .text + """ + + if not elf: + return + + if os.path.islink(path): + return + + phdrs = elf.run_objdump("-p", d) + sane = True + + import re + textrel_re = re.compile("\s+TEXTREL\s+") + for line in phdrs.split("\n"): + if textrel_re.match(line): + sane = False + + if not sane: + messages.append("ELF binary '%s' has relocations in .text" % path) + QAPATHTEST[ldflags] = "package_qa_hash_style" def package_qa_hash_style(path, name, d, elf, messages): """
Signed-off-by: Phil Blundell <pb@pbcl.net> --- This requires qa.elf.run_objdump() so needs to be applied after the patch which adds that function. meta/classes/insane.bbclass | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)