1121986 - Run rilproxy with non-root privileges

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Thomas Zimmermann [:tzimmermann] [:tdz]]

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.

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: 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.

Comment 2

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: Github pull request for gonk-misc

This patch is required in stable branches as well. How do we best handle that?

Comment 3

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: Github pull request for device_generic_goldfish on emulator-kk

Comment 4

[Thomas Zimmermann [:tzimmermann] [:tdz]]

There seems to be a similar bug 812394, but don't have permission to see it.

Comment 5

[Thomas Zimmermann [:tzimmermann] [:tdz]]

(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 6

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8549629 [details]
Github pull request for rilproxy

LGTM

Comment 7

[Michael Wu [:mwu]]

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.

Comment 8

[Michael Wu [:mwu]]

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.

Comment 9

[Michael Wu [:mwu]]

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.

Comment 10

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8549629 [details]
Github pull request for rilproxy

Looks good to me.

Comment 11

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(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.

Comment 12

[Thomas Zimmermann [:tzimmermann] [:tdz]]

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

Updated Github pull request

  - removed logwrapper from service command

Comment 13

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Thanks everyone!

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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