From: "Serge E. Hallyn" <serge@hallyn.com>
To: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: "Serge E. Hallyn" <serge@hallyn.com>,
Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>,
ima-devel <linux-ima-devel@lists.sourceforge.net>,
containers <containers@lists.linux-foundation.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
linux-security-module <linux-security-module@vger.kernel.org>,
Tycho Andersen <tycho@docker.com>,
Yuqiong Sun <sunyuqiong1988@gmail.com>,
David Safford <david.safford@ge.com>,
Mehmet Kayaalp <mkayaalp@cs.binghamton.edu>,
Stefan Berger <stefanb@linux.vnet.ibm.com>
Subject: Re: [RFC PATCH 2/5] ima: Add ns_status for storing namespaced iint data
Date: Tue, 25 Jul 2017 15:49:42 -0500 [thread overview]
Message-ID: <20170725204942.GC4969@mail.hallyn.com> (raw)
In-Reply-To: <1501013725.27413.27.camel@linux.vnet.ibm.com>
On Tue, Jul 25, 2017 at 04:15:25PM -0400, Mimi Zohar wrote:
> On Tue, 2017-07-25 at 14:43 -0500, Serge E. Hallyn wrote:
> > ...
> > > +static void free_ns_status_cache(struct ima_namespace *ns)
> > > +{
> > > + struct ns_status *status, *next;
> > > +
> > > + write_lock(&ns->ns_status_lock);
> > > + rbtree_postorder_for_each_entry_safe(status, next,
> > > + &ns->ns_status_tree, rb_node)
> > > + kmem_cache_free(ns->ns_status_cache, status);
> > > + ns->ns_status_tree = RB_ROOT;
> > > + write_unlock(&ns->ns_status_lock);
> > > + kmem_cache_destroy(ns->ns_status_cache);
> > > +}
> > > +
> > > static void destroy_ima_ns(struct ima_namespace *ns)
> > > {
> > > put_user_ns(ns->user_ns);
> > > ns_free_inum(&ns->ns);
> > > + free_ns_status_cache(ns);
> > > kfree(ns);
> > > }
> > >
> > > @@ -181,3 +198,106 @@ struct ima_namespace init_ima_ns = {
> > > .parent = NULL,
> > > };
> > > EXPORT_SYMBOL(init_ima_ns);
> > > +
> > > +/*
> > > + * __ima_ns_status_find - return the ns_status associated with an inode
> > > + */
> > > +static struct ns_status *__ima_ns_status_find(struct ima_namespace *ns,
> > > + struct inode *inode)
> > > +{
> > > + struct ns_status *status;
> > > + struct rb_node *n = ns->ns_status_tree.rb_node;
> > > +
> > > + while (n) {
> > > + status = rb_entry(n, struct ns_status, rb_node);
> > > +
> > > + if (inode < status->inode)
> > > + n = n->rb_left;
> > > + else if (inode->i_ino > status->i_ino)
> > > + n = n->rb_right;
> > > + else
> > > + break;
> > > + }
> > > + if (!n)
> > > + return NULL;
> > > +
> > > + return status;
> > > +}
> > > +
> > > +/*
> > > + * ima_ns_status_find - return the ns_status associated with an inode
> > > + */
> > > +static struct ns_status *ima_ns_status_find(struct ima_namespace *ns,
> > > + struct inode *inode)
> > > +{
> > > + struct ns_status *status;
> > > +
> > > + read_lock(&ns->ns_status_lock);
> > > + status = __ima_ns_status_find(ns, inode);
> > > + read_unlock(&ns->ns_status_lock);
> > > +
> > > + return status;
> > > +}
> > ...
> > > +
> > > +struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
> > > + struct inode *inode)
> > > +{
> > > + struct ns_status *status;
> > > + int skip_insert = 0;
> > > +
> > > + status = ima_ns_status_find(ns, inode);
> > > + if (status) {
> > > + /*
> > > + * Unlike integrity_iint_cache we are not free'ing the
> > > + * ns_status data when the inode is free'd. So, in addition to
> > > + * checking the inode pointer, we need to make sure the
> > > + * (i_generation, i_ino) pair matches as well. In the future
> > > + * we might want to add support for lazily walking the rbtree
> > > + * to clean it up.
> > > + */
> > > + if (inode->i_ino == status->i_ino &&
> > > + inode->i_generation == status->i_generation)
> > > + return status;
> > > +
> > > + /* Same inode number is reused, overwrite the ns_status */
> > > + skip_insert = 1;
> > > + } else {
> > > + status = kmem_cache_alloc(ns->ns_status_cache, GFP_NOFS);
> > > + if (!status)
> > > + return ERR_PTR(-ENOMEM);
> > > + }
> >
> > What prevents the status from being freed between the read_lock
> > in ima_ns_status_find() and the write_lock in the following line?
> >
> > IIUC it's that ns is always current's ima_ns, which will pin the ns
> > and cause no statuses to be freed. But then the ns should probably
> > not be passed in here? Or a comment should say that ns must be
> > pinned?
> >
> > Just trying to make sure I understand the locking.
>
> iint's are only freed after the last reference to the inode is deleted
> in __fput(). Refer to ima_file_free(). ns_status is a bit different
> in that they are freed on namespace cleanup.
Ok, thanks - that sounds ok then.
next prev parent reply other threads:[~2017-07-25 20:49 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-20 22:50 [RFC PATCH 0/5] ima: namespacing IMA audit messages Mehmet Kayaalp
2017-07-20 22:50 ` [RFC PATCH 1/5] ima: extend clone() with IMA namespace support Mehmet Kayaalp
2017-07-25 17:53 ` Serge E. Hallyn
2017-07-25 18:49 ` James Bottomley
2017-07-25 19:04 ` Serge E. Hallyn
2017-07-25 19:08 ` James Bottomley
2017-07-25 19:48 ` Mimi Zohar
2017-07-25 20:11 ` Stefan Berger
2017-07-25 20:46 ` Serge E. Hallyn
2017-07-25 20:57 ` Mimi Zohar
2017-07-25 21:08 ` Serge E. Hallyn
2017-07-25 21:28 ` Mimi Zohar
2017-07-27 12:51 ` [Linux-ima-devel] " Magalhaes, Guilherme (Brazil R&D-CL)
2017-07-27 14:39 ` Mimi Zohar
2017-07-27 17:18 ` Magalhaes, Guilherme (Brazil R&D-CL)
2017-07-27 17:49 ` Stefan Berger
2017-07-27 19:39 ` Magalhaes, Guilherme (Brazil R&D-CL)
2017-07-27 20:51 ` Stefan Berger
2017-07-28 14:19 ` Magalhaes, Guilherme (Brazil R&D-CL)
2017-07-31 11:31 ` Mimi Zohar
2017-07-25 21:35 ` Stefan Berger
2018-03-08 14:04 ` Stefan Berger
2018-03-09 2:59 ` Serge E. Hallyn
2018-03-09 13:52 ` Stefan Berger
2018-03-11 22:58 ` James Morris
2018-03-13 18:02 ` Stefan Berger
2018-03-13 21:51 ` James Morris
2017-07-25 20:31 ` James Bottomley
2017-07-25 20:47 ` Mimi Zohar
2018-03-08 13:39 ` Stefan Berger
2018-03-08 20:19 ` Serge E. Hallyn
[not found] ` <a6ef5679-6aef-21de-7cdb-48e8af83f874@linux.vnet.ibm.com>
2018-03-08 23:31 ` Serge E. Hallyn
2017-07-20 22:50 ` [RFC PATCH 2/5] ima: Add ns_status for storing namespaced iint data Mehmet Kayaalp
2017-07-25 19:43 ` Serge E. Hallyn
2017-07-25 20:15 ` Mimi Zohar
2017-07-25 20:25 ` Stefan Berger
2017-07-25 20:49 ` Serge E. Hallyn [this message]
2017-08-11 15:00 ` Stefan Berger
2017-07-20 22:50 ` [RFC PATCH 3/5] ima: mamespace audit status flags Mehmet Kayaalp
2017-08-01 17:17 ` Tycho Andersen
2017-08-01 17:25 ` Mehmet Kayaalp
2017-08-02 21:48 ` Tycho Andersen
2017-07-20 22:50 ` [RFC PATCH 4/5] ima: differentiate auditing policy rules from "audit" actions Mehmet Kayaalp
2017-07-20 22:50 ` [RFC PATCH 5/5] ima: Add ns_mnt, dev, ino fields to IMA audit measurement msgs Mehmet Kayaalp
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170725204942.GC4969@mail.hallyn.com \
--to=serge@hallyn.com \
--cc=containers@lists.linux-foundation.org \
--cc=david.safford@ge.com \
--cc=linux-ima-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mkayaalp@cs.binghamton.edu \
--cc=mkayaalp@linux.vnet.ibm.com \
--cc=stefanb@linux.vnet.ibm.com \
--cc=sunyuqiong1988@gmail.com \
--cc=tycho@docker.com \
--cc=zohar@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).