Bug 395

Summary: ELF Loader needs to mark PLT executable
Product: xorg Reporter: John Dennis <jdennis>
Component: Server/GeneralAssignee: Egbert Eich <eich>
Status: RESOLVED FIXED QA Contact:
Severity: blocker    
Priority: high CC: mharris
Version: git   
Hardware: PA-RISC (HP)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 213    
Attachments:
Description Flags
set execute permission on PLT none

Description John Dennis 2004-03-30 11:46:45 UTC 
The ELF loader marks memory being used for executable code as executable via
mprotect. The PLT (Procedure Linkage Table) also contains executable code and
due to an oversight when the elfloader creates the PLT via malloc the memory
used to contain the PLT was not marked exeuctable as is done with other
executable memory regions in the loader. Unless this done any OS which does not
allow by default executable data areas for security reasons will cause the
Xserver to fault.
Comment 1 John Dennis 2004-03-30 11:47:58 UTC 
Created attachment 170 [details] [review]
set execute permission on PLT
Comment 2 Mike A. Harris 2004-04-01 14:21:24 UTC 
This bug should be a blocker of the release IMHO, because without this fix,
Nvidia's proprietary drivers for ia64 are totally unuseable.  The problem
may affect other drivers as well, however the only known one at this point
is Nvidia's.  Adding to blocker bug 213.

Additional note: The patch is from Noah Gibbs of NVidia for credititation

I'm not sure wether this patch should affect all architectures, or wether
it should be:

#if defined(ia64) && (defined(linux) || defined(OpenBSD))

Comments?
Comment 3 John Dennis 2004-04-01 14:42:45 UTC 
Agreed it should be a blocker. As to whether linux and BSD are sufficent all I
can say is that at the moment I believe most of the other execute permission
patches have the same conditionalization so its consistent. But is it
sufficient? That's another story. My suggestion is that we need to do some clean
up in this area, the current patches are kind of a hack. I'd leave the
conditionals the way the are for now, in the near term if its not sufficient
we'll know about it promptly. In the interim maybe we can bring some order to
this stuff. I think the right answer long term is for a separate heap mananger
for executable memory. I'd also LOVE to get rid of the "roll your own and have
it constantly break and not be useable with a debugger" proprietary loader and
instead use system services. If we bring some sanity to module loading some of
these issues are going to evaporate.
Comment 4 Bugzilla Maintainer 2004-04-02 08:58:46 UTC 
*** Bug 399 has been marked as a duplicate of this bug. ***
Comment 5 Egbert Eich 2004-04-02 09:25:33 UTC 
 69. Mark PLT table executable on OSes that don't allow to execute data
     areas by default (Bugzilla 395, John Dennis).

Done.
Comment 6 Mike A. Harris 2004-04-03 02:51:25 UTC 
Agreed, the ELF loader needs to go away.  It brings no real world major
benefits, and does waste a lot of X developer's time trying to fix bugs in it
and problems caused by it.  Additionally it makes it much more difficult to
sanely debug the X server.

I've filed a bug the other day to track the problems in the dlopen() loader
that exist currently, and marked existing bugs as blockers of it:

http://pdx.freedesktop.org/cgi-bin/bugzilla/show_bug.cgi?id=400

There's quite a bit of agreement between many developers on killing the ELF
loader once the dlopen() loader can be fixed to work properly, or at least
generating .so modules by default and deprecating the ELF loader.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.