Code review comment for lp:~zhangew401/usensord/fix-lp-1433590

Revision history for this message
Tyler Hicks (tyhicks) wrote :

I don't love the approach of reading /proc/PID/exe and I have some real blockers that I mention inline below.

Since your entire goal is to identify maliit-server, I'd much prefer that you confine maliit-server with an AppArmor profile (it could even be loosely confined at first). You'd then call GetConnectionCredentials() and use the LinuxSecurityLabel to identify maliit-server by its AppArmor profile name. This approach has the benefit that we confine another process on the system (more hardening) and don't have to rely on oddities in the behavior of /proc/PID/exe (see proc(5) man page for an example seen in child threads).

Confining maliit-server could be easy or it could become more difficult if it requires changes to other AppArmor profiles. I can't really say at this point but I'd like for this approach to at least be considered. If it is too complex then you should at least address the issues I mention inline below.

Thanks for working through this merge feedback! :)

review: Needs Fixing

« Back to merge proposal