Re: [PATCH v22 12/23] LSM: Specify which LSM to display

On 11/7/2020 2:05 PM, John Johansen wrote: > On 11/7/20 1:15 AM, Greg KH wrote: >> On Fri, Nov 06, 2020 at 04:20:43PM -0800, Casey Schaufler wrote: >>> On 11/5/2020 1:22 AM, Greg KH wrote: >>>> On Wed, Nov 04, 2020 at 03:41:03PM -0800, Casey Schaufler wrote: >>>>> Create a new entry "display" in the procfs attr directory for >>>>> controlling which LSM security information is displayed for a >>>>> process. A process can only read or write its own display value. >>>>> >>>>> The name of an active LSM that supplies hooks for >>>>> human readable data may be written to "display" to set the >>>>> value. The name of the LSM currently in use can be read from >>>>> "display". At this point there can only be one LSM capable >>>>> of display active. A helper function lsm_task_display() is >>>>> provided to get the display slot for a task_struct. >>>>> >>>>> Setting the "display" requires that all security modules using >>>>> setprocattr hooks allow the action. Each security module is >>>>> responsible for defining its policy. >>>>> >>>>> AppArmor hook provided by John Johansen <john.johansen(a)canonical.com&gt; >>>>> SELinux hook provided by Stephen Smalley <sds(a)tycho.nsa.gov&gt; >>>>> >>>>> Reviewed-by: Kees Cook <keescook(a)chromium.org&gt; >>>>> Acked-by: Stephen Smalley <sds(a)tycho.nsa.gov&gt; >>>>> Acked-by: Paul Moore <paul(a)paul-moore.com&gt; >>>>> Signed-off-by: Casey Schaufler <casey(a)schaufler-ca.com&gt; >>>>> Cc: linux-api(a)vger.kernel.org >>>>> --- >>>>> fs/proc/base.c | 1 + >>>>> include/linux/lsm_hooks.h | 17 +++ >>>>> security/apparmor/include/apparmor.h | 3 +- >>>>> security/apparmor/lsm.c | 32 +++++ >>>>> security/security.c | 169 ++++++++++++++++++++++++--- >>>>> security/selinux/hooks.c | 11 ++ >>>>> security/selinux/include/classmap.h | 2 +- >>>>> security/smack/smack_lsm.c | 7 ++ >>>>> 8 files changed, 223 insertions(+), 19 deletions(-) >>>>> >>>>> diff --git a/fs/proc/base.c b/fs/proc/base.c >>>>> index 0f707003dda5..7432f24f0132 100644 >>>>> --- a/fs/proc/base.c >>>>> +++ b/fs/proc/base.c >>>>> @@ -2806,6 +2806,7 @@ static const struct pid_entry attr_dir_stuff[] = { >>>>> ATTR(NULL, "fscreate", 0666), >>>>> ATTR(NULL, "keycreate", 0666), >>>>> ATTR(NULL, "sockcreate", 0666), >>>>> + ATTR(NULL, "display", 0666), >>>> That's a vague name, any chance it can be more descriptive? >>> Sure. How about lsm_display, or display_lsm? I wouldn't say that >>> any of the files in /proc/*/attr have especially descriptive names, >>> but that's hardly an excuse. >> I still don't understand what "display" means in this context. Perhaps > its the LSM thats context is being displayed on the shared interface, > ie. /proc/*/attr/* > > thinking about it more owner or even interface_owner might be a better > name I was hoping for a single word, but I see how something more descriptive might be in order. How about "lsm_of_current"? Or "lsm_of_dot_slash_current", if you want to be pedantic. "format_of_current" isn't quite accurate, but might be easier for some people to understand. Maybe "interface_owning_lsm". /proc/*/attr/display answers the question "Which LSM is providing the data I see if I look in /proc/*/attr/current, prev or exec or if that process uses SO_PEERSEC".

>> documentation will help clear it up? >> > yeah this needs documented. Agreed. I've noticed that nothing in /proc/*/attr seems documented in an orderly (documentation/ABI) fashion. I will have to fix some of that for a description of /proc/*/attr/whatever_it_ends_up_getting_called to make sense. Working on it. >> thanks, >> >> greg k-h >>