Bugzilla Bug 90287

Whileing merging x86 (any)..

strip: i686-pc-linux-gnu-strip --strip-unneeded
   usr/lib/libxvidcore.so.4.0

usr/lib/libxvidcore.so.4.0 will 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.
---------------------------------------------------------------

When xvid uses nasm vs gas the resulting library will contain text relocations.
The default behavior of the ebuild is if arch is x86 then it forces nasm into 
the deps. If nasm is not installed then the resulting ELF shared object builds 
properly and passes all ELF q/a checks.

Few ways to work around and or fix this...

solar@simple ~ $ epkginfo media-libs/xvid
Package: media-libs/xvid
Herd: video
Maintainer: media-video@gentoo.org
Location: /var/cvsroot/gentoo-x86/media-libs/xvid
Keywords: xvid-0.9.1:  
Keywords: xvid-0.9.2-r1:  
Keywords: xvid-1.0.0_rc4:  ~hppa 
Keywords: xvid-1.0.1:  ~alpha 
Keywords: xvid-1.0.2:  ppc amd64 ~mips ppc64 arm sparc ia64 alpha hppa x86 
Keywords: xvid-1.0.3:  ~amd64 ~x86 ~ia64 ~ppc ~sparc ~ppc64 
ChangeLog: 6 mholzer, 3 phosphan, 3 weeve, 3 azarah, 2 eradicator, 2 agriffis, 2 kugelfang, 2 corsair, 1 zypher, 1 hardave, 1 tgall, 1 gerk, 1 absinthe, 1 raker, 1 luckyduck, 1 lu_zero, 1 doctomoe, 1 rajiv, 1 agenkin, 1 vapier, 1 kloeri

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 an attachment (id=71295) [details]
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 an attachment (id=74905) [details]
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 an attachment (id=80284) [details]
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 :)