591331 - Allow Linux dumper to work on PTRACE-hardened kernels (Ubuntu 10.10)

Status: RESOLVED FIXED

Description

[Ted Mielczarek [:ted.mielczarek]]

Chris hit this problem while trying to enable crash reporting on Ubuntu builds. Ubuntu's next release (10.10) includes some kernel hardening changes, one of which locks down ptrace (which the Linux minidump code uses extensively). He patched Breakpad, and his change has landed upstream, so we should take it on trunk so we can have working crash reporting on Ubuntu 10.10.

Upstream change:
http://code.google.com/p/google-breakpad/source/detail?r=673
Upstream review:
http://breakpad.appspot.com/166001/show
Kernel hardening info:
https://wiki.ubuntu.com/SecurityTeam/Roadmap/KernelHardening#PTRACE%2520Protection

Comment 1

[Chris Coulson]

Created attachment 469858 [details] [diff] [review]
Allow Linux dumper to work on PTRACE-hardened kernels

Comment 2

[Ted Mielczarek [:ted.mielczarek]]

Comment on attachment 469858 [details] [diff] [review]
Allow Linux dumper to work on PTRACE-hardened kernels

This got r=nealsid upstream. Requesting approval so we can ship a Firefox 4 with crash reporting that works on Ubuntu 10.10.

Comment 3

[Ted Mielczarek [:ted.mielczarek]]

Requesting blocking for good measure. Ubuntu 10.10 will be out pretty soon, it'd suck to have our crash reporting not working on it.

Comment 4

[Ted Mielczarek [:ted.mielczarek]]

Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/e996bccb391d

We'll probably want to take this on branch as well.

Comment 5

[Ted Mielczarek [:ted.mielczarek]]

Backed this out because tinderbox gcc 4.3 chokes on it. *sigh*

Comment 6

[Ted Mielczarek [:ted.mielczarek]]

Build log:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1283282271.1283283041.21914.gz

ccache /tools/gcc-4.3.3/installed/bin/g++ -o nsUrlClassifierUtils.o -c -I../../../../dist/stl_wrappers -I../../../../dist/system_wrappers -include /builds/slave/mozilla-central-linux/build/config/gcc_hidden.h -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DSTATIC_EXPORTABLE_JS_API -DZLIB_INTERNAL -DOSTYPE=\"Linux2.6.18-8\" -DOSARCH=Linux -I/builds/slave/mozilla-central-linux/build/toolkit/components/url-classifier/src/../../build   -I/builds/slave/mozilla-central-linux/build/toolkit/components/url-classifier/src -I. -I../../../../dist/include -I../../../../dist/include/nsprpub  -I/builds/slave/mozilla-central-linux/build/obj-firefox/dist/include/nspr -I/builds/slave/mozilla-central-linux/build/obj-firefox/dist/include/nss       -fPIC  -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -pedantic -Wno-long-long -gdwarf-2 -fno-strict-aliasing -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -gdwarf-2 -Os -freorder-blocks -fomit-frame-pointer    -DMOZILLA_CLIENT -include ../../../../mozilla-config.h -MD -MF .deps/nsUrlClassifierUtils.pp /builds/slave/mozilla-central-linux/build/toolkit/components/url-classifier/src/nsUrlClassifierUtils.cpp
/builds/slave/mozilla-central-linux/build/toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.cc: In member function 'void google_breakpad::ExceptionHandler::SendContinueSignalToChild()':
/builds/slave/mozilla-central-linux/build/toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.cc:405: error: ISO C++ forbids braced-groups within expressions
/builds/slave/mozilla-central-linux/build/toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.cc: In member function 'void google_breakpad::ExceptionHandler::WaitForContinueSignal()':
/builds/slave/mozilla-central-linux/build/toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.cc:420: error: ISO C++ forbids braced-groups within expressions

Of course this built fine on my local GCC 4.4. We want to update to a newer GCC, but it's not going to happen soon enough, so can we fix this patch to compile with GCC 4.3 in the mantime?

Comment 7

[Chris Coulson]

I see the same failure on our daily builds with GCC <= 4.3 too. It seems that the issue is really with the HANDLE_EINTR macro, and I've just exposed the issue by using it and trying to check the return value.

I couldn't think of any way to reimplement the macro without using this GNU extension. It seems like there are 3 possible solutions:

1) Don't build with -pedantic (I think this is why chromium successfully builds with GCC 4.3, which is using this macro quite a lot)
2) Re-implement the macro as a real function
3) Declare the macro as using a GNU-specific extension (by using __extension__).

I think 3) sounds the most sane, so I'm attaching an updated patch which does that (it also gets rid of the "ISO C++ forbids braced-groups within expressions" warning on GCC 4.4)

Comment 8

[Chris Coulson]

Created attachment 471094 [details] [diff] [review]
Allow Linux dumper to work on PTRACE-hardened kernels (GCC 4.3 compatible)

Comment 9

[Ted Mielczarek [:ted.mielczarek]]

This looks like the best solution, but can we guard that so we only use it on GCC?
#ifdef __GNUC__
#define EXTENSION __extension__
#endif
#define HANDLE_EINTR(x) EXTENSION ( { \
...

? (Clearly this will still break other compilers, but I'm not terribly worried about that, since this is Linux-only.)

Comment 10

[Ted Mielczarek [:ted.mielczarek]]

Although I guess if we're going to break other compilers either way, maybe it doesn't matter.

Comment 11

[Ted Mielczarek [:ted.mielczarek]]

Comment on attachment 471094 [details] [diff] [review]
Allow Linux dumper to work on PTRACE-hardened kernels (GCC 4.3 compatible)

Yeah, let's just take this. I'll land it upstream and in mozilla-central tomorrow.

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

It's past "tomorrow" - progress here? :)

Comment 13

[Ted Mielczarek [:ted.mielczarek]]

The patch is done, it's right on this bug. I got caught up handling b7 blockers etc. This can be landed at any time (presumably post-b7), and I'll ensure that the changes land upstream. (The original patch already landed upstream, it's just the changes between that and the new patch that need to land.)

Comment 14

[Dão Gottwald [::dao]]

http://hg.mozilla.org/mozilla-central/rev/bd8bf5eebcd2