Closed Bug 1417452 Opened 2 years ago Closed 2 years ago

glibc-2.25 prevents building with _FORTIFY_SOURCE without any optimization level.

Categories

(Core :: Security, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: nbp, Assigned: tjr)

References

Details

Attachments

(1 file, 1 obsolete file)

glibc-2.25-dev/include/features.h:
> 
> #if defined _FORTIFY_SOURCE && _FORTIFY_SOURCE > 0
> # if !defined __OPTIMIZE__ || __OPTIMIZE__ <= 0
> #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
> # elif !__GNUC_PREREQ (4, 1)
> #  warning _FORTIFY_SOURCE requires GCC 4.1 or later
> # elif _FORTIFY_SOURCE > 1
> #  define __USE_FORTIFY_LEVEL 2
> # else
> #  define __USE_FORTIFY_LEVEL 1
> # endif
> #endif
> #ifndef __USE_FORTIFY_LEVEL
> # define __USE_FORTIFY_LEVEL 0
> #endif
>
Flags: needinfo?(tom)
This patch adds a way to disable FORTIFY_SOURCE macro definition from the
command line if there is no other hardening flags defined.

This patch enable us to compile with --enable-debug, by setting --disable-hardening.
Attachment #8928518 - Flags: review?(nfroyd)
Attachment #8928518 - Flags: feedback?(tom)
Why is --enable-debug a problem?  Do you mean --enable-debug --disable-optimize?

I'm not sure that conditioning this on hardening flags is really the right thing.
I think we should make --disable-hardening turn off FORTIFY_SOURCE.

But I am not sure we want to require --disable-hardening to be added by users affected by this. It seems like we should make the build 'just work' and not require unexpected flags. 

I suppose the best way to do that is to detect the optimization level and not add FORTIFY_SOURCE if it is not set? Although the drawbacks of that would be that we will disable it in places we don't need to (I have to assume not every libc (or glibc version) is affected by this, since try was not) - and if we tried to be smarter I'm not sure we have the libc package name or version in old-configure...
Flags: needinfo?(tom)
(In reply to Nathan Froyd [:froydnj] from comment #2)
> Why is --enable-debug a problem?  Do you mean --enable-debug
> --disable-optimize?

Yes, this is what I meant. Sorry about the lack of --disable-optimize.
(In reply to Tom Ritter [:tjr] from comment #3)
> I think we should make --disable-hardening turn off FORTIFY_SOURCE.
> 
> ... It seems like we should make the build 'just work'
> and not require unexpected flags. 
> 
> I suppose the best way to do that is to detect the optimization level and
> not add FORTIFY_SOURCE if it is not set?

This would be my favorite solution. Unoptimized builds are slow enough that we will always have enough optimized builds around to catch things, so I'm not worried about the coverage.
I'm going to flag :glandium to see what he thinks, I feel like he may have a good POV on how to proceed.
Flags: needinfo?(mh+mozilla)
Considering glibc's features.h, the right thing to do would be to disable FORTIFY_SOURCE with --disable-optimize.
Flags: needinfo?(mh+mozilla)
See Also: → 1418052
Attachment #8928518 - Attachment is obsolete: true
Attachment #8928518 - Flags: review?(nfroyd)
Attachment #8928518 - Flags: feedback?(tom)
I broke out Bug 1418052 to get this one fixed earlier and because that one is a bit more complicated
Comment on attachment 8929142 [details]
Bug 1417452 Do not add FORTIFY_SOURCE if --enable-optimize is not set

https://reviewboard.mozilla.org/r/200434/#review205710
Attachment #8929142 - Flags: review?(mh+mozilla) → review+
Keywords: checkin-needed
Assignee: nobody → tom
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/59e899eba461
Do not add FORTIFY_SOURCE if --enable-optimize is not set r=glandium
Keywords: checkin-needed
I've just bumped into a similar issue on Gentoo where GCC defines _FORTIFY_SOURCE as a built-in, see bug 1418398.
https://hg.mozilla.org/mozilla-central/rev/59e899eba461
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.