On Thu, Aug 11, 2016 at 12:05:12PM -0700, Kees Cook wrote:
> On Thu, Aug 11, 2016 at 12:02 AM, Andrei Vagin <avagin@xxxxxxxxxx> wrote:
> > This patch adds /proc/ucounts where all non-zero ucounts for a current
> > userns are shown.
>
> Eric would have a better sense of this, but I think we would normally
> avoid putting something in the top-level /proc directory, especially
> for namespace things (which IIUC usually appear in /proc/$pid/ns/).
/proc/$pid/ns contains links to namespaces handles. I am not sure that
it's a good place for this file.
And I'm agree that it may be a good idea to move this file to
/proc/$pid/. It's like /proc/PID/net.
In thise case we will not need to switch into a user namespace to get
its ucounts.
>
> Also, this should describe what ucounts are for people unfamiliar with
> them. (i.e. this commit message doesn't really contain a detailed
> description of what's being added.)
>
> And finally, any changes to /proc need an associated entry in
> Documentation/filesystems/proc.txt
I will add documentation and a better description for this patch.
Thank you for the comments.
>
> >
> > $ cat /proc/ucounts
> > user: 1000 1
> > net: 1000 1
> > mnt: 1000 2
> > mnt: 0 10
> > cgroup: 0 1
> >
> > There are three columns: type, uid, value.
> >
> > Signed-off-by: Andrei Vagin <avagin@xxxxxxxxxx>
> > ---
> > kernel/ucount.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 121 insertions(+)
> >
> > diff --git a/kernel/ucount.c b/kernel/ucount.c
> > index 9d20d5d..02a7b7f 100644
> > --- a/kernel/ucount.c
> > +++ b/kernel/ucount.c
> > @@ -9,6 +9,8 @@
> > #include <linux/sysctl.h>
> > #include <linux/slab.h>
> > #include <linux/hash.h>
> > +#include <linux/seq_file.h>
> > +#include <linux/proc_fs.h>
> > #include <linux/user_namespace.h>
> >
> > #define UCOUNTS_HASHTABLE_BITS 10
> > @@ -232,4 +234,123 @@ static __init int user_namespace_sysctl_init(void)
> > }
> > subsys_initcall(user_namespace_sysctl_init);
> >
> > +#ifdef CONFIG_PROC_FS
> > +struct ucounts_iterator {
> > + int hash;
> > +};
> > +
> > +static void *ucounts_start(struct seq_file *f, loff_t *pos)
> > +{
> > + struct ucounts_iterator *iter = f->private;
> > + struct user_namespace *ns = current_user_ns();
> > + int h, i = 0;
> > +
> > + spin_lock(&ucounts_lock);
> > + for (h = 0; h < (1 << UCOUNTS_HASHTABLE_BITS); h++) {
> > + struct ucounts *ucounts;
> > +
> > + hlist_for_each_entry(ucounts, &ucounts_hashtable[h], node) {
> > + if (ucounts->ns != ns)
> > + continue;
> > + if (i++ < *pos)
> > + continue;
> > +
> > + iter->hash = h;
> > +
> > + return ucounts;
> > + }
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static void ucounts_stop(struct seq_file *f, void *v)
> > +{
> > + spin_unlock(&ucounts_lock);
> > +}
>
> I don't see where ucounts_lock is defined, but even still, holding a
> spinlock across start/stop will provide a way DoS anyone else using
> that lock. And sparse will yell at you very loudly for the unbalanced
> locking too. :)
>
> > +static void *ucounts_next(struct seq_file *f, void *v, loff_t *pos)
> > +{
> > + struct ucounts_iterator *iter = f->private;
> > + struct user_namespace *ns = current_user_ns();
> > + struct ucounts *ucounts = v;
> > + int h;
> > +
> > + ++*pos;
> > +
> > + for (h = iter->hash; h < (1 << UCOUNTS_HASHTABLE_BITS); h++) {
> > + struct hlist_node *node;
> > +
> > + if (ucounts == NULL) {
> > + node = ucounts_hashtable[h].first;
> > + iter->hash = h;
> > + } else
> > + node = ucounts->node.next;
> > +
> > + ucounts = hlist_entry(node, struct ucounts, node);
> > +
> > + hlist_for_each_entry_from(ucounts, node) {
> > + if (ucounts->ns != ns)
> > + continue;
> > +
> > + return ucounts;
> > + }
> > +
> > + ucounts = NULL;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static int ucounts_show(struct seq_file *f, void *v)
> > +{
> > + static const char * const ns_strs[] = {
> > + "user", "pid", "uts", "ipc",
> > + "net", "mnt", "cgroup", NULL
> > + };
> > + struct user_namespace *ns = current_user_ns();
> > + struct ucounts *ucounts = v;
> > + uid_t uid;
> > + int i;
> > +
> > + uid = from_kuid_munged(ns, ucounts->uid);
> > +
> > + for (i = 0; ns_strs[i]; i++) {
> > + int val = atomic_read(&ucounts->ucount[i]);
> > +
> > + if (val == 0)
> > + continue;
> > +
> > + seq_printf(f, "%s:\t%10u\t%10d\n", ns_strs[i], uid, val);
> > + }
> >
> > + return 0;
> > +}
> > +
> > +static const struct seq_operations ucounts_seq_operations = {
> > + .start = ucounts_start,
> > + .next = ucounts_next,
> > + .stop = ucounts_stop,
> > + .show = ucounts_show,
> > +};
> > +
> > +static int ucounts_open(struct inode *inode, struct file *filp)
> > +{
> > + return seq_open_private(filp, &ucounts_seq_operations,
> > + sizeof(struct ucounts_iterator));
> > +}
> > +
> > +static const struct file_operations proc_ucounts_operations = {
> > + .open = ucounts_open,
> > + .read = seq_read,
> > + .llseek = seq_lseek,
> > + .release = seq_release_private,
> > +};
> > +
> > +static int __init proc_ucounts_init(void)
> > +{
> > + proc_create("ucounts", 0, NULL, &proc_ucounts_operations);
> > + return 0;
> > +}
> > +fs_initcall(proc_ucounts_init);
> > +#endif /* CONFIG_PROC_FS */
> > --
> > 2.5.5
> >
>
> -Kees
>
> --
> Kees Cook
> Nexus Security
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/containers
