486537 - Disable execstack in freebl x86_64 builds on Linux

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Kai Engert (:KaiE:)]

Originally reported here:
https://bugzilla.redhat.com/show_bug.cgi?id=493663

The current build flags cause libfreebl3 to get built with the execstack flag, causing trouble for selinux.

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Patch v1 (checked in)

Patch proposed by Ulrich

Comment 2

[Robert Relyea]

Comment on attachment 370674 [details] [diff] [review]
Patch v1 (checked in)

r+ rrelyea

for 3.12.4

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 370674 [details] [diff] [review]
Patch v1 (checked in)

Is this an issue only for x86_64 ?

Comment 4

[Ulrich Drepper]

(In reply to comment #3)
> (From update of attachment 370674 [details] [diff] [review])
> Is this an issue only for x86_64 ?

Yes.  The offending asm file is only on x86 and only Linux performs the stack tests.  So it's even Linux-x86-64 specific.

Comment 5

[Wan-Teh Chang]

Comment on attachment 370674 [details] [diff] [review]
Patch v1 (checked in)

r=wtc.

In the past, we have been solving this problem by
adding this magic section to the end of every .s file:

  # Magic indicating no need for an executable stack
  .section .note.GNU-stack,"",@progbits
  .previous

For example, see
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/freebl/arcfour-amd64-gas.s&rev=1.2&mark=118-120#118

So you can also fix this bug by adding this section
to the new .s files added in NSS 3.12.3 (I believe
intel-ase.s is the only one).  I would do that.

If you decide to use the -Wa,--noexecstack solution,
you should do it for all CPU architectures.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

In reply to comment 5,
I remember that we had to remove that magic pseudo-op from one of our .s 
files because gas on some platform didn't recognize it.  I prefer solving
it through a command line option instead of a pseudo-op, if possible,
because it's easier to have command lines that vary by platform than to 
have assembler source that varies by platform.

Comment 7

[Wan-Teh Chang]

Attachment: Use -Wa,--noexecstack for all processor architectures

To be future-proof, you may want to use -Wa,--noexecstack
for all processor architectures.

Comment 8

[Wan-Teh Chang]

Attachment: Set it in coreconf/Linux.mk

We can also set it in coreconf/Linux.mk, which will take
care of all the NSS and JSS directories.

Comment 9

[Wan-Teh Chang]

Comment on attachment 370674 [details] [diff] [review]
Patch v1 (checked in)

Kai, I noticed that you haven't checked in this patch, so I checked it in 
on the NSS trunk (NSS 3.12.4).  I actually prefer passing the -Wa,--noexecstack
flag for all processor architectures (we also have assembly files for
x86), but I checked in this patch because it has received review+.

Checking in Makefile;
/cvsroot/mozilla/security/nss/lib/freebl/Makefile,v  <--  Makefile
new revision: 1.106; previous revision: 1.105
done

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

This bug was originally reported against x86_64 builds on Linux, and was only 
fixed for that platform.  :(

Now it has been reported again for x86_32 builds on Linux in bug 499584.
I propose that we back out the fix committed for this bug, and commit 
wan-Teh's patch in attachment 370782 [details] [diff] [review] instead.  Please follow this issue in 
bug 499584.

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

No. I was mistaken.  Bug 499584 was also about x86_64, and is a duplicate 
of this bug.  However, I do think we should fix it as shown in Wan-Teh's 
patch in attachment 370782 [details] [diff] [review], so that it is fixed for x86_32 also.

Comment 14

[Wan-Teh Chang]

Comment on attachment 370782 [details] [diff] [review]
Set it in coreconf/Linux.mk

If you check in this patch, please back out patch v1 (attachment 370674 [details] [diff] [review])
to avoid duplicate -Wa,--noexecstack flags.

Comment 15

[Robert Relyea]

Wan-Teh, does the duplicate cause a problem with the compilier?

If not, I would prefer to leave it until the next time we open the FIPS token tree.

bob

Comment 16

[Wan-Teh Chang]

Comment on attachment 370782 [details] [diff] [review]
Set it in coreconf/Linux.mk

Then we can wait until then to check in this patch.
I don't think we will add assembly code to any directory
other than lib/freebl.

Comment 17

[Reed Loden [:reed]]

Is this something that should be backported to 3.12.3 for those users still using that branch?

Comment 18

[Nelson Bolyard (seldom reads bugmail)]

Reed, to what branch(es) are you referring?
Naturally, we'd prefer that any products using 3.12.3 simply update to 
3.12.4 now that it has been released.

Comment 19

[Daniel Veditz [:dveditz]]

Linux distros typically use "system NSS" and can easily upgrade to 3.12.4 or even apply this patch to their own 3.12.3 version if they prefer. Firefox will be upgrading to 3.12.4 and any effort should go into making that happen rather than putting this patch on 3.12.3