Re: kdbus: add code to gather metadata

[Andy Lutomirski]

On Wed, Oct 29, 2014 at 3:00 PM, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> From: Daniel Mack <daniel@xxxxxxxxxx>
>
> A connection chooses which metadata it wants to have attached to each
> message it receives with kdbus_cmd_hello.attach_flags. The metadata
> will be attached as items to the messages. All metadata refers to
> information about the sending task at sending time, unless otherwise
> stated. Also, the metadata is copied, not referenced, so even if the
> sending task doesn't exist anymore at the time the message is received,
> the information is still preserved.
>
> See kdbus.txt for more details on which metadata can currently be
> attached to messages.
>
> Signed-off-by: Daniel Mack <daniel@xxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/misc/kdbus/metadata.c | 626 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/misc/kdbus/metadata.h |  51 ++++
>  2 files changed, 677 insertions(+)
>  create mode 100644 drivers/misc/kdbus/metadata.c
>  create mode 100644 drivers/misc/kdbus/metadata.h
>
> diff --git a/drivers/misc/kdbus/metadata.c b/drivers/misc/kdbus/metadata.c
> new file mode 100644
> index 000000000000..8323e6d7a071
> --- /dev/null
> +++ b/drivers/misc/kdbus/metadata.c
> @@ -0,0 +1,626 @@
> +/*
> + * Copyright (C) 2013-2014 Kay Sievers
> + * Copyright (C) 2013-2014 Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> + * Copyright (C) 2013-2014 Daniel Mack <daniel@xxxxxxxxxx>
> + * Copyright (C) 2013-2014 David Herrmann <dh.herrmann@xxxxxxxxx>
> + * Copyright (C) 2013-2014 Linux Foundation
> + *
> + * kdbus is free software; you can redistribute it and/or modify it under
> + * the terms of the GNU Lesser General Public License as published by the
> + * Free Software Foundation; either version 2.1 of the License, or (at
> + * your option) any later version.
> + */
> +
> +#include <linux/audit.h>
> +#include <linux/capability.h>
> +#include <linux/cgroup.h>
> +#include <linux/cred.h>
> +#include <linux/file.h>
> +#include <linux/init.h>
> +#include <linux/mutex.h>
> +#include <linux/pid_namespace.h>
> +#include <linux/sched.h>
> +#include <linux/security.h>
> +#include <linux/sizes.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <linux/user_namespace.h>
> +#include <linux/version.h>
> +
> +#include "connection.h"
> +#include "item.h"
> +#include "message.h"
> +#include "metadata.h"
> +#include "names.h"
> +
> +/**
> + * kdbus_meta_new() - create new metadata object
> + * @meta:              New metadata object
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int kdbus_meta_new(struct kdbus_meta **meta)
> +{
> +       struct kdbus_meta *m;
> +
> +       BUG_ON(*meta);
> +
> +       m = kzalloc(sizeof(*m), GFP_KERNEL);
> +       if (!m)
> +               return -ENOMEM;
> +
> +       /*
> +        * Remember the PID and user namespaces our credentials belong to;
> +        * we need to prevent leaking authorization and security-relevant
> +        * data across different namespaces.
> +        */
> +       m->pid_namespace = get_pid_ns(task_active_pid_ns(current));
> +       m->user_namespace = get_user_ns(current_user_ns());
> +

This is unusual, and it could be very expensive (it will serialize
essentially everyone on an exclusive cacheline).  What attack is it
protecting against?

> +static int kdbus_meta_append_cred(struct kdbus_meta *meta)
> +{
> +       struct kdbus_creds creds = {
> +               .uid = from_kuid_munged(current_user_ns(), current_uid()),
> +               .gid = from_kgid_munged(current_user_ns(), current_gid()),
> +               .pid = task_pid_vnr(current),
> +               .tid = task_tgid_vnr(current),
> +               .starttime = current->start_time,
> +       };
> +
> +       return kdbus_meta_append_data(meta, KDBUS_ITEM_CREDS,
> +                                     &creds, sizeof(creds));
> +}

This seems wrong to me.  Shouldn't this store kuid_t, etc. directly?
Also, why pid, tid, and starttime?

> +
> +       for (i = 0; i < info->ngroups; i++)
> +               gid[i] = from_kgid_munged(current_user_ns(), GROUP_AT(info, i));

Ditto.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/