daily builds do not have the ability to gather memory reports

RESOLVED FIXED

Status

Firefox OS
GonkIntegration
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: onecyrenus, Assigned: dhylands)

Tracking

unspecified
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-basecamp:+)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Daily builds currently don't have the ability to gather memory reports. 

The basics behind this: 

/system/bin/killer application needs to be installed from gonk-misc in order to trigger a memory dump of b2g. 

The current reason why it is not part of the nightly builds is because there is a security hole which requires killer to be launched with root access. 

This bug is to fix the killer application so it doesn't require root in order to trigger a memory dump, or gc.
(Reporter)

Updated

5 years ago
blocking-basecamp: --- → ?
(Reporter)

Updated

5 years ago
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
> This bug is to fix the killer application so it doesn't require root in order to trigger 
> a memory dump, or gc.

ISTM that if we made killer "safe" (in that it could only send certain signals to certain processes) that we could setuid root and be done.  That is, I don't think making killer not require root is a necessary condition here.
(Assignee)

Comment 2

5 years ago
I confirmed that using a setuid root version of killer works when run from and adb shell that was owned by the user shell.
(Assignee)

Updated

5 years ago
Assignee: nobody → dhylands
(Reporter)

Updated

5 years ago
Blocks: 797189
(Assignee)

Comment 3

5 years ago
Created attachment 674867 [details] [diff] [review]
WIP (patch to b2g/gonk-misc): One possible way

This is one possible way to achieve the goal. This particular solution has the negative side-effect that killer would need to be marked as setuid root (which would need to happen in init.rc) in order to work on a production phone (where adb shell runs as user shell rather than user root)
Attachment #674867 - Flags: review?(justin.lebar+bug)
Attachment #674867 - Attachment description: WIP: One possible way → WIP (patch to b2g/gonk-misc): One possible way
Comment on attachment 674867 [details] [diff] [review]
WIP (patch to b2g/gonk-misc): One possible way

Since killer will be setuid(0), we need to cast a paranoid eye on it.

So to be paranoid: A user can largely control which PID a new process gets.  If I can cause the B2G or plugin-container process to die at just the right time, I can cause this program to send a signal to an arbitrary new process.

Is there something we can do about this?
(Assignee)

Comment 5

5 years ago
So, you'd have to launch a process which was also setuid(0) and it would have to do something bad upon receiving one of the signals that we send. Because you can use regular kill on the non-setuid(0) programs.

I don't see anyway to close the window between checking the pid and issuing the kill. In kernel land you could disable preemption, but from user-space you can't do that.

The "right" way to deal with this is to probably abandon the killer approach and use a named-pipe. Then there won't be the same security holes.
> So, you'd have to launch a process which was also setuid(0) and it would have to do 
> something bad upon receiving one of the signals that we send. Because you can use regular 
> kill on the non-setuid(0) programs.

Right.  I think the default action for a realtime signal is to crash.

> The "right" way to deal with this is to probably abandon the killer approach and use a 
> named-pipe. Then there won't be the same security holes.

:-/  That's probably right.

The named pipes thing wouldn't be hard, but we need this now, so let's do this as you've written and we can worry about named pipes later.
Comment on attachment 674867 [details] [diff] [review]
WIP (patch to b2g/gonk-misc): One possible way

r=me, but please update the usage and comment at the top of the file to indicate that this is a severely restricted clone of kill.
Attachment #674867 - Flags: review?(justin.lebar+bug) → review+
Getting memory reports is a blocker. But of course we should make sure that the mechanism to do so is safe.
blocking-basecamp: ? → +
(Assignee)

Comment 9

5 years ago
Making killer part of the user builds has landed.
See:
https://github.com/mozilla-b2g/gonk-misc/pull/47
https://github.com/mozilla-b2g/platform_build/pull/9

The patch attached to this bug (with the comments and usage updated) is here:
https://github.com/mozilla-b2g/gonk-misc/pull/48

By adding setuid root (which needs to be done as part of the kernel/initial ramdisk) needs to be done elsewhere. If this variant of killer is made setuid root, it will allow memory reports to be gathered on a production phone where adb shell is owned by the shell user (currently adb shell is owned by the root user)
(Assignee)

Comment 10

5 years ago
Commit
https://github.com/mozilla-b2g/platform_build/commit/dfc13fee7c539005f869e1335e732e700aa42768
makes killer part of the build

So I'm going to close this bug out.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.