156605 - Solaris: Disable stack execution in the Mozilla client executable

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Wan-Teh Chang]

In Solaris 9, one can, at link time, set a switch in an
executable so that it is prevented from executing code
on the stack  This will prevent most buffer overrun attacks.

It would be nice to build the Mozilla client executable
with stack execution disabled when building on Solaris 9.
I haven't found out exactly how to do that except a
one-sentence mention of /usr/lib/ld/map.noexstk (mapfile
showing a non-executable stack definition) in the ld(1)
man page.  See
http://docs.sun.com/db?p=/doc/816-0210/6m6nb7mcs&a=view#indexterm-283.

Comment 1

[hacker formerly known as [e-mail]]

Mass reassign to new default build assignee

Comment 2

[(default assignee for unassigned bugs. does not receive bugmail)]

Mass reassign of Build/Config bugs to Leaf.

Comment 3

[Chase Phillips]

Benjamin, mind taking this?

Comment 4

[Benjamin Smedberg]

I don't know anything about solaris, somebody with a solaris OS would have to do this.

Comment 5

[Wan-Teh Chang]

Alfred, Brian, I filed this enhancement request three
years ago.  The ld man page on Solaris suggests that one
can use a mapfile (-M <mapfile>) to establish a non-executable
stack in the application.  It would be nice to link the
Mozilla clients (SeaMonkey, Firefox, Thunderbird, etc.)
this way to disable stack execution.  Please reassign the
bug to the appropriate person on your team.

Comment 6

[Alfred Peng]

Hi Wan-Teh,
There is a compiler option "-M" to disable stack executable under Solaris just like you've mentioned. But I don't really understand what you proposed to do. If you just want to fix this issue under Solaris, I guess by adding "-M /usr/lib/ld/map.noexstk" to the LDFLAGS, the stack execution can be disabled(I can help you to verify it). If the compilers of the other platforms(such as Linux, Mach...) provide the same option, I can make another option for the mozilla build config to add this functionality.
Any comments?

Comment 7

[Wan-Teh Chang]

Hi Alfred,

The purpose of this enhancement request is disable executable
stacks in the Mozilla clients (SeaMonkey, Firefox, and Thunderbird).
We should use the most convenient technique to achieve that
goal.  I'm not sure if the "-M /usr/lib/ld/map.noexstk" linker
flag is the best technique, but it's the only one I know.  Perhaps
the -M compiler option you mentioned is better.

If it is possible to disable executable stacks, we should
always do it; it should not be a build config option.

Comment 8

[Alfred Peng]

Attachment: Patch for trunk

Add non-executable stack to the Mozilla client.
Wan-Teh, is this what you propose?

Comment 9

[Wan-Teh Chang]

Comment on attachment 207171 [details] [diff] [review]
Patch for trunk

r=wtc.

Alfred, in the future when you modify configure.in,
please don't include the changes to configure in the
patch because the changes to configure can be generated.
The changes to configure are usually very long and
make code review difficult.

I'm wondering if it is better style to make third comma
inside AC_TRY_LINK stand on its own line to emphasize
that it has no associated code.

Comment 10

[Benjamin Smedberg]

Comment on attachment 207171 [details] [diff] [review]
Patch for trunk

Yes, the empty comma command should be on its own line so it's clear that we're "backing out" the change if the AC_TRY_LINK fails. This probably does bad things to cross-compiling scenarios, do we care about that?

Comment 11

[Alfred Peng]

(In reply to comment #10)
> (From update of attachment 207171 [details] [diff] [review] [edit])
> Yes, the empty comma command should be on its own line so it's clear that we're
> "backing out" the change if the AC_TRY_LINK fails. This probably does bad
> things to cross-compiling scenarios, do we care about that?
> 
Wan-Teh, do we care about the cross-compiling issue?
If the patch is ok, please help me to check it in. Thanks.

Comment 12

[Wan-Teh Chang]

Attachment: Patch for trunk v2

I put the empty comma command on its own line, and
added test -z "$CROSS_COMPILE" to resolve the cross
compilation issue (which doesn't exist right now).

Alfred, could you test this patch?  (Remember to
run autoconf to regenerate configure after applying
this patch.)  Do you know any command you can use
to verify that the executable program really has
a non-executable stack?

Comment 13

[Alfred Peng]

Wan-Teh, I've tested the patch for trunk on Solaris. It works fine.
There are two ways we can use to verify the patch.

1. % elfdump -p firefox-bin | tail -6
Program Header[6]:
    p_vaddr:      0               p_flags:    [ PF_W  PF_R ]
    p_paddr:      0               p_type:     [ PT_SUNWSTACK ]
    p_filesz:     0               p_memsz:    0
    p_offset:     0               p_align:    0
You can see that the stack part has read/write access, but no execute access.

2. You can use ptools on the application to verify the stacks
accessibility:

% truss -Texit ./main
.......
% pmap -x 25345 (PID)
25345: ./firefox
  Address  Kbytes     RSS    Anon  Locked Mode   Mapped File
08036000      72      72      72       - rw---    [ stack ]
08050000     120     120       -       - r-x--  firefox-bin
0807D000      40      40       4       - rwx--  firefox-bin
......
The stack has no execute access.

Comment 14

[Alfred Peng]

Wan-Teh, I'm sorry that I don't have the permission to review the patch.

(In reply to comment #13)
> % truss -Texit ./main
Should be truss -Texit ./firefox

Comment 15

[Wan-Teh Chang]

Comment on attachment 207528 [details] [diff] [review]
Patch for trunk v2

Alfred, thanks for testing the patch and verifying
that the stack is not executable.

I checked in this patch on the Mozilla trunk for
Gecko 1.9 alpha.

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1581; previous revision: 1.1580
done

I also gave you the permission to review patches.

Comment 16

[Alfred Peng]

Comment on attachment 207528 [details] [diff] [review]
Patch for trunk v2

See the comments above

Comment 17

[Alfred Peng]

Comment on attachment 207528 [details] [diff] [review]
Patch for trunk v2

Firefox2.0 will be delivered into opensolaris. And the patch is helpful. Since it's an Solaris specific patch, and I've made some test on Solaris platform, I apply the approval for branch MOZILLA1.8.

Comment 18

[Alfred Peng]

wtchang, please help me to check in the patch to branch1.8.1.

Comment 19

[Wan-Teh Chang]

Attachment: Patch for the MOZILLA_1_8_BRANCH

I checked in the fix on the MOZILLA_1_8_BRANCH, which
required a different patch because the configure.in file
is slightly different.

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1503.2.27; previous revision: 1.1503.2.26
done