Closed Bug 1653852 Opened 4 years ago Closed 2 years ago

Crash reporter is completely broken in Flatpak because of its sandbox

Categories

(Core :: Widget: Gtk, defect, P2)

78 Branch
Desktop
Linux
defect

Tracking

()

RESOLVED FIXED
114 Branch
Tracking Status
firefox111 --- wontfix
firefox112 --- wontfix
firefox113 --- wontfix
firefox114 --- fixed

People

(Reporter: code, Assigned: jld)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

  1. Verify that the “Allow Firefox to send backlogged crash reports on your behalf” preference is enabled.
  2. Do anything that cause a tab in Firefox to crash. (For me, I can login to onedrive.com, or browse on forbes.com or bing.com for 30 seconds to cause a crash on demand.)
  3. Go to about:crashes

Actual results:

No crash reports are listed in about:crashes.

Expected results:

The crash should have listed and reported the crash to Mozilla so it can be fixed in a future update.

Fedora 32 with Firefox 78 installed through Flathub.

Hi,

We do not have right environment set up in order to reproduce this issue but I will set the component for it and maybe one of our developers will be able to reproduce it on their end.

Thanks for the report.

Best regards,

Clara.

Component: Untriaged → Widget: Gtk
Product: Firefox → Core
Blocks: flatpak
Priority: -- → P2

:mkaply, are we officially supporting flatpak stuff now? It's unclear to me based on bug 1278719 comment 33 and preceding discussion. Not getting crash reports from flatpak builds is pretty bad, so if we do own it, we should try and get this resolved quickly.

Flags: needinfo?(mozilla)

Yes, we product this Flatpak and should try to make crash reports work if we can.

Flags: needinfo?(mozilla)

:gsvelto, are you aware of this issue (crash reports not being sent on flatpak builds) and/or if anybody is working on it? Also not sure if this bug should be moved to the Crash Reporting component.

Flags: needinfo?(gsvelto)

Are we shipping a flatpak of our own, or is this bug against Fedora's?

We ship a flatpak of our own.

https://flathub.org/apps/details/org.mozilla.firefox

This is built by us.

To clarify: I reported this against the official Mozilla flatpak that is distributed through Flathub. I forgot to specify that initially.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)

:gsvelto, are you aware of this issue (crash reports not being sent on flatpak builds) and/or if anybody is working on it? Also not sure if this bug should be moved to the Crash Reporting component.

I wasn't. From comment 0 I'd say that crash reports aren't being generated at all. I will try to repro if I find some time this week.

Flags: needinfo?(gsvelto)

I can confirm this bug. Firefox crashes multiple times a day (probably due to WebRenderer being used) but about:crashes is always empty.

Alright, I haven't been able to take a look but I'll pick this up first thing in September when I come back from PTO.

Status: UNCONFIRMED → NEW
Ever confirmed: true

Is there anything I can do as an end-user to work around this issue? I’ve tried unsuccessfully to reproduce the crashes in a non-Flatpak’d version of Firefox, but it seems to only affect the Flatpak version.

Gabriele, have you had a chance to look into this?

Flags: needinfo?(gsvelto)

Yes I have. The issue seems to be related to how flatpak is sandboxing Firefox. From what I can tell no minidump is generated at all, not even the folders which should hold it. I don't know what flatpak does to open() calls so I'd have to look into that; I suspect that it's just a matter of handling the various paths differently.

Flags: needinfo?(gsvelto)

With Snap we had problems with it blocking ptrace; I had to send them a patch to their seccomp-bpf policies (theirs, not ours) to allow a read-only subset of it, because of a kernel bug where the ability to write registers could bypass the seccomp filter. I don't know offhand if Flatpak is similar.

As far as file paths, Flatpak appears to use bubblewrap, which puts the program into its own mount namespace and rearranges the filesystem inside it to limit access. /proc/<pid>/root resolves to the actual root directory (ignore what readlink says), so you can do things like cd /proc/<pid>/root to look around.

The MOZ_CRASHREPORTER_EVENTS_DIRECTORY env is set to /home/user/.mozilla/firefox/my-profile.default-release/crashes/events.

The Flatpak is configured to persist /home/user/.mozilla/, so it’s mounted inside the Flatpak sandbox when it runs as a read-write directory (it’s actual location is /home/user/.var/app/org.mozilla.firefox/.mozilla/, but Firefox and the crashlogger can write to either directory paths).

Are there other folders involved in crashlogging? /tmp, perhaps?

(In reply to Daniel from comment #16)

The Flatpak is configured to persist /home/user/.mozilla/, so it’s mounted inside the Flatpak sandbox when it runs as a read-write directory (it’s actual location is /home/user/.var/app/org.mozilla.firefox/.mozilla/, but Firefox and the crashlogger can write to either directory paths).

Does that mean that calls to open a file under ${HOME}/.mozilla will be redirected automatically? That should work. Moving the files there inside the user profile is not a good idea though: the crash reporter is meant to work even when there is no user profile or if there's several. That's why crashes are not written inside the user profile directory by default.

(In reply to Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ from comment #15)

With Snap we had problems with it blocking ptrace[…]. I don't know offhand if Flatpak is similar.

It is. Here's the line where they block ptrace, and if that's removed then crash reporting works.

As far as a change that the Flatpak project might accept, we'd want to do the same restrictions we did for Snap (to read the process state but not modify it), and ideally it would be applied only to packages like Firefox that need it.

I'll file an issue.

Assignee: nobody → jld
Summary: Firefox flatpak edition crashes frequently but about:crashes is empty → Flatpak sandbox breaks crash reporting
Duplicate of this bug: 1803064
Attached image Screenshot_20230405_190356.png (deleted) —

Debian Testing, KDE Wayland
STR
$ sudo apt install flatpak
$ sudo flatpak remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo
$ sudo flatpak install org.mozilla.firefox
$ flatpak run org.mozilla.firefox https://bug1683946.bmoattachments.org/attachment.cgi?id=9288209
Crash reporter does not work.

Blocks: 1826598
OS: Unspecified → Linux
Hardware: Unspecified → Desktop
Summary: Flatpak sandbox breaks crash reporting → Crash reporter is completely broken in Flatpak because of its sandbox

https://github.com/flatpak/flatpak/issues/5203#issuecomment-1339583654

Flatpaks with --allow=devel (e.g. IDEs) are allowed to use ptrace so it should be generally safe I guess?

Can Flatpak Firefox be build with that flag?

(In reply to Darkspirit from comment #22)

https://github.com/flatpak/flatpak/issues/5203#issuecomment-1339583654

Flatpaks with --allow=devel (e.g. IDEs) are allowed to use ptrace so it should be generally safe I guess?

Can Flatpak Firefox be build with that flag?

In theory, yes: adding that flag to this command should add features=devel to the manifest. We'd want to make sure that that won't cause any scary warnings in installer UIs, at a minimum.

As far as actual security, Firefox is still contained within a pid namespace, so it can't use ptrace (or anything else controlled by features=devel) to attack other processes. There are two things it allows that the crash reporter doesn't need:

  1. Using ptrace to modify thread state (not just read it). In particular, this was a way to escape seccomp-bpf restrictions until it was fixed in kernel 4.8 (Oct. 2016).
  2. perf_event_open, which (as mentioned in the flatpak source) has had a number of security issues; however, we do seem to use that for some optional profiler features, as seen in e.g. bug 1794327.

But, even if we could add a new feature to Flatpak for read-only ptrace access, we wouldn't be able to use it until distros got the new version, and we might not even be able to add it to the manifest without breaking compatibility with old versions (not sure about this part). So, if I understand the situation correctly, --allow=devel seems to be the way to go here.

It seems --allow=devel can also be used with the run command, the crash reporter works then:
$ flatpak run --allow=devel org.mozilla.firefox https://bug1683946.bmoattachments.org/attachment.cgi?id=9288209
= bp-34a80c97-7329-426a-95ca-258810230406

(In reply to Darkspirit from comment #24)

It seems --allow=devel can also be used with the run command, the crash reporter works then:

Indeed. Also, commands like flatpak override --user --allow=devel org.mozilla.firefox allow setting it persistently.

Which also means the crash reporter could be enabled by default and those who don't want it, could disable it:
https://docs.flatpak.org/en/latest/flatpak-command-reference.html

--disallow=FEATURE
Disallow access to a specific feature. This updates the [Context] group in the metadata. FEATURE must be one of: devel, multiarch, bluetooth, canbus, per-app-dev-shm. This option can be used multiple times.

mkaply: Do you know who we should ask about whether adding features=devel to our Flatpak manifest would be reasonable?

Flags: needinfo?(mozilla)

Any thoughts Martin?

Flags: needinfo?(mozilla) → needinfo?(stransky)

Jan is our flatpak person here.

Flags: needinfo?(stransky) → needinfo?(jhorak)

Asked the flatpak guys, they basically support Jed's comment 23.

Flags: needinfo?(jhorak)

Marking this as S2 because for example the Steam Deck is Flatpak.

Severity: -- → S2

In fact, I've been debugging the constant crashes I've had the past few days on 112.0, went to about:crashes, there were two reports which I uploaded...

Here's one: https://crash-stats.mozilla.org/report/index/98b86f9e-75a3-4387-ab24-8d2860230502

But it is showing the wrong Firefox version! It says I'm running 104.0.1 while the About screen says I'm on 112.0.2. So seems like that even if the crash reporter were to work, there's some incorrect data in there.

Pushed by jedavis@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f8cb988231f6 Add features=devel to our flatpak packages to allow our crash reporter to work. r=jhorak
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 114 Branch

The patch landed in nightly and beta is affected.
:jld, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox113 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(jld)

(In reply to Release mgmt bot [:suhaib / :marco/ :calixte] from comment #36)

The patch landed in nightly and beta is affected.
:jld, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox113 to wontfix.

For more information, please visit BugBot documentation.

This is late in the release cycle, and we don't have Nightly Flatpak builds so it won't get user testing until it hits beta. I think this can ride the train with 114.

(In reply to Stephane Travostino from comment #33)

In fact, I've been debugging the constant crashes I've had the past few days on 112.0, went to about:crashes, there were two reports which I uploaded...

Here's one: https://crash-stats.mozilla.org/report/index/98b86f9e-75a3-4387-ab24-8d2860230502

But it is showing the wrong Firefox version! It says I'm running 104.0.1 while the About screen says I'm on 112.0.2. So seems like that even if the crash reporter were to work, there's some incorrect data in there.

Is it possible that that was an old crash report that wasn't submitted until now? I notice that the install time on that crash report is 2022-08-31, and the install time is reset on upgrades, so it seems plausible that that really was from 104.0.1.

QA Whiteboard: [qa-114b-p2]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: