Run rilproxy with non-root privileges

RESOLVED FIXED in Firefox OS master

Status

Firefox OS
RIL
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: tzimmermann, Assigned: tzimmermann)

Tracking

unspecified
2.2 S4 (23jan)
All
Gonk (Firefox OS)

Firefox Tracking Flags

(b2g-master fixed)

Details

Attachments

(3 attachments)

Rilproxy requires root privileges because it re-opens a socket that has already been created by init. If we re-use init's existing socket, we can execute rilproxy without root privileges from the beginning.
Created attachment 8549629 [details]
Github pull request for rilproxy

I tested this change on nexus-4 (JB), emulator (ICS), and emulator-kk (KK with multi-SIM). They all work nicely with these patches. Some changes to the init scripts are required, which I'll upload in a minute.
Attachment #8549629 - Flags: review?(kyle)
Attachment #8549629 - Flags: feedback?(mwu)
Attachment #8549629 - Flags: feedback?(htsai)
Created attachment 8549632 [details]
Github pull request for gonk-misc

This patch is required in stable branches as well. How do we best handle that?
Attachment #8549632 - Flags: review?(mwu)
Attachment #8549632 - Flags: feedback?(htsai)
Created attachment 8549636 [details]
Github pull request for device_generic_goldfish on emulator-kk
Attachment #8549636 - Flags: review?(mwu)
Attachment #8549636 - Flags: feedback?(htsai)
There seems to be a similar bug 812394, but don't have permission to see it.
See Also: → bug 843388
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #2)
> Created attachment 8549632 [details]
> Github pull request for gonk-misc
> 
> This patch is required in stable branches as well. How do we best handle
> that?

Oh, I just saw that rilproxy has stable branches as well. So we don't have to backport the init scripts.
Comment on attachment 8549629 [details]
Github pull request for rilproxy

LGTM
Attachment #8549629 - Flags: review?(kyle) → review+

Comment 7

3 years ago
I would prefer if rilproxy automatically checked the number of rild sockets there are and automatically generated the right sockets so we don't have to copy a rilproxy entry for every sim supported. On the other hand, there probably won't be a real phone with more than 3 sim slots, so it's not too painful. I guess this is a question for the RIL owners.

Updated

3 years ago
Attachment #8549629 - Flags: feedback?(mwu)

Comment 8

3 years ago
Comment on attachment 8549636 [details]
Github pull request for device_generic_goldfish on emulator-kk

r=me if this is the direction we want to go in.
Attachment #8549636 - Flags: review?(mwu) → review+

Comment 9

3 years ago
Comment on attachment 8549632 [details]
Github pull request for gonk-misc

r=me with the logwrapper change removed, assuming this is what we want to do.
Attachment #8549632 - Flags: review?(mwu) → review+
Comment on attachment 8549629 [details]
Github pull request for rilproxy

Looks good to me.
Attachment #8549629 - Flags: feedback?(htsai) → feedback+
(In reply to Michael Wu [:mwu] from comment #7)
> I would prefer if rilproxy automatically checked the number of rild sockets
> there are and automatically generated the right sockets so we don't have to
> copy a rilproxy entry for every sim supported. On the other hand, there
> probably won't be a real phone with more than 3 sim slots, so it's not too
> painful. I guess this is a question for the RIL owners.

Hi,
There are two reasons that we eventually chose to have multiple rilproxy as it's now. 
1) Please see Vicamo's 2nd comment: https://bugzilla.mozilla.org/show_bug.cgi?id=814579#c23
2) Bug 825557 : actually the original proposal was as Michael's suggestion. However, we encountered some problems with parsing data coming from rild that causes many ril parcel dropped. Considering the risk of 2) and considering the real phone cases, I think it's fine to live with it.

Updated

3 years ago
Attachment #8549632 - Flags: feedback?(htsai) → feedback+

Updated

3 years ago
Attachment #8549636 - Flags: feedback?(htsai) → feedback+
Comment on attachment 8549632 [details]
Github pull request for gonk-misc

Updated Github pull request

  - removed logwrapper from service command
Thanks everyone!
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/rilproxy/commit/5ef30994f4778b4052e58a4383dbe7890048c87e

Master: https://github.com/mozilla-b2g/gonk-misc/commit/85df3dfd02a5dd84fc1c0558868b1d10ac855243

b2g-4.4.2_r1: https://github.com/mozilla-b2g/device_generic_goldfish/commit/3dc574ecf7fe233f825233c88c08c2452169a36e
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-b2g-master: --- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S4 (23jan)
You need to log in before you can comment on or make changes to this bug.