From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161264AbcFQPWz (ORCPT ); Fri, 17 Jun 2016 11:22:55 -0400 Received: from h2.hallyn.com ([78.46.35.8]:58616 "EHLO h2.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754470AbcFQPWx (ORCPT ); Fri, 17 Jun 2016 11:22:53 -0400 Date: Fri, 17 Jun 2016 10:22:46 -0500 From: "Serge E. Hallyn" To: Topi Miettinen Cc: linux-kernel@vger.kernel.org, James Morris , "Serge E. Hallyn" , "open list:SECURITY SUBSYSTEM" Subject: Re: [RFC 04/18] device_cgroup: track and present accessed devices Message-ID: <20160617152246.GA2349@mail.hallyn.com> References: <1465847065-3577-1-git-send-email-toiwoton@gmail.com> <1465847065-3577-5-git-send-email-toiwoton@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1465847065-3577-5-git-send-email-toiwoton@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Quoting Topi Miettinen (toiwoton@gmail.com): > Track what devices are accessed and present them cgroup devices.accessed. > > Signed-off-by: Topi Miettinen > --- > security/device_cgroup.c | 70 +++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 60 insertions(+), 10 deletions(-) > > diff --git a/security/device_cgroup.c b/security/device_cgroup.c > index 03c1652..45aa730 100644 > --- a/security/device_cgroup.c > +++ b/security/device_cgroup.c > @@ -48,6 +48,7 @@ struct dev_exception_item { > struct dev_cgroup { > struct cgroup_subsys_state css; > struct list_head exceptions; > + struct list_head accessed; > enum devcg_behavior behavior; > }; > > @@ -90,7 +91,7 @@ free_and_exit: > /* > * called under devcgroup_mutex > */ > -static int dev_exception_add(struct dev_cgroup *dev_cgroup, > +static int dev_exception_add(struct list_head *exceptions, > struct dev_exception_item *ex) If you're going to re-use this function for the accessed list, then it should be renamed, bc as it is it's misleading. It also should be restructured. The add-exceptions case was rare, so doing kmemdup before checking for duplicates was ok. But for the accessed list I think we want to check for duplicates before we kmemdup. > { > struct dev_exception_item *excopy, *walk; > @@ -101,7 +102,7 @@ static int dev_exception_add(struct dev_cgroup *dev_cgroup, > if (!excopy) > return -ENOMEM; > > - list_for_each_entry(walk, &dev_cgroup->exceptions, list) { > + list_for_each_entry(walk, exceptions, list) { > if (walk->type != ex->type) > continue; > if (walk->major != ex->major) > @@ -115,7 +116,7 @@ static int dev_exception_add(struct dev_cgroup *dev_cgroup, > } > > if (excopy != NULL) > - list_add_tail_rcu(&excopy->list, &dev_cgroup->exceptions); > + list_add_tail_rcu(&excopy->list, exceptions); > return 0; > } > > @@ -155,6 +156,16 @@ static void __dev_exception_clean(struct dev_cgroup *dev_cgroup) > } > } > > +static void dev_accessed_clean(struct dev_cgroup *dev_cgroup) > +{ > + struct dev_exception_item *ex, *tmp; > + > + list_for_each_entry_safe(ex, tmp, &dev_cgroup->accessed, list) { > + list_del_rcu(&ex->list); > + kfree_rcu(ex, rcu); > + } > +} > + > /** > * dev_exception_clean - frees all entries of the exception list > * @dev_cgroup: dev_cgroup with the exception list to be cleaned > @@ -221,6 +232,7 @@ devcgroup_css_alloc(struct cgroup_subsys_state *parent_css) > if (!dev_cgroup) > return ERR_PTR(-ENOMEM); > INIT_LIST_HEAD(&dev_cgroup->exceptions); > + INIT_LIST_HEAD(&dev_cgroup->accessed); > dev_cgroup->behavior = DEVCG_DEFAULT_NONE; > > return &dev_cgroup->css; > @@ -231,6 +243,7 @@ static void devcgroup_css_free(struct cgroup_subsys_state *css) > struct dev_cgroup *dev_cgroup = css_to_devcgroup(css); > > __dev_exception_clean(dev_cgroup); > + dev_accessed_clean(dev_cgroup); > kfree(dev_cgroup); > } > > @@ -272,9 +285,9 @@ static void set_majmin(char *str, unsigned m) > sprintf(str, "%u", m); > } > > -static int devcgroup_seq_show(struct seq_file *m, void *v) > +static int devcgroup_seq_show_list(struct seq_file *m, struct dev_cgroup *devcgroup, > + struct list_head *exceptions, bool allow) > { > - struct dev_cgroup *devcgroup = css_to_devcgroup(seq_css(m)); > struct dev_exception_item *ex; > char maj[MAJMINLEN], min[MAJMINLEN], acc[ACCLEN]; > > @@ -285,14 +298,14 @@ static int devcgroup_seq_show(struct seq_file *m, void *v) > * - List the exceptions in case the default policy is to deny > * This way, the file remains as a "whitelist of devices" > */ > - if (devcgroup->behavior == DEVCG_DEFAULT_ALLOW) { > + if (allow) { > set_access(acc, ACC_MASK); > set_majmin(maj, ~0); > set_majmin(min, ~0); > seq_printf(m, "%c %s:%s %s\n", type_to_char(DEV_ALL), > maj, min, acc); > } else { > - list_for_each_entry_rcu(ex, &devcgroup->exceptions, list) { > + list_for_each_entry_rcu(ex, exceptions, list) { > set_access(acc, ex->access); > set_majmin(maj, ex->major); > set_majmin(min, ex->minor); > @@ -305,6 +318,36 @@ static int devcgroup_seq_show(struct seq_file *m, void *v) > return 0; > } > > +static int devcgroup_seq_show(struct seq_file *m, void *v) > +{ > + struct dev_cgroup *devcgroup = css_to_devcgroup(seq_css(m)); > + > + return devcgroup_seq_show_list(m, devcgroup, &devcgroup->exceptions, > + devcgroup->behavior == DEVCG_DEFAULT_ALLOW); > +} > + > +static int devcgroup_seq_show_accessed(struct seq_file *m, void *v) > +{ > + struct dev_cgroup *devcgroup = css_to_devcgroup(seq_css(m)); > + > + return devcgroup_seq_show_list(m, devcgroup, &devcgroup->accessed, false); > +} > + > +static void devcgroup_add_accessed(struct dev_cgroup *dev_cgroup, short type, > + u32 major, u32 minor, short access) > +{ > + struct dev_exception_item ex; > + > + ex.type = type; > + ex.major = major; > + ex.minor = minor; > + ex.access = access; > + > + mutex_lock(&devcgroup_mutex); > + dev_exception_add(&dev_cgroup->accessed, &ex); > + mutex_unlock(&devcgroup_mutex); > +} > + > /** > * match_exception - iterates the exception list trying to find a complete match > * @exceptions: list of exceptions > @@ -566,7 +609,7 @@ static int propagate_exception(struct dev_cgroup *devcg_root, > */ > if (devcg_root->behavior == DEVCG_DEFAULT_ALLOW && > devcg->behavior == DEVCG_DEFAULT_ALLOW) { > - rc = dev_exception_add(devcg, ex); > + rc = dev_exception_add(&devcg->exceptions, ex); > if (rc) > break; > } else { > @@ -736,7 +779,7 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, > > if (!parent_has_perm(devcgroup, &ex)) > return -EPERM; > - rc = dev_exception_add(devcgroup, &ex); > + rc = dev_exception_add(&devcgroup->exceptions, &ex); > break; > case DEVCG_DENY: > /* > @@ -747,7 +790,7 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, > if (devcgroup->behavior == DEVCG_DEFAULT_DENY) > dev_exception_rm(devcgroup, &ex); > else > - rc = dev_exception_add(devcgroup, &ex); > + rc = dev_exception_add(&devcgroup->exceptions, &ex); > > if (rc) > break; > @@ -788,6 +831,11 @@ static struct cftype dev_cgroup_files[] = { > .seq_show = devcgroup_seq_show, > .private = DEVCG_LIST, > }, > + { > + .name = "accessed", > + .seq_show = devcgroup_seq_show_accessed, > + .private = DEVCG_LIST, > + }, > { } /* terminate */ > }; > > @@ -830,6 +878,8 @@ static int __devcgroup_check_permission(short type, u32 major, u32 minor, > if (!rc) > return -EPERM; > > + devcgroup_add_accessed(dev_cgroup, type, major, minor, access); > + > return 0; > } > > -- > 2.8.1