GNOME Bugzilla – Bug 150020
Add ability to display SELinux security context for each process (where available)
Last modified: 2011-11-11 10:03:55 UTC
If the system is running SELinux, each process has a "security context", which can be displayed at the command line in ps using the -Z option. GNOME's system-monitor should be able to display it as well, as an optional field in the preferences dialog (probably off by default).
Created attachment 30804 [details] [review] First pass at adding SELinux support This patch adds a new column to the process view, the selinux security context. It adds a --enable-selinux configuration option. It adds a new string ("Security Context") Screenshot available here: http://people.redhat.com/dmalcolm/Screenshot-System%20Monitor.png If the kernel doesn't support SELinux, or the configuration option is not enabled, the column is hidden. The column is always present, to deal with problems where the GConf column settings persist from another session where you booted the kernel with a different selinux setting.
There are some unnecessary whitespace changes in there. Maybe those could be removed. But other than that it looks sane to me at least. Could the column just be greyed out if it's not running on a SELinux enabled kernel maybe? So that people know it's there but not enabled?
+/* Move this somehow to glibtop ? */ selinux means Linux. If you know something similar on other systems, this could be added to libgtop. We should discuss this by email. Btw, i had bad experiences with #ifdef in .c, following the Linux Kernel style is good : http://www.linuxjournal.com/article.php?sid=5780 -> "No ifdefs in .c Code" I think it would be nice to provide a fallback saying something like "Not Available" in the column instead of hiding it, no ?
Created attachment 33152 [details] [review] 2nd pass at the patch
I believe I've removed the whitespace changes. I removed the glibtop comment, since I think this is very much Linux-specific. I've slightly reorganised the #ifdef code so it's more gathered near the top of the file, rather than being scattered throughout. I'm not quite sure about what to do about the column when SELinux is disabled/unavailable. A problem is that column visibility is stored in GConf, and I regularly share an NFS-mounted home directory between a variety of machines with and without SELinux support (and reboot with kernel parameters to enable/disable support as well). Hence I favour an approach where the column is programatically hidden (along with its entry in the preferences dialog) where it's unavailable, ideally without affecting the GConf values. Does this sound like a good approach to the maintainers? (I've kept the existence of the column in the enum even when you have selinux support disabled at configure-time so that we have a consistent meaning of the numbered GConf keys for packages built with and without the support)
all of this looks pretty OK to me. I'm afraid i have to live with #ifdef in .c (still have nightmare about it, think about a function in .c file handling a dozen of nested & mixed #ifdef ...). Your patch looks much cleaner to me. I've still not branched, and i'll be cleaning 2.8 until 2.8.2. After 2.8.2, i'll branch and apply. Then i'll close the bug. Get back to me if i forget. Thank you very much.
Merged in HEAD Good job. Thank you very much.