| Summary: | media-libs/xvid: ELF text relocations / executable stacks | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | solar (RETIRED) <solar> |
| Component: | [OLD] Library | Assignee: | Gentoo Media-video project <media-video> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | flash3001, kevquinn, lares.moreau, pageexec, qa |
| Priority: | High | ||
| Version: | 2005.0 | ||
| Hardware: | x86 | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Attachments: |
TEXTREL fix for xvid
xvid-1.1.0-noexec-stack.patch TEXTREL fix for xvid-1.1.0 |
||
|
Description
solar (RETIRED)
2005-04-24 13:54:13 UTC
xvid-1.1.0_beta2-r1 behaves this way also. solar can you provide a patch or at least info on how to fix this? i don't really know. The quickest way is to do. ac_cv_prog_ac_nasm=no econf Kevin F. Quinn (kevquinn) is pretty handy at ASM PIC fixes so he might be willing to take a stab at this bug. Also in ./build/generic/bootstrap.sh you probably would want to change to supress errors. -AC_VER=`$AUTOCONF --version | head -1 | sed 's/'^[^0-9]*'/''/'` +AC_VER=`$AUTOCONF --version | head -n 1 | sed 's/'^[^0-9]*'/''/'` Till then.
Index: xvid-1.1.0_beta2-r1.ebuild
===================================================================
RCS file: /var/cvsroot/gentoo-x86/media-libs/xvid/xvid-1.1.0_beta2-r1.ebuild,v
retrieving revision 1.4
diff -u -b -B -w -p -r1.4 xvid-1.1.0_beta2-r1.ebuild
--- xvid-1.1.0_beta2-r1.ebuild 28 Jun 2005 22:00:13 -0000 1.4
+++ xvid-1.1.0_beta2-r1.ebuild 4 Aug 2005 20:53:55 -0000
@@ -37,6 +37,7 @@ src_unpack() {
}
src_compile() {
+ export ac_cv_prog_ac_nasm=no
econf \
$(use_enable altivec) \
|| die "econf failed"
I'll put it on my list :) Nothing can be done without sacrificing nasm optimizations? I've just had a look, and it's not a quick job. In just the x86 asm code, there are 61 global symbols, and getting on for 700 references of those symbols in total (by a quick very rough count). These all need to be changed to GOT-relative; this means significant changes to all the asm files. Then there's the asm for the other architectures... I don't know how popular this would be, but how about a 'strictnotextrel' use flag, and do the "export ac_cv_prog_ac_nasm=no" conditional on that so that users could specify whether they are prepared to take the performance hit of configuring with no optimised asm code. 'strictnotextrel' should not even be an option. TEXTREL's in shared objects are already prohibited by some policy or another. This bug needs to be fixed or pulled out of stable on atleast x86. Perhaps the inverse would be better, say 'USE=nastytextrels'. Thing is, the performance benefits of the asm code are significant, and for most people textrels in shared libraries that are only loaded by one application are not a significant issue; it's not often one would run two separate xvid applications simultaneously, and the work incurred by textrels at load time (which non-hardened users can alleviate with prelink) isn't as important as the speed at which the application runs. It'd be easy to use.mask it or whatever in the hardened profile, where the conflict between textrels and grsecurity make it a significant issue. An asm patch is being developed by the PaX author for this. This bug has been open for 6 months and has had a working solution on it for atleast 2 months (comment #3) and has still been ignored by media-video@ for several months. These are real QA problems. i'll tackle this in portage/upstream if the video herd doesnt care Created attachment 71295 [details, diff]
TEXTREL fix for xvid
this is my first shot, at least it builds and gets rid of all textrels and
fixes GNU_STACK as well. it's rather big and i expect to have all kinds of
funny bugs in this, any review/testing/etc is more than welcome. some
implementation notes:
1. yasm is out as it can't produce section names with a dash in it, so we
couldn't produce GNU-stack. nor does it support negative section flags
(noalloc, noexec, etc), and even though it has usable defaults, nasm doesn't so
we at least have to use noalloc - no dice.
2. the general approach was to find an unused register and use it as the PIC
base register, i hope i got it all right, but i'm not an mmx/xmm/3dn/etc guru,
there could be side effects of some insns (or i just plain missed something)
that would invalidate my choice - review, review, review ;-).
3. the get_pc stubs are not in any special linker section, i figured it's not
worth the extra typing/setup to save just a few bytes.
The video team is quite busy with other problems. Such patches are often rejected by upstream because problematic in many ways. I'll be more happy if the patch is pushed upstream before taking action. (In reply to comment #13) > The video team is quite busy with other problems. Such patches are often > rejected by upstream because problematic in many ways. I'll be more happy if the > patch is pushed upstream before taking action. of course it's best when upstream takes (and fixes) it, but that's about the last thing i have time for myself, so someone else will have to work that out. in the meantime, can you at least offer it in hardened gentoo? *some* testing is still better than nothing... since the code was taking COFF into consideration, i think this is a case where using ELF format checks around the stack markings makes sense ;) (In reply to comment #15) > since the code was taking COFF into consideration, i think this is a > case where using ELF format checks around the stack markings makes sense ;) be my guest ;-), i've spent way too much time on this already... probably diff -D can be of some help. *** Bug 115562 has been marked as a duplicate of this bug. *** Mike want to submit a version with ELF checks around gnu stack markings? If you'll do, I'll commit it finally (took time to be able to understand what it does, but I think I got it someway...). that isnt possible atm unless i missed something yasm doesnt allow section names with a '-' in them, so using SECTION .note.GNU-stack noalloc noexec nowrite progbits will cause yasm to abort That's why the first chunk of the patch actually disable yasm? (In reply to comment #20) > That's why the first chunk of the patch actually disable yasm? if comment #12 says so... ;-) ok, turns out the syntax has changed in yasm ... this works: SECTION ".note.GNU-stack" noalloc noexec nowrite progbits in yasm, we need to quote, but in nasm, that'll create the wrong section name (nasm will output the " as part of the name in the object file) ok, i chatted with yasm upstream, and it seems that the 0.4.0 has a few bugs in it that'll prevent the fixes for xvid from actually working ... but said bugs have been fixed in the latest svn imo we should apply the correct patch even though the current yasm release is broken and will still generate executable stack code Created attachment 74905 [details, diff]
xvid-1.1.0-noexec-stack.patch
this fixes exec stack for amd64(yasm), x86(nasm), and ia64(gas)
I've committed the -noexec-stack.patch. As soon as Mike attach the other patch I'll do a revbump, too. About yasm, there's something we can backport to get it working on amd64 or when a new release is planned? i dunno about back porting, just might be easier to ask for a new release /me does so now they said they're going to try for a 0.5.0 in jan/feb so that's OK for me to wait Created attachment 80284 [details, diff]
TEXTREL fix for xvid-1.1.0
updated patch for 1.1.0, it's on top of (after) the GNU-stack patch already in portage.
also, considering that
1. upstream wants (?) gcc-2.x support,
2. gentoo doesn't support gcc-2.x (profile masked),
3. i have no time for this anyway,
could it be enabled on USE=hardened at least?
media-libs/xvid-1.1.0: QA Notice: the following files contain executable stacks Files with executable stacks will not work properly (or at all!) on some architectures/operating systems. A bug should be filed at http://bugs.gentoo.org/ to make sure the file is fixed. Please include this file in your report: /var/tmp/portage/xvid-1.1.0/temp/scanelf-exec.log RWX --- --- usr/lib64/libxvidcore.so.4.1 (In reply to comment #29) > media-libs/xvid-1.1.0: what's your USE flags and ebuild used for this compilation? > Please include this file in your report: > /var/tmp/portage/xvid-1.1.0/temp/scanelf-exec.log > RWX --- --- usr/lib64/libxvidcore.so.4.1 was that all in the .log (you could post the scanelf -T output in any case)? also, lib64 implies that you're on a 64 bit arch, so my fixes are unlikely to help given they're for i386. i'm pretty sure Sandro's pax-utils is broken; i'm handling that in a different bug i checked xvid on my amd64 though and the patch addresses that issue already This is fixed with latest yasm, so closing down. I've just emerged xvid-1.1.0 on my system (x86) and I got this:
QA Notice: the following files contain runtime text relocations
Text relocations require a lot of extra work to be preformed by the
dynamic linker which will cause serious performance impact on IA-32
and might not function properly on other architectures hppa for example.
If you are a programmer please take a closer look at this package and
consider writing a patch which addresses this problem.
TEXTREL usr/lib/libxvidcore.so.4.1
> This is fixed with latest yasm, so closing down.
x86 xvid uses nasm not yasm and it seems it's not fixed there yet
Hmm Mike can you confirm? I'm on amd64 but I thought we were just waiting for yasm right now? no ... we were talking about execstacks, not TEXTRELs (comment #24) Okay I've added the patch, sorry for the confusion. I've removed the part that removes yasm for amd64, it doesn't build textrels here anyway. Thanks a lot for the patch as usual :) |