* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
From: Lukasz Pawelczyk @ 2015-07-08 11:09 UTC (permalink / raw)
To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
Serge E. Hallyn, Kees Cook, Tetsuo Handa, Stephen Smalley,
Neil Brown, Mark Rustad, Greg Kroah-Hartman, Daniel Mack,
David Herrmann, Djalal Harouni, Shuah Khan, Al Viro,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
Cc: Karol Lewandowski, Lukasz Skalski
In-Reply-To: <1436351110-5902-6-git-send-email-p.osmialowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On śro, 2015-07-08 at 12:25 +0200, Paul Osmialowski wrote:
> Originates from:
>
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd
> -v212)
> commit: aa0885489d19be92fa41c6f0a71df28763228a40
>
> Signed-off-by: Karol Lewandowski <k.lewandowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Paul Osmialowski <p.osmialowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> ipc/kdbus/bus.c | 12 ++++++++++-
> ipc/kdbus/bus.h | 3 +++
> ipc/kdbus/connection.c | 54
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> ipc/kdbus/connection.h | 4 ++++
> ipc/kdbus/domain.c | 9 ++++++++-
> ipc/kdbus/domain.h | 2 ++
> ipc/kdbus/endpoint.c | 11 ++++++++++
> ipc/kdbus/names.c | 11 ++++++++++
> ipc/kdbus/queue.c | 30 ++++++++++++++++++----------
> 9 files changed, 124 insertions(+), 12 deletions(-)
>
> diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
> index bbdf0f2..9894895 100644
> --- a/ipc/kdbus/bus.c
> +++ b/ipc/kdbus/bus.c
> @@ -22,6 +22,7 @@
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "notify.h"
> @@ -51,6 +52,7 @@ static void kdbus_bus_free(struct kdbus_node *node)
> kdbus_domain_unref(bus->domain);
> kdbus_policy_db_clear(&bus->policy_db);
> kdbus_meta_proc_unref(bus->creator_meta);
> + security_kdbus_bus_free(bus);
> kfree(bus);
> }
>
> @@ -161,6 +163,12 @@ static struct kdbus_bus *kdbus_bus_new(struct
> kdbus_domain *domain,
> goto exit_unref;
> }
>
> + ret = security_kdbus_bus_alloc(b);
> + if (ret) {
> + ret = -EPERM;
> + goto exit_unref;
> + }
Also, why aren't the error codes passed from LSM? LSM can return
anything, from EINVAL, to ENOMEM. Returning EPERM can be misleading.
> +
> /*
> * Bus-limits of the creator are accounted on its real UID,
> just like
> * all other per-user limits.
> @@ -169,11 +177,13 @@ static struct kdbus_bus *kdbus_bus_new(struct
> kdbus_domain *domain,
> if (IS_ERR(b->creator)) {
> ret = PTR_ERR(b->creator);
> b->creator = NULL;
> - goto exit_unref;
> + goto exit_free_security;
> }
>
> return b;
>
> +exit_free_security:
> + security_kdbus_bus_free(b);
> exit_unref:
> kdbus_node_deactivate(&b->node);
> kdbus_node_unref(&b->node);
> diff --git a/ipc/kdbus/bus.h b/ipc/kdbus/bus.h
> index 5bea5ef..03e4a54 100644
> --- a/ipc/kdbus/bus.h
> +++ b/ipc/kdbus/bus.h
> @@ -53,6 +53,7 @@ struct kdbus_user;
> * @notify_list: List of pending kernel-generated messages
> * @notify_lock: Notification list lock
> * @notify_flush_lock: Notification flushing lock
> + * @security: LSM security blob
> */
> struct kdbus_bus {
> struct kdbus_node node;
> @@ -81,6 +82,8 @@ struct kdbus_bus {
> struct list_head notify_list;
> spinlock_t notify_lock;
> struct mutex notify_flush_lock;
> +
> + void *security;
> };
>
> struct kdbus_bus *kdbus_bus_ref(struct kdbus_bus *bus);
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index 9993753..b85cdc7 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -31,6 +31,7 @@
> #include <linux/slab.h>
> #include <linux/syscalls.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "connection.h"
> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct
> kdbus_ep *ep, bool privileged,
> bool is_activator;
> bool is_monitor;
> struct kvec kvec;
> + u32 sid, len;
> + char *label;
> int ret;
>
> struct {
> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct
> kdbus_ep *ep, bool privileged,
> }
> }
>
> + security_task_getsecid(current, &sid);
> + security_secid_to_secctx(sid, &label, &len);
> + ret = security_kdbus_connect(conn, label, len);
> + if (ret) {
> + ret = -EPERM;
> + goto exit_unref;
> + }
> +
> if (atomic_inc_return(&conn->user->connections) >
> KDBUS_USER_MAX_CONN) {
> /* decremented by destructor as conn->user is valid
> */
> ret = -EMFILE;
> @@ -276,6 +287,7 @@ static void __kdbus_conn_free(struct kref *kref)
> kdbus_pool_free(conn->pool);
> kdbus_ep_unref(conn->ep);
> put_cred(conn->cred);
> + security_kdbus_conn_free(conn);
> kfree(conn->description);
> kfree(conn->quota);
> kfree(conn);
> @@ -1107,6 +1119,12 @@ static int kdbus_conn_reply(struct kdbus_conn
> *src, struct kdbus_kmsg *kmsg)
> if (ret < 0)
> goto exit;
>
> + ret = security_kdbus_talk(src, dst);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> mutex_lock(&dst->lock);
> reply = kdbus_reply_find(src, dst, kmsg->msg.cookie_reply);
> if (reply) {
> @@ -1187,6 +1205,12 @@ static struct kdbus_reply
> *kdbus_conn_call(struct kdbus_conn *src,
> if (ret < 0)
> goto exit;
>
> + ret = security_kdbus_talk(src, dst);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> if (!kdbus_conn_policy_talk(src, current_cred(), dst)) {
> ret = -EPERM;
> goto exit;
> @@ -1248,6 +1272,12 @@ static int kdbus_conn_unicast(struct
> kdbus_conn *src, struct kdbus_kmsg *kmsg)
> if (ret < 0)
> goto exit;
>
> + ret = security_kdbus_talk(src, dst);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> if (is_signal) {
> /* like broadcasts we eavesdrop even if the msg is
> dropped */
> kdbus_bus_eavesdrop(bus, src, kmsg);
> @@ -1639,6 +1669,12 @@ struct kdbus_conn *kdbus_cmd_hello(struct
> kdbus_ep *ep, bool privileged,
> if (ret < 0)
> goto exit;
>
> + ret = security_kdbus_ep_setpolicy(c->ep->bus);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> ret = kdbus_policy_set(&c->ep->bus->policy_db,
> args.items,
> args.items_size, 1,
>
> kdbus_conn_is_policy_holder(c), c);
> @@ -1732,6 +1768,10 @@ int kdbus_cmd_conn_info(struct kdbus_conn
> *conn, void __user *argp)
> if (ret != 0)
> return ret;
>
> + ret = security_kdbus_conn_info(conn);
> + if (ret)
> + return -EPERM;
> +
> /* registry must be held throughout lookup *and* collecting
> data */
> down_read(&bus->name_registry->rwlock);
>
> @@ -1905,6 +1945,12 @@ int kdbus_cmd_update(struct kdbus_conn *conn,
> void __user *argp)
> /* now that we verified the input, update the connection */
>
> if (item_policy) {
> + ret = security_kdbus_ep_setpolicy(conn->ep->bus);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> ret = kdbus_policy_set(&conn->ep->bus->policy_db,
> cmd->items,
> KDBUS_ITEMS_SIZE(cmd, items),
> 1, true, conn);
> @@ -1948,6 +1994,10 @@ int kdbus_cmd_send(struct kdbus_conn *conn,
> struct file *f, void __user *argp)
> .argc = ARRAY_SIZE(argv),
> };
>
> + ret = security_kdbus_send(conn, conn->ep->bus);
> + if (ret)
> + return -EPERM;
> +
> if (!kdbus_conn_is_ordinary(conn))
> return -EOPNOTSUPP;
>
> @@ -2044,6 +2094,10 @@ int kdbus_cmd_recv(struct kdbus_conn *conn,
> void __user *argp)
> .argc = ARRAY_SIZE(argv),
> };
>
> + ret = security_kdbus_recv(conn, conn->ep->bus);
> + if (ret)
> + return -EPERM;
> +
> if (!kdbus_conn_is_ordinary(conn) &&
> !kdbus_conn_is_monitor(conn) &&
> !kdbus_conn_is_activator(conn))
> diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
> index d1ffe90..1f91d39 100644
> --- a/ipc/kdbus/connection.h
> +++ b/ipc/kdbus/connection.h
> @@ -19,6 +19,7 @@
> #include <linux/kref.h>
> #include <linux/lockdep.h>
> #include <linux/path.h>
> +#include <uapi/linux/kdbus.h>
>
> #include "limits.h"
> #include "metadata.h"
> @@ -73,6 +74,7 @@ struct kdbus_kmsg;
> * @names_queue_list: Well-known names this connection waits for
> * @privileged: Whether this connection is
> privileged on the bus
> * @faked_meta: Whether the metadata was faked on
> HELLO
> + * @security: LSM security blob
> */
> struct kdbus_conn {
> struct kref kref;
> @@ -113,6 +115,8 @@ struct kdbus_conn {
>
> bool privileged:1;
> bool faked_meta:1;
> +
> + void *security;
> };
>
> struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn);
> diff --git a/ipc/kdbus/domain.c b/ipc/kdbus/domain.c
> index ac9f760..da9cdab 100644
> --- a/ipc/kdbus/domain.c
> +++ b/ipc/kdbus/domain.c
> @@ -20,6 +20,7 @@
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "domain.h"
> @@ -73,6 +74,7 @@ static void kdbus_domain_free(struct kdbus_node
> *node)
> put_user_ns(domain->user_namespace);
> ida_destroy(&domain->user_ida);
> idr_destroy(&domain->user_idr);
> + security_kdbus_domain_free(domain);
> kfree(domain);
> }
>
> @@ -104,6 +106,10 @@ struct kdbus_domain *kdbus_domain_new(unsigned
> int access)
> idr_init(&d->user_idr);
> ida_init(&d->user_ida);
>
> + ret = security_kdbus_domain_alloc(d);
> + if (ret)
> + return ERR_PTR(-EPERM);
> +
> /* Pin user namespace so we can guarantee domain-unique bus
> * names. */
> d->user_namespace = get_user_ns(current_user_ns());
>
> @@ -116,6 +122,7 @@ struct kdbus_domain *kdbus_domain_new(unsigned
> int access)
> exit_unref:
> kdbus_node_deactivate(&d->node);
> kdbus_node_unref(&d->node);
> + security_kdbus_domain_free(d);
> return ERR_PTR(ret);
> }
>
> @@ -264,7 +271,7 @@ static void __kdbus_user_free(struct kref *kref)
> if (uid_valid(user->uid))
> idr_remove(&user->domain->user_idr, __kuid_val(user
> ->uid));
> mutex_unlock(&user->domain->lock);
> -
> + security_kdbus_domain_free(user->domain);
> kdbus_domain_unref(user->domain);
> kfree(user);
> }
> diff --git a/ipc/kdbus/domain.h b/ipc/kdbus/domain.h
> index 447a2bd..3db06d8 100644
> --- a/ipc/kdbus/domain.h
> +++ b/ipc/kdbus/domain.h
> @@ -31,6 +31,7 @@
> * @user_ida: Set of all users to compute small indices
> * @user_namespace: User namespace, pinned at creation time
> * @dentry: Root dentry of VFS mount (don't use outside
> of kdbusfs)
> + * @security: LSM security blob
> */
> struct kdbus_domain {
> struct kdbus_node node;
> @@ -40,6 +41,7 @@ struct kdbus_domain {
> struct ida user_ida;
> struct user_namespace *user_namespace;
> struct dentry *dentry;
> + void *security;
> };
>
> /**
> diff --git a/ipc/kdbus/endpoint.c b/ipc/kdbus/endpoint.c
> index 9a95a5e..380228f 100644
> --- a/ipc/kdbus/endpoint.c
> +++ b/ipc/kdbus/endpoint.c
> @@ -21,6 +21,7 @@
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "connection.h"
> @@ -122,6 +123,12 @@ struct kdbus_ep *kdbus_ep_new(struct kdbus_bus
> *bus, const char *name,
> kdbus_policy_db_init(&e->policy_db);
> e->bus = kdbus_bus_ref(bus);
>
> + ret = security_kdbus_ep_create(bus);
> + if (ret) {
> + ret = -EPERM;
> + goto exit_unref;
> + }
> +
> ret = kdbus_node_link(&e->node, &bus->node, name);
> if (ret < 0)
> goto exit_unref;
> @@ -265,6 +272,10 @@ int kdbus_cmd_ep_update(struct kdbus_ep *ep,
> void __user *argp)
> .argc = ARRAY_SIZE(argv),
> };
>
> + ret = security_kdbus_ep_setpolicy(ep->bus);
> + if (ret)
> + return -EPERM;
> +
> ret = kdbus_args_parse(&args, argp, &cmd);
> if (ret != 0)
> return ret;
> diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
> index d77ee08..dd20bea 100644
> --- a/ipc/kdbus/names.c
> +++ b/ipc/kdbus/names.c
> @@ -24,6 +24,7 @@
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "connection.h"
> @@ -503,6 +504,12 @@ int kdbus_cmd_name_acquire(struct kdbus_conn
> *conn, void __user *argp)
> goto exit;
> }
>
> + ret = security_kdbus_name_acquire(conn, item_name);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> /*
> * Do atomic_inc_return here to reserve our slot, then
> decrement
> * it before returning.
> @@ -724,6 +731,10 @@ int kdbus_cmd_list(struct kdbus_conn *conn, void
> __user *argp)
> if (ret != 0)
> return ret;
>
> + ret = security_kdbus_name_list(conn->ep->bus);
> + if (ret)
> + return -EPERM;
> +
> /* lock order: domain -> bus -> ep -> names -> conn */
> down_read(®->rwlock);
> down_read(&conn->ep->bus->conn_rwlock);
> diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
> index 25bb3ad..9872fb4 100644
> --- a/ipc/kdbus/queue.c
> +++ b/ipc/kdbus/queue.c
> @@ -28,6 +28,7 @@
> #include <linux/slab.h>
> #include <linux/syscalls.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "util.h"
> #include "domain.h"
> @@ -514,12 +515,17 @@ int kdbus_queue_entry_install(struct
> kdbus_queue_entry *entry,
>
> for (i = 0; i < res->fds_count; i++) {
> if (install_fds) {
> - fds[i] =
> get_unused_fd_flags(O_CLOEXEC);
> - if (fds[i] >= 0)
> - fd_install(fds[i],
> - get_file(res
> ->fds[i]));
> - else
> + if (security_file_receive(res
> ->fds[i])) {
> + fds[i] = -1;
> incomplete_fds = true;
> + } else {
> + fds[i] =
> get_unused_fd_flags(O_CLOEXEC);
> + if (fds[i] >= 0)
> + fd_install(fds[i],
> + get_file(res
> ->fds[i]));
> + else
> + incomplete_fds =
> true;
> + }
> } else {
> fds[i] = -1;
> }
> @@ -557,13 +563,17 @@ int kdbus_queue_entry_install(struct
> kdbus_queue_entry *entry,
> m.fd = -1;
>
> if (install_fds) {
> - m.fd = get_unused_fd_flags(O_CLOEXEC);
> - if (m.fd < 0) {
> - m.fd = -1;
> + if (security_file_receive(d->memfd.file)) {
> incomplete_fds = true;
> } else {
> - fd_install(m.fd,
> - get_file(d->memfd.file));
> + m.fd =
> get_unused_fd_flags(O_CLOEXEC);
> + if (m.fd < 0) {
> + m.fd = -1;
> + incomplete_fds = true;
> + } else {
> + fd_install(m.fd,
> + get_file(d
> ->memfd.file));
> + }
> }
> }
>
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [RFC 3/8] kmsg: introduce additional kmsg devices support
From: Petr Mladek @ 2015-07-08 11:10 UTC (permalink / raw)
To: Marcin Niesluchowski
Cc: linux-doc, linux-kernel, linux-api, Jonathan Corbet,
Greg Kroah-Hartman, Tejun Heo, Kay Sievers, Andrew Morton,
Joe Perches, Karol Lewandowski, Bartlomiej Zolnierkiewicz
In-Reply-To: <1435920595-30879-4-git-send-email-m.niesluchow@samsung.com>
On Fri 2015-07-03 12:49:50, Marcin Niesluchowski wrote:
> kmsg device provides operations on cyclic logging buffer used mainly
> by kernel but also in userspace by privileged processes.
>
> Additional kmsg devices keep the same log format but may be added
> dynamically with custom size.
>
> Signed-off-by: Marcin Niesluchowski <m.niesluchow@samsung.com>
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -234,29 +234,37 @@ struct printk_log {
> u8 level:3; /* syslog level */
> };
Just in case, this is accepted. If you already touch the API, I would
suggest to rename struct printk_log to printk_msg. The current name
is pretty misleading.
> +struct log_buffer {
> +#ifdef CONFIG_PRINTK
> + struct list_head list; /* kmsg as head of the list */
> + char *buf; /* cyclic log buffer */
> + u32 len; /* buffer length */
> + wait_queue_head_t wait; /* wait queue for kmsg buffer */
> +#endif
> /*
> - * The logbuf_lock protects kmsg buffer, indices, counters. This can be taken
> - * within the scheduler's rq lock. It must be released before calling
> - * console_unlock() or anything else that might wake up a process.
> + * The lock protects kmsg buffer, indices, counters. This can be taken within
> + * the scheduler's rq lock. It must be released before calling console_unlock()
> + * or anything else that might wake up a process.
> */
> -static DEFINE_RAW_SPINLOCK(logbuf_lock);
> + raw_spinlock_t lock;
> + u64 first_seq; /* sequence number of the first record stored */
> + u32 first_idx; /* index of the first record stored */
> +/* sequence number of the next record to store */
> + u64 next_seq;
> +#ifdef CONFIG_PRINTK
> + u32 next_idx; /* index of the next record to store */
> + int mode; /* mode of device (kmsg_sys only) */
> + int minor; /* minor representing buffer device */
> +#endif
> +};
> @@ -1069,10 +1253,10 @@ const struct file_operations kmsg_fops = {
> */
> void log_buf_kexec_setup(void)
> {
> - VMCOREINFO_SYMBOL(log_buf);
> - VMCOREINFO_SYMBOL(log_buf_len);
> - VMCOREINFO_SYMBOL(log_first_idx);
> - VMCOREINFO_SYMBOL(log_next_idx);
> + VMCOREINFO_SYMBOL(log_buf.buf);
> + VMCOREINFO_SYMBOL(log_buf.len);
> + VMCOREINFO_SYMBOL(log_buf.first_idx);
> + VMCOREINFO_SYMBOL(log_buf.next_idx);
Have you tried to use this in crash or some other utility, please?
I guess that it will need to be exported a similar way like
struct printk_log, something like:
VMCOREINFO_SYMBOL(log_buf);
VMCOREINFO_STRUCT_SIZE(log_buffer);
VMCOREINFO_OFFSET(log_buffer, buf);
VMCOREINFO_OFFSET(log_buffer, len);
VMCOREINFO_OFFSET(log_buffer, first_idx);
VMCOREINFO_OFFSET(log_buffer, next_idx);
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC 0/8] Additional kmsg devices
From: Bartlomiej Zolnierkiewicz @ 2015-07-08 11:17 UTC (permalink / raw)
To: Richard Weinberger
Cc: Marcin Niesluchowski, linux-doc@vger.kernel.org, LKML,
open list:ABI/API, Jonathan Corbet, Greg Kroah-Hartman,
Petr Mladek, Tejun Heo, Kay Sievers, Andrew Morton, Joe Perches,
Karol Lewandowski
In-Reply-To: <559CE110.70503@nod.at>
Hi,
On Wednesday, July 08, 2015 10:36:32 AM Richard Weinberger wrote:
> Am 08.07.2015 um 10:30 schrieb Marcin Niesluchowski:
> > On 07/03/2015 05:19 PM, Richard Weinberger wrote:
> >> Am 03.07.2015 um 17:09 schrieb Marcin Niesluchowski:
> >>>> Why can't you just make sure that your target has a working
> >>>> syslogd/rsyslogd/journald/whatever?
> >>>> All can be done perfectly fine in userspace.
> >>> * Message credibility: Lets imagine simple service which collects logs via unix sockets. There is no reliable way of identifying logging process. getsockopt() with SO_PEERCRED
> >>> option would give pid form cred structure, but according to manual it may not be of actual logging process:
> >>> "The returned credentials are those that were in effect at the time of the call to connect(2) or socketpair(2)."
> >>> - select(7)
> >> This interface can be improved. Should be easy.
> >
> > What kind of improvement do you have in mind?
>
> I was wrong, we have the needed functionality already.
> See Andy's reply.
Please note that Andy has pointed out that the existing interface
(SCM_CREDENTIALS) is dangerous (=> should not be used).
Unfortunately his code for SCM_IDENTITY (which would replace
SCM_CREDENTIALS) has not materialized beyond initial 10% done
a year ago during SCP_PROCINFO discussion (it also has not been
explained enough to allow implementation by someone else).
> >>> * Early userspace tool: Helpful especially for embeded systems.
> >> This is what we do already. In early user space spawn your logger as early as possible.
> >> "embedded Linux is special" is not an excuse btw. ;)
> >
> > I would say "embedded Linux is real use case"instead of "special". What I meant that it does only require one ioctl and no additional resources are needed.
> >
> >>> * Reliability: Userspace service may be killed due to out of memory (OOM). This is kernel cyclic buffer, which size can be specified differently according to situation.
> >> This is what we have /proc/<pid>/oom_adj and /proc/<pid>/oom_score_adj for.
> >
> > You are right, but additional resources and complexity is required.
>
> A few "echo foo > /proc/xy/bar" commands are far less complexity than adding a pseudo syslogd to kernel land...
Please read actual patches. In roughly 600 new LOC they are doing
mainly two things:
* adding possibility to have more than one /dev/kmsg device & kernel
log buffer (~200 LOC)
* adding user interface for managing these additional devices/buffers
(~400 LOC)
I actually imagine that some time in the future we may also want to
have separate kernel log buffers for kernel usage itself..
> >>> * Possibility of using it with pstore: This code could be extended to log additional buffers to persistent storage same way main (kmsg) log buffer is.
> >> pstorefs and friends?
> >
> > pstore filesystem is used to access already stored kernel data (e.g. kmsg buffer). But does not provide mechanism of storing userspace memory.
>
> Which can be easily improved. Again, it will be less complex than your current approach.
>
> >>> * Use case of attaching file descriptor to stdout/stderr: Especially in early userspace.
> >> You can redirect these also in userspace.
> >
> > True for that, but as I said in my first argument there is no possibility of logging process identification in case of sockets.
> >
> >
> >>> * Performance: Those services mentioned by You are weeker solutions in that case. Especially systemd-journald is much too heavy soulution.
> >> Do you have numbers? I agree systemd-journald is heavy wight. But it is by far not the only logging daemon we have...
> >
> > I compared write operations on kmsg buffervia write/read operations on socketon SOCK_STREAM socket and sendmsg/recv on SOCK_DGRAM socket. Compared toSOCK_STREAM socket it was about
> > 39% slowerbut compared toSOCK_DGRAM socket it was about 326% faster.syslogfor example uses SOCK_DGRAM sockets.In all cases there were 2^20 (1048576) write/sendmsg operations of 2^8
> > (256) bytes.
>
> I still think the whole approach is wrong. Instead of giving up and going to kernel land, come up with a minimal userspace ringbuffer-syslogd.
> If the kernel lacks some support you need, add it. But don't move the whole thing int the kernel.
When it comes to possibility of logging things from user space to
kernel log buffer (through /dev/kmsg) then it has been added 3 years
ago in v3.5..
The changes being proposed are not doing what you're are trying to
imply - this is not kernel syslogd (like kdbus is a kernel dbus
implementation). They are merely enhancing existing /dev/kmsg
interface and may be useful also for kernel logging purposes some
time in the future..
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
From: Paul Osmialowski @ 2015-07-08 12:12 UTC (permalink / raw)
To: Lukasz Pawelczyk
Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown, Mark Rustad,
Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
Shuah Khan, Al Viro, linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Karol Lewandowski,
Lukasz Skalski
In-Reply-To: <1436353775.2331.2.camel-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 299 bytes --]
You're right about it. To be fixed. Thanks for spotting this.
On Wed, 8 Jul 2015, Lukasz Pawelczyk wrote:
>> + ret = -EPERM;
>> + goto exit_unref;
>> + }
>
> Also, why aren't the error codes passed from LSM? LSM can return
> anything, from EINVAL, to ENOMEM. Returning EPERM can be misleading.
>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-diff; name=0005-kdbus-use-LSM-hooks-in-kdbus-code.patch, Size: 13167 bytes --]
From 9c37667302f104e0f412ba2727c4d3df81fb59da Mon Sep 17 00:00:00 2001
From: Paul Osmialowski <p.osmialowsk@samsung.com>
Date: Tue, 26 May 2015 12:00:33 +0200
Subject: [RFC 5/8] kdbus: use LSM hooks in kdbus code
Originates from:
https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
commit: aa0885489d19be92fa41c6f0a71df28763228a40
Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
ipc/kdbus/bus.c | 11 ++++++++++-
ipc/kdbus/bus.h | 5 +++++
ipc/kdbus/connection.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
ipc/kdbus/connection.h | 6 ++++++
ipc/kdbus/domain.c | 9 ++++++++-
ipc/kdbus/domain.h | 4 ++++
ipc/kdbus/endpoint.c | 10 ++++++++++
ipc/kdbus/names.c | 10 ++++++++++
ipc/kdbus/queue.c | 30 ++++++++++++++++++++----------
9 files changed, 121 insertions(+), 12 deletions(-)
diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
index bbdf0f2..be7ed81 100644
--- a/ipc/kdbus/bus.c
+++ b/ipc/kdbus/bus.c
@@ -22,6 +22,7 @@
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/uio.h>
+#include <linux/security.h>
#include "bus.h"
#include "notify.h"
@@ -51,6 +52,7 @@ static void kdbus_bus_free(struct kdbus_node *node)
kdbus_domain_unref(bus->domain);
kdbus_policy_db_clear(&bus->policy_db);
kdbus_meta_proc_unref(bus->creator_meta);
+ security_kdbus_bus_free(bus);
kfree(bus);
}
@@ -161,6 +163,11 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
goto exit_unref;
}
+ ret = security_kdbus_bus_alloc(b);
+ if (ret) {
+ goto exit_unref;
+ }
+
/*
* Bus-limits of the creator are accounted on its real UID, just like
* all other per-user limits.
@@ -169,11 +176,13 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
if (IS_ERR(b->creator)) {
ret = PTR_ERR(b->creator);
b->creator = NULL;
- goto exit_unref;
+ goto exit_free_security;
}
return b;
+exit_free_security:
+ security_kdbus_bus_free(b);
exit_unref:
kdbus_node_deactivate(&b->node);
kdbus_node_unref(&b->node);
diff --git a/ipc/kdbus/bus.h b/ipc/kdbus/bus.h
index 5bea5ef..521e692 100644
--- a/ipc/kdbus/bus.h
+++ b/ipc/kdbus/bus.h
@@ -53,6 +53,7 @@ struct kdbus_user;
* @notify_list: List of pending kernel-generated messages
* @notify_lock: Notification list lock
* @notify_flush_lock: Notification flushing lock
+ * @security: LSM security blob
*/
struct kdbus_bus {
struct kdbus_node node;
@@ -81,6 +82,10 @@ struct kdbus_bus {
struct list_head notify_list;
spinlock_t notify_lock;
struct mutex notify_flush_lock;
+
+#ifdef CONFIG_SECURITY
+ void *security;
+#endif
};
struct kdbus_bus *kdbus_bus_ref(struct kdbus_bus *bus);
diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 9993753..4b9f0b6 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -31,6 +31,7 @@
#include <linux/slab.h>
#include <linux/syscalls.h>
#include <linux/uio.h>
+#include <linux/security.h>
#include "bus.h"
#include "connection.h"
@@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
bool is_activator;
bool is_monitor;
struct kvec kvec;
+ u32 sid, len;
+ char *label;
int ret;
struct {
@@ -222,6 +225,13 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
}
}
+ security_task_getsecid(current, &sid);
+ security_secid_to_secctx(sid, &label, &len);
+ ret = security_kdbus_connect(conn, label, len);
+ if (ret) {
+ goto exit_unref;
+ }
+
if (atomic_inc_return(&conn->user->connections) > KDBUS_USER_MAX_CONN) {
/* decremented by destructor as conn->user is valid */
ret = -EMFILE;
@@ -276,6 +286,7 @@ static void __kdbus_conn_free(struct kref *kref)
kdbus_pool_free(conn->pool);
kdbus_ep_unref(conn->ep);
put_cred(conn->cred);
+ security_kdbus_conn_free(conn);
kfree(conn->description);
kfree(conn->quota);
kfree(conn);
@@ -1107,6 +1118,11 @@ static int kdbus_conn_reply(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
if (ret < 0)
goto exit;
+ ret = security_kdbus_talk(src, dst);
+ if (ret) {
+ goto exit;
+ }
+
mutex_lock(&dst->lock);
reply = kdbus_reply_find(src, dst, kmsg->msg.cookie_reply);
if (reply) {
@@ -1187,6 +1203,11 @@ static struct kdbus_reply *kdbus_conn_call(struct kdbus_conn *src,
if (ret < 0)
goto exit;
+ ret = security_kdbus_talk(src, dst);
+ if (ret) {
+ goto exit;
+ }
+
if (!kdbus_conn_policy_talk(src, current_cred(), dst)) {
ret = -EPERM;
goto exit;
@@ -1248,6 +1269,11 @@ static int kdbus_conn_unicast(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
if (ret < 0)
goto exit;
+ ret = security_kdbus_talk(src, dst);
+ if (ret) {
+ goto exit;
+ }
+
if (is_signal) {
/* like broadcasts we eavesdrop even if the msg is dropped */
kdbus_bus_eavesdrop(bus, src, kmsg);
@@ -1639,6 +1665,11 @@ struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, bool privileged,
if (ret < 0)
goto exit;
+ ret = security_kdbus_ep_setpolicy(c->ep->bus);
+ if (ret) {
+ goto exit;
+ }
+
ret = kdbus_policy_set(&c->ep->bus->policy_db, args.items,
args.items_size, 1,
kdbus_conn_is_policy_holder(c), c);
@@ -1732,6 +1763,10 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp)
if (ret != 0)
return ret;
+ ret = security_kdbus_conn_info(conn);
+ if (ret)
+ return ret;
+
/* registry must be held throughout lookup *and* collecting data */
down_read(&bus->name_registry->rwlock);
@@ -1905,6 +1940,11 @@ int kdbus_cmd_update(struct kdbus_conn *conn, void __user *argp)
/* now that we verified the input, update the connection */
if (item_policy) {
+ ret = security_kdbus_ep_setpolicy(conn->ep->bus);
+ if (ret) {
+ goto exit;
+ }
+
ret = kdbus_policy_set(&conn->ep->bus->policy_db, cmd->items,
KDBUS_ITEMS_SIZE(cmd, items),
1, true, conn);
@@ -1948,6 +1988,10 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
.argc = ARRAY_SIZE(argv),
};
+ ret = security_kdbus_send(conn, conn->ep->bus);
+ if (ret)
+ return ret;
+
if (!kdbus_conn_is_ordinary(conn))
return -EOPNOTSUPP;
@@ -2044,6 +2088,10 @@ int kdbus_cmd_recv(struct kdbus_conn *conn, void __user *argp)
.argc = ARRAY_SIZE(argv),
};
+ ret = security_kdbus_recv(conn, conn->ep->bus);
+ if (ret)
+ return ret;
+
if (!kdbus_conn_is_ordinary(conn) &&
!kdbus_conn_is_monitor(conn) &&
!kdbus_conn_is_activator(conn))
diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
index d1ffe90..951f9b0 100644
--- a/ipc/kdbus/connection.h
+++ b/ipc/kdbus/connection.h
@@ -19,6 +19,7 @@
#include <linux/kref.h>
#include <linux/lockdep.h>
#include <linux/path.h>
+#include <uapi/linux/kdbus.h>
#include "limits.h"
#include "metadata.h"
@@ -73,6 +74,7 @@ struct kdbus_kmsg;
* @names_queue_list: Well-known names this connection waits for
* @privileged: Whether this connection is privileged on the bus
* @faked_meta: Whether the metadata was faked on HELLO
+ * @security: LSM security blob
*/
struct kdbus_conn {
struct kref kref;
@@ -113,6 +115,10 @@ struct kdbus_conn {
bool privileged:1;
bool faked_meta:1;
+
+#ifdef CONFIG_SECURITY
+ void *security;
+#endif
};
struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn);
diff --git a/ipc/kdbus/domain.c b/ipc/kdbus/domain.c
index ac9f760..e79080f 100644
--- a/ipc/kdbus/domain.c
+++ b/ipc/kdbus/domain.c
@@ -20,6 +20,7 @@
#include <linux/sizes.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/security.h>
#include "bus.h"
#include "domain.h"
@@ -73,6 +74,7 @@ static void kdbus_domain_free(struct kdbus_node *node)
put_user_ns(domain->user_namespace);
ida_destroy(&domain->user_ida);
idr_destroy(&domain->user_idr);
+ security_kdbus_domain_free(domain);
kfree(domain);
}
@@ -104,6 +106,10 @@ struct kdbus_domain *kdbus_domain_new(unsigned int access)
idr_init(&d->user_idr);
ida_init(&d->user_ida);
+ ret = security_kdbus_domain_alloc(d);
+ if (ret)
+ return ERR_PTR(ret);
+
/* Pin user namespace so we can guarantee domain-unique bus * names. */
d->user_namespace = get_user_ns(current_user_ns());
@@ -116,6 +122,7 @@ struct kdbus_domain *kdbus_domain_new(unsigned int access)
exit_unref:
kdbus_node_deactivate(&d->node);
kdbus_node_unref(&d->node);
+ security_kdbus_domain_free(d);
return ERR_PTR(ret);
}
@@ -264,7 +271,7 @@ static void __kdbus_user_free(struct kref *kref)
if (uid_valid(user->uid))
idr_remove(&user->domain->user_idr, __kuid_val(user->uid));
mutex_unlock(&user->domain->lock);
-
+ security_kdbus_domain_free(user->domain);
kdbus_domain_unref(user->domain);
kfree(user);
}
diff --git a/ipc/kdbus/domain.h b/ipc/kdbus/domain.h
index 447a2bd..4a935aa 100644
--- a/ipc/kdbus/domain.h
+++ b/ipc/kdbus/domain.h
@@ -31,6 +31,7 @@
* @user_ida: Set of all users to compute small indices
* @user_namespace: User namespace, pinned at creation time
* @dentry: Root dentry of VFS mount (don't use outside of kdbusfs)
+ * @security: LSM security blob
*/
struct kdbus_domain {
struct kdbus_node node;
@@ -40,6 +41,9 @@ struct kdbus_domain {
struct ida user_ida;
struct user_namespace *user_namespace;
struct dentry *dentry;
+#ifdef CONFIG_SECURITY
+ void *security;
+#endif
};
/**
diff --git a/ipc/kdbus/endpoint.c b/ipc/kdbus/endpoint.c
index 9a95a5e..6ff74ee 100644
--- a/ipc/kdbus/endpoint.c
+++ b/ipc/kdbus/endpoint.c
@@ -21,6 +21,7 @@
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/uio.h>
+#include <linux/security.h>
#include "bus.h"
#include "connection.h"
@@ -122,6 +123,11 @@ struct kdbus_ep *kdbus_ep_new(struct kdbus_bus *bus, const char *name,
kdbus_policy_db_init(&e->policy_db);
e->bus = kdbus_bus_ref(bus);
+ ret = security_kdbus_ep_create(bus);
+ if (ret) {
+ goto exit_unref;
+ }
+
ret = kdbus_node_link(&e->node, &bus->node, name);
if (ret < 0)
goto exit_unref;
@@ -265,6 +271,10 @@ int kdbus_cmd_ep_update(struct kdbus_ep *ep, void __user *argp)
.argc = ARRAY_SIZE(argv),
};
+ ret = security_kdbus_ep_setpolicy(ep->bus);
+ if (ret)
+ return ret;
+
ret = kdbus_args_parse(&args, argp, &cmd);
if (ret != 0)
return ret;
diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index d77ee08..fcc1a49 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -24,6 +24,7 @@
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/uio.h>
+#include <linux/security.h>
#include "bus.h"
#include "connection.h"
@@ -503,6 +504,11 @@ int kdbus_cmd_name_acquire(struct kdbus_conn *conn, void __user *argp)
goto exit;
}
+ ret = security_kdbus_name_acquire(conn, item_name);
+ if (ret) {
+ goto exit;
+ }
+
/*
* Do atomic_inc_return here to reserve our slot, then decrement
* it before returning.
@@ -724,6 +730,10 @@ int kdbus_cmd_list(struct kdbus_conn *conn, void __user *argp)
if (ret != 0)
return ret;
+ ret = security_kdbus_name_list(conn->ep->bus);
+ if (ret)
+ return ret;
+
/* lock order: domain -> bus -> ep -> names -> conn */
down_read(®->rwlock);
down_read(&conn->ep->bus->conn_rwlock);
diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
index 25bb3ad..9872fb4 100644
--- a/ipc/kdbus/queue.c
+++ b/ipc/kdbus/queue.c
@@ -28,6 +28,7 @@
#include <linux/slab.h>
#include <linux/syscalls.h>
#include <linux/uio.h>
+#include <linux/security.h>
#include "util.h"
#include "domain.h"
@@ -514,12 +515,17 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
for (i = 0; i < res->fds_count; i++) {
if (install_fds) {
- fds[i] = get_unused_fd_flags(O_CLOEXEC);
- if (fds[i] >= 0)
- fd_install(fds[i],
- get_file(res->fds[i]));
- else
+ if (security_file_receive(res->fds[i])) {
+ fds[i] = -1;
incomplete_fds = true;
+ } else {
+ fds[i] = get_unused_fd_flags(O_CLOEXEC);
+ if (fds[i] >= 0)
+ fd_install(fds[i],
+ get_file(res->fds[i]));
+ else
+ incomplete_fds = true;
+ }
} else {
fds[i] = -1;
}
@@ -557,13 +563,17 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
m.fd = -1;
if (install_fds) {
- m.fd = get_unused_fd_flags(O_CLOEXEC);
- if (m.fd < 0) {
- m.fd = -1;
+ if (security_file_receive(d->memfd.file)) {
incomplete_fds = true;
} else {
- fd_install(m.fd,
- get_file(d->memfd.file));
+ m.fd = get_unused_fd_flags(O_CLOEXEC);
+ if (m.fd < 0) {
+ m.fd = -1;
+ incomplete_fds = true;
+ } else {
+ fd_install(m.fd,
+ get_file(d->memfd.file));
+ }
}
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v3 01/11] stm class: Introduce an abstraction for System Trace Module devices
From: Chunyan Zhang @ 2015-07-08 12:32 UTC (permalink / raw)
To: Alexander Shishkin
Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org, mathieu.poirier,
peter.lachner, norbert.schulz, keven.boell, yann.fouassier,
laurent.fert, linux-api@vger.kernel.org
In-Reply-To: <1436177344-16751-2-git-send-email-alexander.shishkin@linux.intel.com>
On Mon, Jul 6, 2015 at 6:08 PM, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> A System Trace Module (STM) is a device exporting data in System Trace
> Protocol (STP) format as defined by MIPI STP standards. Examples of such
> devices are Intel Trace Hub and Coresight STM.
>
> This abstraction provides a unified interface for software trace sources
> to send their data over an STM device to a debug host. In order to do
> that, such a trace source needs to be assigned a pair of master/channel
> identifiers that all the data from this source will be tagged with. The
> STP decoder on the debug host side will use these master/channel tags to
> distinguish different trace streams from one another inside one STP
> stream.
>
> This abstraction provides a configfs-based policy management mechanism
> for dynamic allocation of these master/channel pairs based on trace
> source-supplied string identifier. It has the flexibility of being
> defined at runtime and at the same time (provided that the policy
> definition is aligned with the decoding end) consistency.
>
> For userspace trace sources, this abstraction provides write()-based and
> mmap()-based (if the underlying stm device allows this) output mechanism.
>
> For kernel-side trace sources, we provide "stm_source" device class that
> can be connected to an stm device at run time.
>
> Cc: linux-api@vger.kernel.org
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> ---
> Documentation/ABI/testing/configfs-stp-policy | 48 +
> Documentation/ABI/testing/sysfs-class-stm | 14 +
> Documentation/ABI/testing/sysfs-class-stm_source | 11 +
> Documentation/ioctl/ioctl-number.txt | 3 +
> Documentation/trace/stm.txt | 80 ++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/hwtracing/stm/Kconfig | 8 +
> drivers/hwtracing/stm/Makefile | 3 +
> drivers/hwtracing/stm/core.c | 1029 ++++++++++++++++++++++
> drivers/hwtracing/stm/policy.c | 529 +++++++++++
> drivers/hwtracing/stm/stm.h | 87 ++
> include/linux/stm.h | 126 +++
> include/uapi/linux/stm.h | 50 ++
> 14 files changed, 1991 insertions(+)
> create mode 100644 Documentation/ABI/testing/configfs-stp-policy
> create mode 100644 Documentation/ABI/testing/sysfs-class-stm
> create mode 100644 Documentation/ABI/testing/sysfs-class-stm_source
> create mode 100644 Documentation/trace/stm.txt
> create mode 100644 drivers/hwtracing/stm/Kconfig
> create mode 100644 drivers/hwtracing/stm/Makefile
> create mode 100644 drivers/hwtracing/stm/core.c
> create mode 100644 drivers/hwtracing/stm/policy.c
> create mode 100644 drivers/hwtracing/stm/stm.h
> create mode 100644 include/linux/stm.h
> create mode 100644 include/uapi/linux/stm.h
>
> diff --git a/Documentation/ABI/testing/configfs-stp-policy b/Documentation/ABI/testing/configfs-stp-policy
> new file mode 100644
> index 0000000000..421ce6825c
> --- /dev/null
> +++ b/Documentation/ABI/testing/configfs-stp-policy
> @@ -0,0 +1,48 @@
> +What: /config/stp-policy
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + This group contains policies mandating Master/Channel allocation
> + for software sources wishing to send trace data over an STM
> + device.
> +
> +What: /config/stp-policy/<device>.<policy>
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + This group is the root of a policy; its name is a concatenation
> + of an stm device name to which this policy applies and an
> + arbitrary string. If <device> part doesn't match an existing
> + stm device, mkdir will fail with ENODEV; if that device already
> + has a policy assigned to it, mkdir will fail with EBUSY.
> +
> +What: /config/stp-policy/<device>.<policy>/device
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + STM device to which this policy applies, read only. Same as the
> + <device> component of its parent directory.
> +
> +What: /config/stp-policy/<device>.<policy>/<node>
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + Policy node is a string identifier that software clients will
> + use to request a master/channel to be allocated and assigned to
> + them.
> +
> +What: /config/stp-policy/<device>.<policy>/<node>/masters
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + Range of masters from which to allocate for users of this node.
> + Write two numbers: the first master and the last master number.
> +
> +What: /config/stp-policy/<device>.<policy>/<node>/channels
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + Range of channels from which to allocate for users of this node.
> + Write two numbers: the first channel and the last channel
> + number.
> +
> diff --git a/Documentation/ABI/testing/sysfs-class-stm b/Documentation/ABI/testing/sysfs-class-stm
> new file mode 100644
> index 0000000000..c9aa4f3fc9
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-stm
> @@ -0,0 +1,14 @@
> +What: /sys/class/stm/<stm>/masters
> +Date: June 2015
> +KernelVersion: 4.3
> +Contact: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> +Description:
> + Shows first and last available to software master numbers on
> + this STM device.
> +
> +What: /sys/class/stm/<stm>/channels
> +Date: June 2015
> +KernelVersion: 4.3
> +Contact: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> +Description:
> + Shows the number of channels per master on this STM device.
> diff --git a/Documentation/ABI/testing/sysfs-class-stm_source b/Documentation/ABI/testing/sysfs-class-stm_source
> new file mode 100644
> index 0000000000..57b8dd39bb
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-stm_source
> @@ -0,0 +1,11 @@
> +What: /sys/class/stm_source/<stm_source>/stm_source_link
> +Date: June 2015
> +KernelVersion: 4.3
> +Contact: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> +Description:
> + stm_source device linkage to stm device, where its tracing data
> + is directed. Reads return an existing connection or "<none>" if
> + this stm_source is not connected to any stm device yet.
> + Write an existing (registered) stm device's name here to
> + connect that device. If a device is already connected to this
> + stm_source, it will first be disconnected.
> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
> index 611c52267d..7fa29b4ece 100644
> --- a/Documentation/ioctl/ioctl-number.txt
> +++ b/Documentation/ioctl/ioctl-number.txt
> @@ -81,6 +81,9 @@ Code Seq#(hex) Include File Comments
> 0x22 all scsi/sg.h
> '#' 00-3F IEEE 1394 Subsystem Block for the entire subsystem
> '$' 00-0F linux/perf_counter.h, linux/perf_event.h
> +'%' 00-0F include/uapi/linux/stm.h
> + System Trace Module subsystem
> + <mailto:alexander.shishkin@linux.intel.com>
> '&' 00-07 drivers/firewire/nosy-user.h
> '1' 00-1F <linux/timepps.h> PPS kit from Ulrich Windl
> <ftp://ftp.de.kernel.org/pub/linux/daemons/ntp/PPS/>
> diff --git a/Documentation/trace/stm.txt b/Documentation/trace/stm.txt
> new file mode 100644
> index 0000000000..e140f2e8d7
> --- /dev/null
> +++ b/Documentation/trace/stm.txt
> @@ -0,0 +1,80 @@
> +System Trace Module
> +===================
> +
> +System Trace Module (STM) is a device described in MIPI STP specs as
> +STP trace stream generator. STP (System Trace Protocol) is a trace
> +protocol multiplexing data from multiple trace sources, each one of
> +which is assigned a unique pair of master and channel. While some of
> +these masters and channels are statically allocated to certain
> +hardware trace sources, others are available to software. Software
> +trace sources are usually free to pick for themselves any
> +master/channel combination from this pool.
> +
> +On the receiving end of this STP stream (the decoder side), trace
> +sources can only be identified by master/channel combination, so in
> +order for the decoder to be able to make sense of the trace that
> +involves multiple trace sources, it needs to be able to map those
> +master/channel pairs to the trace sources that it understands.
> +
> +For instance, it is helpful to know that syslog messages come on
> +master 7 channel 15, while arbitrary user applications can use masters
> +48 to 63 and channels 0 to 127.
> +
> +To solve this mapping problem, stm class provides a policy management
> +mechanism via configfs, that allows defining rules that map string
> +identifiers to ranges of masters and channels. If these rules (policy)
> +are consistent with what decoder expects, it will be able to properly
> +process the trace data.
> +
> +This policy is a tree structure containing rules (policy_node) that
> +have a name (string identifier) and a range of masters and channels
> +associated with it, located in "stp-policy" subsystem directory in
> +configfs. The topmost directory's name (the policy) is formatted as
> +the STM device name to which this policy applies and and arbitrary
> +string identifier separated by a stop. From the examle above, a rule
> +may look like this:
> +
> +$ ls /config/stp-policy/dummy_stm.my-policy/user
> +channels masters
> +$ cat /config/stp-policy/dummy_stm.my-policy/user/masters
> +48 63
> +$ cat /config/stp-policy/dummy_stm.my-policy/user/channels
> +0 127
> +
> +which means that the master allocation pool for this rule consists of
> +masters 48 through 63 and channel allocation pool has channels 0
> +through 127 in it. Now, any producer (trace source) identifying itself
> +with "user" identification string will be allocated a master and
> +channel from within these ranges.
> +
> +These rules can be nested, for example, one can define a rule "dummy"
> +under "user" directory from the example above and this new rule will
> +be used for trace sources with the id string of "user/dummy".
> +
> +Trace sources have to open the stm class device's node and write their
> +trace data into its file descriptor. In order to identify themselves
> +to the policy, they need to do a STP_POLICY_ID_SET ioctl on this file
> +descriptor providing their id string. Otherwise, they will be
> +automatically allocated a master/channel pair upon first write to this
> +file descriptor according to the "default" rule of the policy, if such
> +exists.
> +
> +Some STM devices may allow direct mapping of the channel mmio regions
> +to userspace for zero-copy writing. One mappable page (in terms of
> +mmu) will usually contain multiple channels' mmios, so the user will
> +need to allocate that many channels to themselves (via the
> +aforementioned ioctl() call) to be able to do this. That is, if your
> +stm device's channel mmio region is 64 bytes and hardware page size is
> +4096 bytes, after a successful STP_POLICY_ID_SET ioctl() call with
> +width==64, you should be able to mmap() one page on this file
> +descriptor and obtain direct access to an mmio region for 64 channels.
> +
> +For kernel-based trace sources, there is "stm_source" device
> +class. Devices of this class can be connected and disconnected to/from
> +stm devices at runtime via a sysfs attribute.
> +
> +Examples of STM devices are Intel Trace Hub [1] and Coresight STM
> +[2].
> +
> +[1] https://software.intel.com/sites/default/files/managed/d3/3c/intel-th-developer-manual.pdf
> +[2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0444b/index.html
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 6e973b8e3a..96efe4522c 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -184,4 +184,6 @@ source "drivers/android/Kconfig"
>
> source "drivers/nvdimm/Kconfig"
>
> +source "drivers/hwtracing/stm/Kconfig"
> +
> endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index b64b49f6e0..6647bc923f 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -164,4 +164,5 @@ obj-$(CONFIG_MCB) += mcb/
> obj-$(CONFIG_RAS) += ras/
> obj-$(CONFIG_THUNDERBOLT) += thunderbolt/
> obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
> +obj-$(CONFIG_STM) += hwtracing/stm/
> obj-$(CONFIG_ANDROID) += android/
> diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
> new file mode 100644
> index 0000000000..90ed327461
> --- /dev/null
> +++ b/drivers/hwtracing/stm/Kconfig
> @@ -0,0 +1,8 @@
> +config STM
> + tristate "System Trace Module devices"
> + help
> + A System Trace Module (STM) is a device exporting data in System
> + Trace Protocol (STP) format as defined by MIPI STP standards.
> + Examples of such devices are Intel Trace Hub and Coresight STM.
> +
> + Say Y here to enable System Trace Module device support.
> diff --git a/drivers/hwtracing/stm/Makefile b/drivers/hwtracing/stm/Makefile
> new file mode 100644
> index 0000000000..adec701649
> --- /dev/null
> +++ b/drivers/hwtracing/stm/Makefile
> @@ -0,0 +1,3 @@
> +obj-$(CONFIG_STM) += stm_core.o
> +
> +stm_core-y := core.o policy.o
> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
> new file mode 100644
> index 0000000000..b79c42c625
> --- /dev/null
> +++ b/drivers/hwtracing/stm/core.c
> @@ -0,0 +1,1029 @@
> +/*
> + * System Trace Module (STM) infrastructure
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * STM class implements generic infrastructure for System Trace Module devices
> + * as defined in MIPI STPv2 specification.
> + */
> +
> +#include <linux/uaccess.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/compat.h>
> +#include <linux/kdev_t.h>
> +#include <linux/srcu.h>
> +#include <linux/slab.h>
> +#include <linux/stm.h>
> +#include <linux/fs.h>
> +#include <linux/mm.h>
> +#include "stm.h"
> +
> +#include <uapi/linux/stm.h>
> +
> +static unsigned int stm_core_up;
> +
> +/*
> + * The SRCU here makes sure that STM device doesn't disappear from under a
> + * stm_source_write() caller, which may want to have as little overhead as
> + * possible.
> + */
> +static struct srcu_struct stm_source_srcu;
> +
> +static ssize_t masters_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct stm_device *stm = to_stm_device(dev);
> + int ret;
> +
> + ret = sprintf(buf, "%u %u\n", stm->data->sw_start, stm->data->sw_end);
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR_RO(masters);
> +
> +static ssize_t channels_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct stm_device *stm = to_stm_device(dev);
> + int ret;
> +
> + ret = sprintf(buf, "%u\n", stm->data->sw_nchannels);
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR_RO(channels);
> +
> +static struct attribute *stm_attrs[] = {
> + &dev_attr_masters.attr,
> + &dev_attr_channels.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(stm);
> +
> +static struct class stm_class = {
> + .name = "stm",
> + .dev_groups = stm_groups,
> +};
> +
> +static int stm_dev_match(struct device *dev, const void *data)
> +{
> + const char *name = data;
> +
> + return sysfs_streq(name, dev_name(dev));
> +}
> +
> +/**
> + * stm_find_device() - find stm device by name
> + * @buf: character buffer containing the name
> + *
> + * This is called when either policy gets assigned to an stm device or an
> + * stm_source device gets linked to an stm device.
> + *
> + * This grabs device's reference (get_device()) and module reference, both
> + * of which the calling path needs to make sure to drop with stm_put_device().
> + *
> + * Return: stm device pointer or null if lookup failed.
> + */
> +struct stm_device *stm_find_device(const char *buf)
> +{
> + struct stm_device *stm;
> + struct device *dev;
> +
> + if (!stm_core_up)
> + return NULL;
> +
> + dev = class_find_device(&stm_class, NULL, buf, stm_dev_match);
> + if (!dev)
> + return NULL;
> +
> + stm = to_stm_device(dev);
> + if (!try_module_get(stm->owner)) {
> + put_device(dev);
> + return NULL;
> + }
> +
> + return stm;
> +}
> +
> +/**
> + * stm_put_device() - drop references on the stm device
> + * @stm: stm device, previously acquired by stm_find_device()
> + *
> + * This drops the module reference and device reference taken by
> + * stm_find_device().
> + */
> +void stm_put_device(struct stm_device *stm)
> +{
> + module_put(stm->owner);
> + put_device(&stm->dev);
> +}
> +
> +/*
> + * Internally we only care about software-writable masters here, that is the
> + * ones in the range [stm_data->sw_start..stm_data..sw_end], however we need
> + * original master numbers to be visible externally, since they are the ones
> + * that will appear in the STP stream. Thus, the internal bookkeeping uses
> + * $master - stm_data->sw_start to reference master descriptors and such.
> + */
> +
> +#define __stm_master(_s, _m) \
> + ((_s)->masters[(_m) - (_s)->data->sw_start])
> +
> +static inline struct stp_master *
> +stm_master(struct stm_device *stm, unsigned int idx)
> +{
> + if (idx < stm->data->sw_start || idx > stm->data->sw_end)
> + return NULL;
> +
> + return __stm_master(stm, idx);
> +}
> +
> +static int stp_master_alloc(struct stm_device *stm, unsigned int idx)
> +{
> + struct stp_master *master;
> + size_t size;
> +
> + size = ALIGN(stm->data->sw_nchannels, 8) / 8;
> + size += sizeof(struct stp_master);
> + master = kzalloc(size, GFP_ATOMIC);
> + if (!master)
> + return -ENOMEM;
> +
> + master->nr_free = stm->data->sw_nchannels;
> + __stm_master(stm, idx) = master;
> +
> + return 0;
> +}
> +
> +static void stp_master_free(struct stm_device *stm, unsigned int idx)
> +{
> + struct stp_master *master = stm_master(stm, idx);
> +
> + if (!master)
> + return;
> +
> + __stm_master(stm, idx) = NULL;
> + kfree(master);
> +}
> +
> +static void stm_output_claim(struct stm_device *stm, struct stm_output *output)
> +{
> + struct stp_master *master = stm_master(stm, output->master);
> +
> + if (WARN_ON_ONCE(master->nr_free < output->nr_chans))
> + return;
> +
> + bitmap_allocate_region(&master->chan_map[0], output->channel,
> + ilog2(output->nr_chans));
> +
> + master->nr_free -= output->nr_chans;
> +}
> +
> +static void
> +stm_output_disclaim(struct stm_device *stm, struct stm_output *output)
> +{
> + struct stp_master *master = stm_master(stm, output->master);
> +
> + bitmap_release_region(&master->chan_map[0], output->channel,
> + ilog2(output->nr_chans));
> +
> + output->nr_chans = 0;
> + master->nr_free += output->nr_chans;
> +}
> +
> +/*
> + * This is like bitmap_find_free_region(), except it can ignore @start bits
> + * at the beginning.
> + */
> +static int find_free_channels(unsigned long *bitmap, unsigned int start,
> + unsigned int end, unsigned int width)
> +{
> + unsigned int pos;
> + int i;
> +
> + for (pos = start; pos < end + 1; pos = ALIGN(pos, width)) {
> + pos = find_next_zero_bit(bitmap, end + 1, pos);
> + if (pos + width > end + 1)
> + break;
> +
> + if (pos & (width - 1))
> + continue;
> +
> + for (i = 1; i < width && !test_bit(pos + i, bitmap); i++)
> + ;
> + if (i == width)
> + return pos;
> + }
> +
> + return -1;
> +}
> +
> +static unsigned int
> +stm_find_master_chan(struct stm_device *stm, unsigned int width,
> + unsigned int *mstart, unsigned int mend,
> + unsigned int *cstart, unsigned int cend)
> +{
> + struct stp_master *master;
> + unsigned int midx;
> + int pos, err;
> +
> + for (midx = *mstart; midx <= mend; midx++) {
> + if (!stm_master(stm, midx)) {
> + err = stp_master_alloc(stm, midx);
> + if (err)
> + return err;
> + }
> +
> + master = stm_master(stm, midx);
> +
> + if (!master->nr_free)
> + continue;
> +
> + pos = find_free_channels(master->chan_map, *cstart, cend,
> + width);
> + if (pos < 0)
> + continue;
> +
> + *mstart = midx;
> + *cstart = pos;
> + return 0;
> + }
> +
> + return -ENOSPC;
> +}
> +
> +static int stm_output_assign(struct stm_device *stm, unsigned int width,
> + struct stp_policy_node *policy_node,
> + struct stm_output *output)
> +{
> + unsigned int midx, cidx, mend, cend;
> + int ret = -EINVAL;
> +
> + if (width > stm->data->sw_nchannels)
> + return -EINVAL;
> +
> + if (policy_node) {
> + stp_policy_node_get_ranges(policy_node,
> + &midx, &mend, &cidx, &cend);
> + } else {
> + midx = stm->data->sw_start;
> + cidx = 0;
> + mend = stm->data->sw_end;
> + cend = stm->data->sw_nchannels - 1;
> + }
> +
> + spin_lock(&stm->mc_lock);
> + /* output is already assigned -- shouldn't happen */
> + if (WARN_ON_ONCE(output->nr_chans))
> + goto unlock;
> +
> + ret = stm_find_master_chan(stm, width, &midx, mend, &cidx, cend);
> + if (ret)
> + goto unlock;
> +
> + output->master = midx;
> + output->channel = cidx;
> + output->nr_chans = width;
> + stm_output_claim(stm, output);
> + dev_dbg(&stm->dev, "assigned %u:%u (+%u)\n", midx, cidx, width);
> +
> + ret = 0;
> +unlock:
> + spin_unlock(&stm->mc_lock);
> +
> + return ret;
> +}
> +
> +static void stm_output_free(struct stm_device *stm, struct stm_output *output)
> +{
> + spin_lock(&stm->mc_lock);
> + if (output->nr_chans)
> + stm_output_disclaim(stm, output);
> + spin_unlock(&stm->mc_lock);
> +}
> +
> +static int major_match(struct device *dev, const void *data)
> +{
> + unsigned int major = *(unsigned int *)data;
> +
> + return MAJOR(dev->devt) == major;
> +}
> +
> +static int stm_char_open(struct inode *inode, struct file *file)
> +{
> + struct stm_file *stmf;
> + struct device *dev;
> + unsigned int major = imajor(inode);
> + int err = -ENODEV;
> +
> + dev = class_find_device(&stm_class, NULL, &major, major_match);
> + if (!dev)
> + return -ENODEV;
> +
> + stmf = kzalloc(sizeof(*stmf), GFP_KERNEL);
> + if (!stmf)
> + return -ENOMEM;
> +
> + stmf->stm = to_stm_device(dev);
> +
> + if (!try_module_get(stmf->stm->owner))
> + goto err_free;
> +
> + file->private_data = stmf;
> +
> + return nonseekable_open(inode, file);
> +
> +err_free:
> + kfree(stmf);
> +
> + return err;
> +}
> +
> +static int stm_char_release(struct inode *inode, struct file *file)
> +{
> + struct stm_file *stmf = file->private_data;
> +
> + stm_output_free(stmf->stm, &stmf->output);
> + stm_put_device(stmf->stm);
> + kfree(stmf);
> +
> + return 0;
> +}
> +
> +static int stm_file_assign(struct stm_file *stmf, char *id, unsigned int width)
> +{
> + struct stm_device *stm = stmf->stm;
> + int ret;
> +
> + stmf->policy_node = stp_policy_node_lookup(stm, id);
> +
> + ret = stm_output_assign(stm, width, stmf->policy_node, &stmf->output);
> +
> + if (stmf->policy_node)
> + stp_policy_node_put(stmf->policy_node);
> +
> + return ret;
> +}
> +
> +static void stm_write(struct stm_data *data, unsigned int master,
> + unsigned int channel, const char *buf, size_t count)
> +{
> + unsigned int flags = STP_PACKET_TIMESTAMPED;
> + const unsigned char *p = buf, nil = 0;
> + size_t pos;
> + ssize_t sz;
> +
> + for (pos = 0, p = buf; count > pos; pos += sz, p += sz) {
> + sz = min_t(unsigned int, count - pos, 8);
> + sz = data->packet(data, master, channel, STP_PACKET_DATA, flags,
> + sz, p);
> + flags = 0;
> + }
> +
> + data->packet(data, master, channel, STP_PACKET_FLAG, 0, 0, &nil);
> +}
> +
> +static ssize_t stm_char_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct stm_file *stmf = file->private_data;
> + struct stm_device *stm = stmf->stm;
> + char *kbuf;
> + int err;
> +
> + /*
> + * if no m/c have been assigned to this writer up to this
> + * point, use "default" policy entry
> + */
> + if (!stmf->output.nr_chans) {
> + err = stm_file_assign(stmf, "default", 1);
> + /*
> + * EBUSY means that somebody else just assigned this
> + * output, which is just fine for write()
> + */
> + if (err && err != -EBUSY)
> + return err;
> + }
> +
> + kbuf = kmalloc(count + 1, GFP_KERNEL);
> + if (!kbuf)
> + return -ENOMEM;
> +
> + err = copy_from_user(kbuf, buf, count);
> + if (err) {
> + kfree(kbuf);
> + return -EFAULT;
> + }
> +
> + stm_write(stm->data, stmf->output.master, stmf->output.channel, kbuf,
> + count);
> +
> + kfree(kbuf);
> +
> + return count;
> +}
> +
> +static int stm_char_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct stm_file *stmf = file->private_data;
> + struct stm_device *stm = stmf->stm;
> + unsigned long size, phys;
> +
> + if (!stm->data->mmio_addr)
> + return -EOPNOTSUPP;
> +
> + if (vma->vm_pgoff)
> + return -EINVAL;
> +
> + size = vma->vm_end - vma->vm_start;
> +
> + if (stmf->output.nr_chans * stm->data->sw_mmiosz != size)
> + return -EINVAL;
> +
> + phys = stm->data->mmio_addr(stm->data, stmf->output.master,
> + stmf->output.channel,
> + stmf->output.nr_chans);
> +
> + if (!phys)
> + return -EINVAL;
> +
> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
> + vm_iomap_memory(vma, phys, size);
> +
> + return 0;
> +}
> +
> +static int stm_char_policy_set_ioctl(struct stm_file *stmf, void __user *arg)
> +{
> + struct stm_device *stm = stmf->stm;
> + struct stp_policy_id *id;
> + int ret = -EINVAL;
> + u32 size;
> +
> + if (stmf->output.nr_chans)
> + return -EBUSY;
> +
> + if (copy_from_user(&size, arg, sizeof(size)))
> + return -EFAULT;
> +
> + if (size >= PATH_MAX + sizeof(*id))
> + return -EINVAL;
> +
> + /*
> + * size + 1 to make sure the .id string at the bottom is terminated,
> + * which is also why memdup_user() is not useful here
> + */
> + id = kzalloc(size + 1, GFP_KERNEL);
> + if (!id)
> + return -ENOMEM;
> +
> + if (copy_from_user(id, arg, size)) {
> + ret = -EFAULT;
> + goto err_free;
> + }
> +
> + if (id->__reserved_0 || id->__reserved_1)
> + goto err_free;
> +
> + if (id->width < 1 ||
> + id->width > PAGE_SIZE / stm->data->sw_mmiosz)
> + goto err_free;
> +
> + ret = stm_file_assign(stmf, id->id, id->width);
> + if (ret)
> + goto err_free;
> +
> + ret = 0;
> +
> + if (stm->data->link)
> + ret = stm->data->link(stm->data, stmf->output.master,
> + stmf->output.channel);
> +
> + if (ret) {
> + stm_output_free(stmf->stm, &stmf->output);
> + stm_put_device(stmf->stm);
> + }
> +
> +err_free:
> + kfree(id);
> +
> + return ret;
> +}
> +
> +static int stm_char_policy_get_ioctl(struct stm_file *stmf, void __user *arg)
> +{
> + struct stp_policy_id id = {
> + .size = sizeof(id),
> + .master = stmf->output.master,
> + .channel = stmf->output.channel,
> + .width = stmf->output.nr_chans,
> + .__reserved_0 = 0,
> + .__reserved_1 = 0,
> + };
> +
> + return copy_to_user(arg, &id, id.size) ? -EFAULT : 0;
> +}
> +
> +static long
> +stm_char_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + struct stm_file *stmf = file->private_data;
> + struct stm_data *stm_data = stmf->stm->data;
> + int err = -ENOTTY;
> + u64 options;
> +
> + switch (cmd) {
> + case STP_POLICY_ID_SET:
> + err = stm_char_policy_set_ioctl(stmf, (void __user *)arg);
> + if (err)
> + return err;
> +
> + return stm_char_policy_get_ioctl(stmf, (void __user *)arg);
> +
> + case STP_POLICY_ID_GET:
> + return stm_char_policy_get_ioctl(stmf, (void __user *)arg);
> +
> + case STP_SET_OPTIONS:
> + if (copy_from_user(&options, (u64 __user *)arg, sizeof(u64)))
> + return -EFAULT;
> +
> + if (stm_data->set_options)
> + err = stm_data->set_options(stm_data,
> + stmf->output.master,
> + stmf->output.channel,
> + stmf->output.nr_chans,
> + options);
> +
> + break;
> + default:
> + break;
> + }
> +
> + return err;
> +}
> +
> +#ifdef CONFIG_COMPAT
> +static long
> +stm_char_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + return stm_char_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> +}
> +#else
> +#define stm_char_compat_ioctl NULL
> +#endif
> +
> +static const struct file_operations stm_fops = {
> + .open = stm_char_open,
> + .release = stm_char_release,
> + .write = stm_char_write,
> + .mmap = stm_char_mmap,
> + .unlocked_ioctl = stm_char_ioctl,
> + .compat_ioctl = stm_char_compat_ioctl,
> + .llseek = no_llseek,
> +};
> +
> +static void stm_device_release(struct device *dev)
> +{
> + struct stm_device *stm = to_stm_device(dev);
> +
> + kfree(stm);
> +}
> +
> +int stm_register_device(struct device *parent, struct stm_data *stm_data,
> + struct module *owner)
> +{
> + struct stm_device *stm;
> + unsigned int nmasters;
> + int err = -ENOMEM;
> +
> + if (!stm_core_up)
> + return -EPROBE_DEFER;
> +
> + if (!stm_data->packet || !stm_data->sw_nchannels)
> + return -EINVAL;
> +
> + nmasters = stm_data->sw_end - stm_data->sw_start;
> + stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
> + if (!stm)
> + return -ENOMEM;
> +
> + stm->major = register_chrdev(0, stm_data->name, &stm_fops);
> + if (stm->major < 0)
> + goto err_free;
> +
> + device_initialize(&stm->dev);
> + stm->dev.devt = MKDEV(stm->major, 0);
> + stm->dev.class = &stm_class;
> + stm->dev.parent = parent;
> + stm->dev.release = stm_device_release;
> +
> + err = kobject_set_name(&stm->dev.kobj, "%s", stm_data->name);
> + if (err)
> + goto err_device;
> +
> + err = device_add(&stm->dev);
> + if (err)
> + goto err_device;
> +
> + spin_lock_init(&stm->link_lock);
> + INIT_LIST_HEAD(&stm->link_list);
> +
> + spin_lock_init(&stm->mc_lock);
> + mutex_init(&stm->policy_mutex);
> + stm->sw_nmasters = nmasters;
> + stm->owner = owner;
> + stm->data = stm_data;
> + stm_data->stm = stm;
> +
> + return 0;
> +
> +err_device:
> + put_device(&stm->dev);
> +err_free:
> + kfree(stm);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(stm_register_device);
> +
> +static void __stm_source_link_drop(struct stm_source_device *src,
> + struct stm_device *stm);
> +
> +void stm_unregister_device(struct stm_data *stm_data)
> +{
> + struct stm_device *stm = stm_data->stm;
> + struct stm_source_device *src, *iter;
> + int i;
> +
> + spin_lock(&stm->link_lock);
> + list_for_each_entry_safe(src, iter, &stm->link_list, link_entry) {
> + __stm_source_link_drop(src, stm);
> + }
> + spin_unlock(&stm->link_lock);
> +
> + synchronize_srcu(&stm_source_srcu);
> +
> + unregister_chrdev(stm->major, stm_data->name);
> +
> + mutex_lock(&stm->policy_mutex);
> + if (stm->policy)
> + stp_policy_unbind(stm->policy);
> + mutex_unlock(&stm->policy_mutex);
> +
> + for (i = 0; i < stm->sw_nmasters; i++)
> + stp_master_free(stm, i);
> +
> + device_unregister(&stm->dev);
> + stm_data->stm = NULL;
> +}
> +EXPORT_SYMBOL_GPL(stm_unregister_device);
> +
> +/**
> + * stm_source_link_add() - connect an stm_source device to an stm device
> + * @src: stm_source device
> + * @stm: stm device
> + *
> + * This function establishes a link from stm_source to an stm device so that
> + * the former can send out trace data to the latter.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int stm_source_link_add(struct stm_source_device *src,
> + struct stm_device *stm)
> +{
> + char *id;
> + int err;
> +
> + spin_lock(&stm->link_lock);
> + spin_lock(&src->link_lock);
> +
> + /* src->link is dereferenced under stm_source_srcu but not the list */
> + rcu_assign_pointer(src->link, stm);
> + list_add_tail(&src->link_entry, &stm->link_list);
> +
> + spin_unlock(&src->link_lock);
> + spin_unlock(&stm->link_lock);
> +
> + id = kstrdup(src->data->name, GFP_KERNEL);
> + if (id) {
> + src->policy_node =
> + stp_policy_node_lookup(stm, id);
> +
> + kfree(id);
> + }
> +
> + err = stm_output_assign(stm, src->data->nr_chans,
> + src->policy_node, &src->output);
> +
> + if (src->policy_node)
> + stp_policy_node_put(src->policy_node);
> +
> + if (err)
> + goto fail_detach;
> +
> + /* this is to notify the STM device that a new link has been made */
> + if (stm->data->link)
> + err = stm->data->link(stm->data, src->output.master,
> + src->output.channel);
> +
> + if (err)
> + goto fail_free_output;
> +
> + /* this is to let the source carry out all necessary preparations */
> + if (src->data->link)
> + src->data->link(src->data);
> +
> + return 0;
> +
> +fail_free_output:
> + stm_output_free(stm, &src->output);
> + stm_put_device(stm);
> +
> +fail_detach:
> + spin_lock(&stm->link_lock);
> + spin_lock(&src->link_lock);
> +
> + rcu_assign_pointer(src->link, NULL);
> + list_del_init(&src->link_entry);
> +
> + spin_unlock(&src->link_lock);
> + spin_unlock(&stm->link_lock);
> +
> + return err;
> +}
> +
> +/**
> + * __stm_source_link_drop() - detach stm_source from an stm device
> + * @src: stm_source device
> + * @stm: stm device
> + *
> + * If @stm is @src::link, disconnect them from one another and put the
> + * reference on the @stm device.
> + *
> + * Caller must hold stm::link_lock.
> + */
> +static void __stm_source_link_drop(struct stm_source_device *src,
> + struct stm_device *stm)
> +{
> + spin_lock(&src->link_lock);
> + if (WARN_ON_ONCE(src->link != stm)) {
> + spin_unlock(&src->link_lock);
> + return;
> + }
> +
> + stm_output_free(src->link, &src->output);
> + /* caller must hold stm::link_lock */
> + list_del_init(&src->link_entry);
> + /* matches stm_find_device() from stm_source_link_store() */
> + stm_put_device(src->link);
> + rcu_assign_pointer(src->link, NULL);
> +
> + spin_unlock(&src->link_lock);
> +}
> +
> +/**
> + * stm_source_link_drop() - detach stm_source from its stm device
> + * @src: stm_source device
> + *
> + * Unlinking means disconnecting from source's STM device; after this
> + * writes will be unsuccessful until it is linked to a new STM device.
> + *
> + * This will happen on "stm_source_link" sysfs attribute write to undo
> + * the existing link (if any), or on linked STM device's de-registration.
> + */
> +static void stm_source_link_drop(struct stm_source_device *src)
> +{
> + struct stm_device *stm;
> + int idx;
> +
> + idx = srcu_read_lock(&stm_source_srcu);
> + stm = srcu_dereference(src->link, &stm_source_srcu);
> +
> + if (stm) {
> + if (src->data->unlink)
> + src->data->unlink(src->data);
> +
> + spin_lock(&stm->link_lock);
> + __stm_source_link_drop(src, stm);
> + spin_unlock(&stm->link_lock);
> + }
> +
> + srcu_read_unlock(&stm_source_srcu, idx);
> +}
> +
> +static ssize_t stm_source_link_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct stm_source_device *src = to_stm_source_device(dev);
> + struct stm_device *stm;
> + int idx, ret;
> +
> + idx = srcu_read_lock(&stm_source_srcu);
> + stm = srcu_dereference(src->link, &stm_source_srcu);
> + ret = sprintf(buf, "%s\n",
> + stm ? dev_name(&stm->dev) : "<none>");
> + srcu_read_unlock(&stm_source_srcu, idx);
> +
> + return ret;
> +}
> +
> +static ssize_t stm_source_link_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct stm_source_device *src = to_stm_source_device(dev);
> + struct stm_device *link;
> + int err;
> +
> + stm_source_link_drop(src);
> +
> + link = stm_find_device(buf);
> + if (!link)
> + return -EINVAL;
> +
> + err = stm_source_link_add(src, link);
> + if (err)
> + stm_put_device(link);
> +
> + return err ? : count;
> +}
> +
> +static DEVICE_ATTR_RW(stm_source_link);
> +
> +static struct attribute *stm_source_attrs[] = {
> + &dev_attr_stm_source_link.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(stm_source);
> +
> +static struct class stm_source_class = {
> + .name = "stm_source",
> + .dev_groups = stm_source_groups,
> +};
> +
> +static void stm_source_device_release(struct device *dev)
> +{
> + struct stm_source_device *src = to_stm_source_device(dev);
> +
> + kfree(src);
> +}
> +
> +/**
> + * stm_source_register_device() - register an stm_source device
> + * @parent: parent device
> + * @data: device description structure
> + *
> + * This will create a device of stm_source class that can write
> + * data to an stm device once linked.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int stm_source_register_device(struct device *parent,
> + struct stm_source_data *data)
> +{
> + struct stm_source_device *src;
> + int err;
> +
> + if (!stm_core_up)
> + return -EPROBE_DEFER;
> +
> + src = kzalloc(sizeof(*src), GFP_KERNEL);
> + if (!src)
> + return -ENOMEM;
> +
> + device_initialize(&src->dev);
> + src->dev.class = &stm_source_class;
> + src->dev.parent = parent;
> + src->dev.release = stm_source_device_release;
> +
> + err = kobject_set_name(&src->dev.kobj, "%s", data->name);
> + if (err)
> + goto err;
> +
> + err = device_add(&src->dev);
> + if (err)
> + goto err;
> +
> + spin_lock_init(&src->link_lock);
> + INIT_LIST_HEAD(&src->link_entry);
> + src->data = data;
> + data->src = src;
> +
> + return 0;
> +
> +err:
> + put_device(&src->dev);
> + kfree(src);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(stm_source_register_device);
> +
> +/**
> + * stm_source_unregister_device() - unregister an stm_source device
> + * @data: device description that was used to register the device
> + *
> + * This will remove a previously created stm_source device from the system.
> + */
> +void stm_source_unregister_device(struct stm_source_data *data)
> +{
> + struct stm_source_device *src = data->src;
> +
> + stm_source_link_drop(src);
> +
> + device_destroy(&stm_source_class, src->dev.devt);
> +}
> +EXPORT_SYMBOL_GPL(stm_source_unregister_device);
> +
> +int stm_source_write(struct stm_source_data *data, unsigned int chan,
> + const char *buf, size_t count)
> +{
> + struct stm_source_device *src = data->src;
> + struct stm_device *stm;
> + int idx;
> +
> + if (!src->output.nr_chans)
> + return -ENODEV;
> +
> + if (chan >= src->output.nr_chans)
> + return -EINVAL;
> +
> + idx = srcu_read_lock(&stm_source_srcu);
> +
> + stm = srcu_dereference(src->link, &stm_source_srcu);
> + if (stm)
> + stm_write(stm->data, src->output.master,
> + src->output.channel + chan,
> + buf, count);
> + else
> + count = -ENODEV;
> +
> + srcu_read_unlock(&stm_source_srcu, idx);
> +
> + return count;
> +}
> +EXPORT_SYMBOL_GPL(stm_source_write);
> +
> +static int __init stm_core_init(void)
> +{
> + int err;
> +
> + err = class_register(&stm_class);
> + if (err)
> + return err;
> +
> + err = class_register(&stm_source_class);
> + if (err)
> + goto err_stm;
> +
> + err = stp_configfs_init();
> + if (err)
> + goto err_src;
> +
> + init_srcu_struct(&stm_source_srcu);
> +
> + stm_core_up++;
> +
> + return 0;
> +
> +err_src:
> + class_unregister(&stm_source_class);
> +err_stm:
> + class_unregister(&stm_class);
> +
> + return err;
> +}
> +
> +module_init(stm_core_init);
> +
> +static void __exit stm_core_exit(void)
> +{
> + cleanup_srcu_struct(&stm_source_srcu);
> + class_unregister(&stm_source_class);
> + class_unregister(&stm_class);
> + stp_configfs_exit();
> +}
> +
> +module_exit(stm_core_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("System Trace Module device class");
> +MODULE_AUTHOR("Alexander Shishkin <alexander.shishkin@linux.intel.com>");
> diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c
> new file mode 100644
> index 0000000000..6498a9dbb7
> --- /dev/null
> +++ b/drivers/hwtracing/stm/policy.c
> @@ -0,0 +1,529 @@
> +/*
> + * System Trace Module (STM) master/channel allocation policy management
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * A master/channel allocation policy allows mapping string identifiers to
> + * master and channel ranges, where allocation can be done.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/configfs.h>
> +#include <linux/slab.h>
> +#include <linux/stm.h>
> +#include "stm.h"
> +
> +/*
> + * STP Master/Channel allocation policy configfs layout.
> + */
> +
> +struct stp_policy {
> + struct config_group group;
> + struct stm_device *stm;
> +};
> +
> +struct stp_policy_node {
> + struct config_group group;
> + struct stp_policy *policy;
> + unsigned int first_master;
> + unsigned int last_master;
> + unsigned int first_channel;
> + unsigned int last_channel;
> +};
> +
> +static struct configfs_subsystem stp_policy_subsys;
> +
> +void stp_policy_node_get_ranges(struct stp_policy_node *policy_node,
> + unsigned int *mstart, unsigned int *mend,
> + unsigned int *cstart, unsigned int *cend)
> +{
> + *mstart = policy_node->first_master;
> + *mend = policy_node->last_master;
> + *cstart = policy_node->first_channel;
> + *cend = policy_node->last_channel;
> +}
> +
> +static inline char *stp_policy_node_name(struct stp_policy_node *policy_node)
> +{
> + return policy_node->group.cg_item.ci_name ? : "<none>";
> +}
> +
> +static inline struct stp_policy *to_stp_policy(struct config_item *item)
> +{
> + return item ?
> + container_of(to_config_group(item), struct stp_policy, group) :
> + NULL;
> +}
> +
> +static inline struct stp_policy_node *
> +to_stp_policy_node(struct config_item *item)
> +{
> + return item ?
> + container_of(to_config_group(item), struct stp_policy_node,
> + group) :
> + NULL;
> +}
> +
> +static ssize_t stp_policy_node_masters_show(struct stp_policy_node *policy_node,
> + char *page)
> +{
> + ssize_t count;
> +
> + count = sprintf(page, "%u %u\n", policy_node->first_master,
> + policy_node->last_master);
> +
> + return count;
> +}
> +
> +static ssize_t
> +stp_policy_node_masters_store(struct stp_policy_node *policy_node,
> + const char *page, size_t count)
> +{
> + unsigned int first, last;
> + struct stm_device *stm;
> + char *p = (char *)page;
> + ssize_t ret = -ENODEV;
> +
> + if (sscanf(p, "%u %u", &first, &last) != 2)
> + return -EINVAL;
> +
> + mutex_lock(&stp_policy_subsys.su_mutex);
> + stm = policy_node->policy->stm;
> + if (!stm)
> + goto unlock;
> +
> + /* must be within [sw_start..sw_end], which is an inclusive range */
> + if (first > INT_MAX || last > INT_MAX || first > last ||
> + first < stm->data->sw_start ||
> + last > stm->data->sw_end) {
> + ret = -ERANGE;
> + goto unlock;
> + }
> +
> + ret = count;
> + policy_node->first_master = first;
> + policy_node->last_master = last;
> +
> +unlock:
> + mutex_unlock(&stp_policy_subsys.su_mutex);
> +
> + return ret;
> +}
> +
> +static ssize_t
> +stp_policy_node_channels_show(struct stp_policy_node *policy_node, char *page)
> +{
> + ssize_t count;
> +
> + count = sprintf(page, "%u %u\n", policy_node->first_channel,
> + policy_node->last_channel);
> +
> + return count;
> +}
> +
> +static ssize_t
> +stp_policy_node_channels_store(struct stp_policy_node *policy_node,
> + const char *page, size_t count)
> +{
> + unsigned int first, last;
> + struct stm_device *stm;
> + char *p = (char *)page;
> + ssize_t ret = -ENODEV;
> +
> + if (sscanf(p, "%u %u", &first, &last) != 2)
> + return -EINVAL;
> +
> + mutex_lock(&stp_policy_subsys.su_mutex);
> + stm = policy_node->policy->stm;
> + if (!stm)
> + goto unlock;
> +
> + if (first > INT_MAX || last > INT_MAX || first > last ||
> + last >= stm->data->sw_nchannels) {
> + ret = -ERANGE;
> + goto unlock;
> + }
> +
> + ret = count;
> + policy_node->first_channel = first;
> + policy_node->last_channel = last;
> +
> +unlock:
> + mutex_unlock(&stp_policy_subsys.su_mutex);
> +
> + return ret;
> +}
> +
> +static void stp_policy_node_release(struct config_item *item)
> +{
> + kfree(to_stp_policy_node(item));
> +}
> +
> +struct stp_policy_node_attribute {
> + struct configfs_attribute attr;
> + ssize_t (*show)(struct stp_policy_node *, char *);
> + ssize_t (*store)(struct stp_policy_node *, const char *, size_t);
> +};
> +
> +static ssize_t stp_policy_node_attr_show(struct config_item *item,
> + struct configfs_attribute *attr,
> + char *page)
> +{
> + struct stp_policy_node *policy_node = to_stp_policy_node(item);
> + struct stp_policy_node_attribute *pn_attr =
> + container_of(attr, struct stp_policy_node_attribute, attr);
> + ssize_t count = 0;
> +
> + if (pn_attr->show)
> + count = pn_attr->show(policy_node, page);
> +
> + return count;
> +}
> +
> +static ssize_t stp_policy_node_attr_store(struct config_item *item,
> + struct configfs_attribute *attr,
> + const char *page, size_t len)
> +{
> + struct stp_policy_node *policy_node = to_stp_policy_node(item);
> + struct stp_policy_node_attribute *pn_attr =
> + container_of(attr, struct stp_policy_node_attribute, attr);
> + ssize_t count = -EINVAL;
> +
> + if (pn_attr->store)
> + count = pn_attr->store(policy_node, page, len);
> +
> + return count;
> +}
> +
> +static struct configfs_item_operations stp_policy_node_item_ops = {
> + .release = stp_policy_node_release,
> + .show_attribute = stp_policy_node_attr_show,
> + .store_attribute = stp_policy_node_attr_store,
> +};
> +
> +static struct stp_policy_node_attribute stp_policy_node_attr_range = {
> + .attr = {
> + .ca_owner = THIS_MODULE,
> + .ca_name = "masters",
> + .ca_mode = S_IRUGO | S_IWUSR,
> + },
> + .show = stp_policy_node_masters_show,
> + .store = stp_policy_node_masters_store,
> +};
> +
> +static struct stp_policy_node_attribute stp_policy_node_attr_channels = {
> + .attr = {
> + .ca_owner = THIS_MODULE,
> + .ca_name = "channels",
> + .ca_mode = S_IRUGO | S_IWUSR,
> + },
> + .show = stp_policy_node_channels_show,
> + .store = stp_policy_node_channels_store,
> +};
> +
> +static struct configfs_attribute *stp_policy_node_attrs[] = {
> + &stp_policy_node_attr_range.attr,
> + &stp_policy_node_attr_channels.attr,
> + NULL,
> +};
> +
> +static struct config_item_type stp_policy_type;
> +static struct config_item_type stp_policy_node_type;
> +
> +static struct config_group *
> +stp_policy_node_make(struct config_group *group, const char *name)
> +{
> + struct stp_policy_node *policy_node, *parent_node;
> + struct stp_policy *policy;
> +
> + if (group->cg_item.ci_type == &stp_policy_type) {
> + policy = container_of(group, struct stp_policy, group);
> + } else {
> + parent_node = container_of(group, struct stp_policy_node,
> + group);
> + policy = parent_node->policy;
> + }
> +
> + if (!policy->stm)
> + return ERR_PTR(-ENODEV);
> +
> + policy_node = kzalloc(sizeof(struct stp_policy_node), GFP_KERNEL);
> + if (!policy_node)
> + return ERR_PTR(-ENOMEM);
> +
> + config_group_init_type_name(&policy_node->group, name,
> + &stp_policy_node_type);
> +
> + policy_node->policy = policy;
> +
> + /* default values for the attributes */
> + policy_node->first_master = policy->stm->data->sw_start;
> + policy_node->last_master = policy->stm->data->sw_end;
> + policy_node->first_channel = 0;
> + policy_node->last_channel = policy->stm->data->sw_nchannels - 1;
> +
> + return &policy_node->group;
> +}
> +
> +static void
> +stp_policy_node_drop(struct config_group *group, struct config_item *item)
> +{
> + config_item_put(item);
> +}
> +
> +static struct configfs_group_operations stp_policy_node_group_ops = {
> + .make_group = stp_policy_node_make,
> + .drop_item = stp_policy_node_drop,
> +};
> +
> +static struct config_item_type stp_policy_node_type = {
> + .ct_item_ops = &stp_policy_node_item_ops,
> + .ct_group_ops = &stp_policy_node_group_ops,
> + .ct_attrs = stp_policy_node_attrs,
> + .ct_owner = THIS_MODULE,
> +};
> +
> +/*
> + * Root group: policies.
> + */
> +static struct configfs_attribute stp_policy_attr_device = {
> + .ca_owner = THIS_MODULE,
> + .ca_name = "device",
> + .ca_mode = S_IRUGO,
> +};
> +
> +static struct configfs_attribute *stp_policy_attrs[] = {
> + &stp_policy_attr_device,
> + NULL,
> +};
> +
> +static ssize_t stp_policy_attr_show(struct config_item *item,
> + struct configfs_attribute *attr,
> + char *page)
> +{
> + struct stp_policy *policy = to_stp_policy(item);
> + ssize_t count;
> +
> + count = sprintf(page, "%s\n",
> + (policy && policy->stm) ?
> + policy->stm->data->name :
> + "<none>");
> +
> + return count;
> +}
> +
> +void stp_policy_unbind(struct stp_policy *policy)
> +{
> + struct stm_device *stm = policy->stm;
> +
> + if (WARN_ON_ONCE(!policy->stm))
> + return;
> +
> + mutex_lock(&stm->policy_mutex);
> + stm->policy = NULL;
> + mutex_unlock(&stm->policy_mutex);
> +
> + policy->stm = NULL;
> +
> + stm_put_device(stm);
> +}
> +
> +static void stp_policy_release(struct config_item *item)
> +{
> + struct stp_policy *policy = to_stp_policy(item);
> +
> + stp_policy_unbind(policy);
> + kfree(policy);
> +}
> +
> +static struct configfs_item_operations stp_policy_item_ops = {
> + .release = stp_policy_release,
> + .show_attribute = stp_policy_attr_show,
> +};
> +
> +static struct configfs_group_operations stp_policy_group_ops = {
> + .make_group = stp_policy_node_make,
> +};
> +
> +static struct config_item_type stp_policy_type = {
> + .ct_item_ops = &stp_policy_item_ops,
> + .ct_group_ops = &stp_policy_group_ops,
> + .ct_attrs = stp_policy_attrs,
> + .ct_owner = THIS_MODULE,
> +};
> +
> +static struct config_group *
> +stp_policies_make(struct config_group *group, const char *name)
> +{
> + struct config_group *ret;
> + struct stm_device *stm;
> + char *devname, *p;
> +
> + devname = kasprintf(GFP_KERNEL, "%s", name);
> + if (!devname)
> + return ERR_PTR(-ENOMEM);
> +
> + /*
> + * node must look like <device_name>.<policy_name>, where
> + * <device_name> is the name of an existing stm device and
> + * <policy_name> is an arbitrary string
> + */
> + p = strchr(devname, '.');
> + if (!p) {
> + kfree(devname);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + *p++ = '\0';
> +
> + stm = stm_find_device(devname);
> + kfree(devname);
> +
> + if (!stm)
> + return ERR_PTR(-ENODEV);
> +
> + mutex_lock(&stm->policy_mutex);
> + if (stm->policy) {
> + ret = ERR_PTR(-EBUSY);
> + goto unlock_policy;
> + }
> +
> + stm->policy = kzalloc(sizeof(*stm->policy), GFP_KERNEL);
> + if (!stm->policy) {
> + ret = ERR_PTR(-ENOMEM);
> + goto unlock_policy;
> + }
> +
> + config_group_init_type_name(&stm->policy->group, name,
> + &stp_policy_type);
> + stm->policy->stm = stm;
> +
> + ret = &stm->policy->group;
> +
> +unlock_policy:
> + mutex_unlock(&stm->policy_mutex);
> +
> + if (IS_ERR(ret))
> + stm_put_device(stm);
> +
> + return ret;
> +}
> +
> +static struct configfs_group_operations stp_policies_group_ops = {
> + .make_group = stp_policies_make,
> +};
> +
> +static struct config_item_type stp_policies_type = {
> + .ct_group_ops = &stp_policies_group_ops,
> + .ct_owner = THIS_MODULE,
> +};
> +
> +static struct configfs_subsystem stp_policy_subsys = {
> + .su_group = {
> + .cg_item = {
> + .ci_namebuf = "stp-policy",
> + .ci_type = &stp_policies_type,
> + },
> + },
> +};
> +
> +/*
> + * Lock the policy mutex from the outside
> + */
> +static struct stp_policy_node *
> +__stp_policy_node_lookup(struct stp_policy *policy, char *s)
> +{
> + struct stp_policy_node *policy_node, *ret;
> + struct list_head *head = &policy->group.cg_children;
> + struct config_item *item;
> + char *start, *end = s;
> +
> + if (list_empty(head))
> + return NULL;
> +
> + /* return the first entry if everything else fails */
> + item = list_entry(head->next, struct config_item, ci_entry);
> + ret = to_stp_policy_node(item);
> +
> +next:
> + for (;;) {
> + start = strsep(&end, "/");
> + if (!start)
> + break;
> +
> + if (!*start)
> + continue;
> +
> + list_for_each_entry(item, head, ci_entry) {
> + policy_node = to_stp_policy_node(item);
> +
> + if (!strcmp(start,
> + policy_node->group.cg_item.ci_name)) {
> + ret = policy_node;
> +
> + if (!end)
> + goto out;
> +
> + head = &policy_node->group.cg_children;
> + goto next;
> + }
> + }
> + break;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +
> +struct stp_policy_node *
> +stp_policy_node_lookup(struct stm_device *stm, char *s)
> +{
> + struct stp_policy_node *policy_node = NULL;
> +
> + mutex_lock(&stp_policy_subsys.su_mutex);
> +
> + mutex_lock(&stm->policy_mutex);
> + if (stm->policy)
> + policy_node = __stp_policy_node_lookup(stm->policy, s);
> + mutex_unlock(&stm->policy_mutex);
> +
> + if (policy_node)
> + config_item_get(&policy_node->group.cg_item);
> + mutex_unlock(&stp_policy_subsys.su_mutex);
> +
> + return policy_node;
> +}
> +
> +void stp_policy_node_put(struct stp_policy_node *policy_node)
> +{
> + config_item_put(&policy_node->group.cg_item);
> +}
> +
> +int __init stp_configfs_init(void)
> +{
> + int err;
> +
> + config_group_init(&stp_policy_subsys.su_group);
> + mutex_init(&stp_policy_subsys.su_mutex);
> + err = configfs_register_subsystem(&stp_policy_subsys);
> +
> + return err;
One small suggestion, is it better like this:
return configfs_register_subsystem(&stp_policy_subsys);
As such, we don't need the local variable "err" any more.
> +}
> +
> +void __exit stp_configfs_exit(void)
> +{
> + configfs_unregister_subsystem(&stp_policy_subsys);
> +}
> diff --git a/drivers/hwtracing/stm/stm.h b/drivers/hwtracing/stm/stm.h
> new file mode 100644
> index 0000000000..cf33bf976a
> --- /dev/null
> +++ b/drivers/hwtracing/stm/stm.h
> @@ -0,0 +1,87 @@
> +/*
> + * System Trace Module (STM) infrastructure
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * STM class implements generic infrastructure for System Trace Module devices
> + * as defined in MIPI STPv2 specification.
> + */
> +
> +#ifndef _STM_STM_H_
> +#define _STM_STM_H_
> +
> +struct stp_policy;
> +struct stp_policy_node;
> +
> +struct stp_policy_node *
> +stp_policy_node_lookup(struct stm_device *stm, char *s);
> +void stp_policy_node_put(struct stp_policy_node *policy_node);
> +void stp_policy_unbind(struct stp_policy *policy);
> +
> +void stp_policy_node_get_ranges(struct stp_policy_node *policy_node,
> + unsigned int *mstart, unsigned int *mend,
> + unsigned int *cstart, unsigned int *cend);
> +int stp_configfs_init(void);
> +void stp_configfs_exit(void);
> +
> +struct stp_master {
> + unsigned int nr_free;
> + unsigned long chan_map[0];
> +};
> +
> +struct stm_device {
> + struct device dev;
> + struct module *owner;
> + struct stp_policy *policy;
> + struct mutex policy_mutex;
> + int major;
> + unsigned int sw_nmasters;
> + struct stm_data *data;
> + spinlock_t link_lock;
> + struct list_head link_list;
> + /* master allocation */
> + spinlock_t mc_lock;
> + struct stp_master *masters[0];
> +};
> +
> +#define to_stm_device(_d) \
> + container_of((_d), struct stm_device, dev)
> +
> +struct stm_output {
> + unsigned int master;
> + unsigned int channel;
> + unsigned int nr_chans;
> +};
> +
> +struct stm_file {
> + struct stm_device *stm;
> + struct stp_policy_node *policy_node;
> + struct stm_output output;
> +};
> +
> +struct stm_device *stm_find_device(const char *name);
> +void stm_put_device(struct stm_device *stm);
> +
> +struct stm_source_device {
> + struct device dev;
> + struct stm_source_data *data;
> + spinlock_t link_lock;
> + struct stm_device *link;
> + struct list_head link_entry;
> + /* one output per stm_source device */
> + struct stp_policy_node *policy_node;
> + struct stm_output output;
> +};
> +
> +#define to_stm_source_device(_d) \
> + container_of((_d), struct stm_source_device, dev)
> +
> +#endif /* _STM_STM_H_ */
> diff --git a/include/linux/stm.h b/include/linux/stm.h
> new file mode 100644
> index 0000000000..9d0083d364
> --- /dev/null
> +++ b/include/linux/stm.h
> @@ -0,0 +1,126 @@
> +/*
> + * System Trace Module (STM) infrastructure apis
> + * Copyright (C) 2014 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#ifndef _STM_H_
> +#define _STM_H_
> +
> +#include <linux/device.h>
> +
> +/**
> + * enum stp_packet_type - STP packets that an STM driver sends
> + */
> +enum stp_packet_type {
> + STP_PACKET_DATA = 0,
> + STP_PACKET_FLAG,
> + STP_PACKET_USER,
> + STP_PACKET_MERR,
> + STP_PACKET_GERR,
> + STP_PACKET_TRIG,
> + STP_PACKET_XSYNC,
> +};
> +
> +/**
> + * enum stp_packet_flags - STP packet modifiers
> + */
> +enum stp_packet_flags {
> + STP_PACKET_MARKED = 0x1,
> + STP_PACKET_TIMESTAMPED = 0x2,
> +};
> +
> +struct stp_policy;
> +
> +struct stm_device;
> +
> +/**
> + * struct stm_data - STM device description and callbacks
> + * @name: device name
> + * @stm: internal structure, only used by stm class code
> + * @sw_start: first STP master available to software
> + * @sw_end: last STP master available to software
> + * @sw_nchannels: number of STP channels per master
> + * @sw_mmiosz: size of one channel's IO space, for mmap, optional
> + * @packet: callback that sends an STP packet
> + * @mmio_addr: mmap callback, optional
> + * @link: called when a new stm_source gets linked to us, optional
> + * @unlink: likewise for unlinking, again optional
> + * @set_options: set device-specific options on a channel
> + *
> + * Fill out this structure before calling stm_register_device() to create
> + * an STM device and stm_unregister_device() to destroy it. It will also be
> + * passed back to @packet(), @mmio_addr(), @link(), @unlink() and @set_options()
> + * callbacks.
> + *
> + * Normally, an STM device will have a range of masters available to software
> + * and the rest being statically assigned to various hardware trace sources.
> + * The former is defined by the the range [@sw_start..@sw_end] of the device
> + * description. That is, the lowest master that can be allocated to software
> + * writers is @sw_start and data from this writer will appear is @sw_start
> + * master in the STP stream.
> + */
> +struct stm_data {
> + const char *name;
> + struct stm_device *stm;
> + unsigned int sw_start;
> + unsigned int sw_end;
> + unsigned int sw_nchannels;
> + unsigned int sw_mmiosz;
> + ssize_t (*packet)(struct stm_data *, unsigned int,
> + unsigned int, unsigned int,
> + unsigned int, unsigned int,
> + const unsigned char *);
> + phys_addr_t (*mmio_addr)(struct stm_data *, unsigned int,
> + unsigned int, unsigned int);
> + int (*link)(struct stm_data *, unsigned int,
> + unsigned int);
> + void (*unlink)(struct stm_data *, unsigned int,
> + unsigned int);
> + long (*set_options)(struct stm_data *, unsigned int,
> + unsigned int, unsigned int,
> + unsigned long);
> +};
> +
> +int stm_register_device(struct device *parent, struct stm_data *stm_data,
> + struct module *owner);
> +void stm_unregister_device(struct stm_data *stm_data);
> +
> +struct stm_source_device;
> +
> +/**
> + * struct stm_source_data - STM source device description and callbacks
> + * @name: device name, will be used for policy lookup
> + * @src: internal structure, only used by stm class code
> + * @nr_chans: number of channels to allocate
> + * @link: called when this source gets linked to an STM device
> + * @unlink: called when this source is about to get unlinked from its STM
> + *
> + * Fill in this structure before calling stm_source_register_device() to
> + * register a source device. Also pass it to unregister and write calls.
> + */
> +struct stm_source_data {
> + const char *name;
> + struct stm_source_device *src;
> + unsigned int percpu;
> + unsigned int nr_chans;
> + int (*link)(struct stm_source_data *data);
> + void (*unlink)(struct stm_source_data *data);
> +};
> +
> +int stm_source_register_device(struct device *parent,
> + struct stm_source_data *data);
> +void stm_source_unregister_device(struct stm_source_data *data);
> +
> +int stm_source_write(struct stm_source_data *data, unsigned int chan,
> + const char *buf, size_t count);
> +
> +#endif /* _STM_H_ */
> diff --git a/include/uapi/linux/stm.h b/include/uapi/linux/stm.h
> new file mode 100644
> index 0000000000..626a8d3f63
> --- /dev/null
> +++ b/include/uapi/linux/stm.h
> @@ -0,0 +1,50 @@
> +/*
> + * System Trace Module (STM) userspace interfaces
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * STM class implements generic infrastructure for System Trace Module devices
> + * as defined in MIPI STPv2 specification.
> + */
> +
> +#ifndef _UAPI_LINUX_STM_H
> +#define _UAPI_LINUX_STM_H
> +
> +#include <linux/types.h>
> +
> +/**
> + * struct stp_policy_id - identification for the STP policy
> + * @size: size of the structure including real id[] length
> + * @master: assigned master
> + * @channel: first assigned channel
> + * @width: number of requested channels
> + * @id: identification string
> + *
> + * User must calculate the total size of the structure and put it into
> + * @size field, fill out the @id and desired @width. In return, kernel
> + * fills out @master, @channel and @width.
> + */
> +struct stp_policy_id {
> + __u32 size;
> + __u16 master;
> + __u16 channel;
> + __u16 width;
> + /* padding */
> + __u16 __reserved_0;
> + __u32 __reserved_1;
> + char id[0];
> +};
> +
> +#define STP_POLICY_ID_SET _IOWR('%', 0, struct stp_policy_id)
> +#define STP_POLICY_ID_GET _IOR('%', 1, struct stp_policy_id)
> +#define STP_SET_OPTIONS _IOW('%', 2, __u64)
> +
> +#endif /* _UAPI_LINUX_STM_H */
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH v3 01/11] stm class: Introduce an abstraction for System Trace Module devices
From: Alexander Shishkin @ 2015-07-08 12:49 UTC (permalink / raw)
To: Chunyan Zhang
Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org, mathieu.poirier,
peter.lachner, norbert.schulz, keven.boell, yann.fouassier,
laurent.fert, linux-api@vger.kernel.org
In-Reply-To: <CAAfSe-vDUdA+kY04-u-SXT-pdy_hn1hY1yG6-e-k_tbLsFzjKQ@mail.gmail.com>
Chunyan Zhang <zhang.lyra@gmail.com> writes:
>> +int __init stp_configfs_init(void)
>> +{
>> + int err;
>> +
>> + config_group_init(&stp_policy_subsys.su_group);
>> + mutex_init(&stp_policy_subsys.su_mutex);
>> + err = configfs_register_subsystem(&stp_policy_subsys);
>> +
>> + return err;
>
> One small suggestion, is it better like this:
> return configfs_register_subsystem(&stp_policy_subsys);
> As such, we don't need the local variable "err" any more.
You are right, err doesn't serve any purpose here.
Thanks,
--
Alex
^ permalink raw reply
* Re: [PATCH V3 0/5] Allow user to request memory to be locked on page fault
From: Eric B Munson @ 2015-07-08 13:23 UTC (permalink / raw)
To: Andrew Morton
Cc: Shuah Khan, Michal Hocko, Michael Kerrisk, Vlastimil Babka,
linux-alpha, linux-kernel, linux-mips, linux-parisc, linuxppc-dev,
sparclinux, linux-xtensa, linux-mm, linux-arch, linux-api
In-Reply-To: <20150707141613.f945c98279dcb71c9743d5f2@linux-foundation.org>
[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]
On Tue, 07 Jul 2015, Andrew Morton wrote:
> On Tue, 7 Jul 2015 13:03:38 -0400 Eric B Munson <emunson@akamai.com> wrote:
>
> > mlock() allows a user to control page out of program memory, but this
> > comes at the cost of faulting in the entire mapping when it is
> > allocated. For large mappings where the entire area is not necessary
> > this is not ideal. Instead of forcing all locked pages to be present
> > when they are allocated, this set creates a middle ground. Pages are
> > marked to be placed on the unevictable LRU (locked) when they are first
> > used, but they are not faulted in by the mlock call.
> >
> > This series introduces a new mlock() system call that takes a flags
> > argument along with the start address and size. This flags argument
> > gives the caller the ability to request memory be locked in the
> > traditional way, or to be locked after the page is faulted in. New
> > calls are added for munlock() and munlockall() which give the called a
> > way to specify which flags are supposed to be cleared. A new MCL flag
> > is added to mirror the lock on fault behavior from mlock() in
> > mlockall(). Finally, a flag for mmap() is added that allows a user to
> > specify that the covered are should not be paged out, but only after the
> > memory has been used the first time.
>
> Thanks for sticking with this. Adding new syscalls is a bit of a
> hassle but I do think we end up with a better interface - the existing
> mlock/munlock/mlockall interfaces just aren't appropriate for these
> things.
>
> I don't know whether these syscalls should be documented via new
> manpages, or if we should instead add them to the existing
> mlock/munlock/mlockall manpages. Michael, could you please advise?
>
Thanks for adding the series. I owe you several updates (getting the
new syscall right for all architectures and a set of tests for the new
syscalls). Would you prefer a new pair of patches or I update this set?
Eric
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
From: Stephen Smalley @ 2015-07-08 13:37 UTC (permalink / raw)
To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
Serge E. Hallyn, Kees Cook, Tetsuo Handa, Neil Brown, Mark Rustad,
Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
Shuah Khan, Al Viro, linux-security-module, linux-kernel,
linux-api
Cc: Karol Lewandowski, Lukasz Skalski
In-Reply-To: <1436351110-5902-6-git-send-email-p.osmialowsk@samsung.com>
On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
> Originates from:
>
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: aa0885489d19be92fa41c6f0a71df28763228a40
>
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
> ipc/kdbus/bus.c | 12 ++++++++++-
> ipc/kdbus/bus.h | 3 +++
> ipc/kdbus/connection.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
> ipc/kdbus/connection.h | 4 ++++
> ipc/kdbus/domain.c | 9 ++++++++-
> ipc/kdbus/domain.h | 2 ++
> ipc/kdbus/endpoint.c | 11 ++++++++++
> ipc/kdbus/names.c | 11 ++++++++++
> ipc/kdbus/queue.c | 30 ++++++++++++++++++----------
> 9 files changed, 124 insertions(+), 12 deletions(-)
>
>
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index 9993753..b85cdc7 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -31,6 +31,7 @@
> #include <linux/slab.h>
> #include <linux/syscalls.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "connection.h"
> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
> bool is_activator;
> bool is_monitor;
> struct kvec kvec;
> + u32 sid, len;
> + char *label;
> int ret;
>
> struct {
> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
> }
> }
>
> + security_task_getsecid(current, &sid);
> + security_secid_to_secctx(sid, &label, &len);
> + ret = security_kdbus_connect(conn, label, len);
> + if (ret) {
> + ret = -EPERM;
> + goto exit_unref;
> + }
This seems convoluted and expensive. If you always want the label of
the current task here, then why not just have security_kdbus_connect()
internally extract the label of the current task?
> @@ -1107,6 +1119,12 @@ static int kdbus_conn_reply(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
> if (ret < 0)
> goto exit;
>
> + ret = security_kdbus_talk(src, dst);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
Where does kdbus apply its uid-based or other restrictions on
connections? Why do we need to insert separate hooks into each of these
functions? Is there no central chokepoint already for permission
checking that we can hook?
> diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
> index d1ffe90..1f91d39 100644
> --- a/ipc/kdbus/connection.h
> +++ b/ipc/kdbus/connection.h
> @@ -19,6 +19,7 @@
> #include <linux/kref.h>
> #include <linux/lockdep.h>
> #include <linux/path.h>
> +#include <uapi/linux/kdbus.h>
>
> #include "limits.h"
> #include "metadata.h"
> @@ -73,6 +74,7 @@ struct kdbus_kmsg;
> * @names_queue_list: Well-known names this connection waits for
> * @privileged: Whether this connection is privileged on the bus
> * @faked_meta: Whether the metadata was faked on HELLO
> + * @security: LSM security blob
> */
> struct kdbus_conn {
> struct kref kref;
> @@ -113,6 +115,8 @@ struct kdbus_conn {
>
> bool privileged:1;
> bool faked_meta:1;
> +
> + void *security;
> };
Unless I missed it, you may have missed the most important thing of all:
controlling kdbus's notion of "privileged". kdbus sets privileged to
true if the process has CAP_IPC_OWNER or the process euid matches the
uid of the bus creator, and then it allows those processes to do many
dangerous things, including monitoring all traffic, impersonating
credentials, pids, or seclabel, etc.
I don't believe we should ever permit impersonating seclabel information.
^ permalink raw reply
* Re: [RFC 4/8] lsm: smack: smack callbacks for kdbus security hooks
From: Stephen Smalley @ 2015-07-08 13:42 UTC (permalink / raw)
To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
Serge E. Hallyn, Kees Cook, Tetsuo Handa, Neil Brown, Mark Rustad,
Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
Shuah Khan, Al Viro, linux-security-module, linux-kernel,
linux-api
Cc: Karol Lewandowski, Lukasz Skalski
In-Reply-To: <1436351110-5902-5-git-send-email-p.osmialowsk@samsung.com>
On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
> This adds implementation of three smack callbacks sitting behind kdbus
> security hooks as proposed by Karol Lewandowski.
>
> Originates from:
>
> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
> commit: fc3505d058c001fe72a6f66b833e0be5b2d118f3
>
> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: 103c26fd27d1ec8c32d85dd3d85681f936ac66fb
>
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
> security/smack/smack_lsm.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index a143328..033b756 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -41,6 +41,7 @@
> #include <linux/msg.h>
> #include <linux/shm.h>
> #include <linux/binfmts.h>
> +#include <kdbus/connection.h>
> #include "smack.h"
>
> #define TRANS_TRUE "TRUE"
> @@ -3336,6 +3337,69 @@ static int smack_setprocattr(struct task_struct *p, char *name,
> }
>
> /**
> + * smack_kdbus_connect - Set the security blob for a KDBus connection
> + * @conn: the connection
> + * @secctx: smack label
> + * @seclen: smack label length
> + *
> + * Returns 0
> + */
> +static int smack_kdbus_connect(struct kdbus_conn *conn,
> + const char *secctx, u32 seclen)
> +{
> + struct smack_known *skp;
> +
> + if (secctx && seclen > 0)
> + skp = smk_import_entry(secctx, seclen);
> + else
> + skp = smk_of_current();
> + conn->security = skp;
> +
> + return 0;
> +}
> +
> +/**
> + * smack_kdbus_conn_free - Clear the security blob for a KDBus connection
> + * @conn: the connection
> + *
> + * Clears the blob pointer
> + */
> +static void smack_kdbus_conn_free(struct kdbus_conn *conn)
> +{
> + conn->security = NULL;
> +}
> +
> +/**
> + * smack_kdbus_talk - Smack access on KDBus
> + * @src: source kdbus connection
> + * @dst: destination kdbus connection
> + *
> + * Return 0 if a subject with the smack of sock could access
> + * an object with the smack of other, otherwise an error code
> + */
> +static int smack_kdbus_talk(const struct kdbus_conn *src,
> + const struct kdbus_conn *dst)
> +{
> + struct smk_audit_info ad;
> + struct smack_known *sskp = src->security;
> + struct smack_known *dskp = dst->security;
> + int ret;
> +
> + BUG_ON(sskp == NULL);
> + BUG_ON(dskp == NULL);
> +
> + if (smack_privileged(CAP_MAC_OVERRIDE))
> + return 0;
> +
> + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NONE);
> +
> + ret = smk_access(sskp, dskp, MAY_WRITE, &ad);
> + if (ret)
> + return ret;
> + return 0;
> +}
> +
> +/**
> * smack_unix_stream_connect - Smack access on UDS
> * @sock: one sock
> * @other: the other sock
> @@ -4393,6 +4457,10 @@ struct security_hook_list smack_hooks[] = {
> LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
> LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
> LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx),
> +
> + LSM_HOOK_INIT(kdbus_connect, smack_kdbus_connect),
> + LSM_HOOK_INIT(kdbus_conn_free, smack_kdbus_conn_free),
> + LSM_HOOK_INIT(kdbus_talk, smack_kdbus_talk),
> };
If Smack only truly needs 3 hooks, then it begs the question of why
there are so many other hooks defined. Are the other hooks just to
support finer-grained distinctions, or is Smack's coverage incomplete?
^ permalink raw reply
* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
From: Greg Kroah-Hartman @ 2015-07-08 14:13 UTC (permalink / raw)
To: Paul Osmialowski
Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown, Mark Rustad,
Daniel Mack, David Herrmann, Djalal Harouni, Shuah Khan, Al Viro,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Karol Lewandowski,
Lukasz Skalski
In-Reply-To: <1436351110-5902-6-git-send-email-p.osmialowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Wed, Jul 08, 2015 at 12:25:07PM +0200, Paul Osmialowski wrote:
> Originates from:
>
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: aa0885489d19be92fa41c6f0a71df28763228a40
That's a horrid changelog entry, what is it supposed to mean? Don't
refer to a different tree, which will not be around for forever, and
have no information here at all as to what is going on in the change.
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC 3/8] lsm: kdbus security hooks
From: Greg Kroah-Hartman @ 2015-07-08 14:14 UTC (permalink / raw)
To: Paul Osmialowski
Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown, Mark Rustad,
Daniel Mack, David Herrmann, Djalal Harouni, Shuah Khan, Al Viro,
linux-security-module, linux-kernel, linux-api, Karol Lewandowski,
Lukasz Skalski
In-Reply-To: <1436351110-5902-4-git-send-email-p.osmialowsk@samsung.com>
On Wed, Jul 08, 2015 at 12:25:05PM +0200, Paul Osmialowski wrote:
> This is combination of the work by Karol Lewandowski and Paul Moore
> on LSM hooks for kdbus.
>
> Originates from:
>
> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
> commit: 7050f206a79564886938d0edc4e1e9da5972c72d
>
> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: a9fe4c33b6e5ab25a243e0590df406aabb6add12
Again, horrid commit log entries :(
^ permalink raw reply
* Re: [RFC 8/8] kdbus: Ability to run kdbus test by executable binary name
From: Greg Kroah-Hartman @ 2015-07-08 14:16 UTC (permalink / raw)
To: Paul Osmialowski
Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown, Mark Rustad,
Daniel Mack, David Herrmann, Djalal Harouni, Shuah Khan, Al Viro,
linux-security-module, linux-kernel, linux-api, Karol Lewandowski,
Lukasz Skalski
In-Reply-To: <1436351110-5902-9-git-send-email-p.osmialowsk@samsung.com>
On Wed, Jul 08, 2015 at 12:25:10PM +0200, Paul Osmialowski wrote:
> With this applied, you can do following:
>
> $ cp kdbus-test daemon
> $ cp kdbus-test send
>
> Then run 'daemon' in one shell session:
>
> $ ./daemon --bus test
>
> ...and 'send' in another:
>
> $ ./send --bus test
>
> Useful for testing features introduced by previous patches.
>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
> tools/testing/selftests/kdbus/kdbus-test.c | 5 +++++
> 1 file changed, 5 insertions(+)
Why not create symlinks automatically so this works without having to
manually run 'cp'? But really, is spelling out the command line option
really a big issue?
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC 8/8] kdbus: Ability to run kdbus test by executable binary name
From: Paul Osmialowski @ 2015-07-08 14:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown, Mark Rustad,
Daniel Mack, David Herrmann, Djalal Harouni, Shuah Khan, Al Viro,
linux-security-module, linux-kernel, linux-api, Karol Lewandowski,
Lukasz Skalski
In-Reply-To: <20150708141608.GC10137@kroah.com>
Hi Greg,
Thanks for your comments,
On Wed, 8 Jul 2015, Greg Kroah-Hartman wrote:
>
> Why not create symlinks automatically so this works without having to
> manually run 'cp'?
It can be anything on which one can apply security label. Symlinks can do.
> But really, is spelling out the command line option
> really a big issue?
Maybe. I couldn't think about any easier (and easy to present) use case
that could explain the purpose of this change.
>
> thanks,
>
> greg k-h
>
^ permalink raw reply
* Re: [PATCH 0/24] kernel: add a netlink interface to get information about processes (v2)
From: Andrew Vagin @ 2015-07-08 16:10 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andrey Vagin,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
Oleg Nesterov, Andrew Morton, Cyrill Gorcunov, Pavel Emelyanov,
Roger Luethi, Arnd Bergmann, Arnaldo Carvalho de Melo,
David Ahern, Pavel Odintsov
In-Reply-To: <CALCETrVSRkMSAVPz9JW4XCV7DmrgkyGK54HRUrue2R756f5C=Q@mail.gmail.com>
On Tue, Jul 07, 2015 at 08:56:37AM -0700, Andy Lutomirski wrote:
> On Tue, Jul 7, 2015 at 8:43 AM, Andrew Vagin <avagin-wo1vFcy6AUs@public.gmane.org> wrote:
> > On Mon, Jul 06, 2015 at 10:10:32AM -0700, Andy Lutomirski wrote:
> >> On Mon, Jul 6, 2015 at 1:47 AM, Andrey Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org> wrote:
> >> > Currently we use the proc file system, where all information are
> >> > presented in text files, what is convenient for humans. But if we need
> >> > to get information about processes from code (e.g. in C), the procfs
> >> > doesn't look so cool.
> >> >
> >> > From code we would prefer to get information in binary format and to be
> >> > able to specify which information and for which tasks are required. Here
> >> > is a new interface with all these features, which is called task_diag.
> >> > In addition it's much faster than procfs.
> >> >
> >> > task_diag is based on netlink sockets and looks like socket-diag, which
> >> > is used to get information about sockets.
> >>
> >> I think I like this in principle, but I have can see a few potential
> >> problems with using netlink for this:
> >>
> >> 1. Netlink very naturally handles net namespaces, but it doesn't
> >> naturally handle any other kind of namespace. In fact, the taskstats
> >> code that you're building on has highly broken user and pid namespace
> >> support. (Look for some obviously useless init_user_ns and
> >> init_pid_ns references. But that's only the obvious problem. That
> >> code calls current_user_ns() and task_active_pid_ns(current) from
> >> .doit, which is, in turn, called from sys_write, and looking at
> >> current's security state from sys_write is a big no-no.)
> >>
> >> You could partially fix it by looking at f_cred's namespaces, but that
> >> would be a change of what it means to create a netlink socket, and I'm
> >> not sure that's a good idea.
> >
> > If I don't miss something, all problems around pidns and userns are
> > related with multicast functionality. task_diag is using
> > request/response scheme and doesn't send multicast packets.
>
> It has nothing to do with multicast. task_diag needs to know what
> pidns and userns to use for a request, but netlink isn't set up to
> give you any reasonably way to do that. A netlink socket is
> fundamentally tied to a *net* ns (it's a socket, after all). But you
> can send it requests using write(2), and calling current_user_ns()
> from write(2) is bad. There's a long history of bugs and
> vulnerabilities related to thinking that current_cred() and similar
> are acceptable things to use in write(2) implementations.
>
As far as I understand, socket_diag doesn't have this problem, becaus
each socket has a link on a namespace where it was created.
What if we will pin the current pidns and credentials to a task_diag
socket in a moment when it's created.
Thanks,
Andrew
^ permalink raw reply
* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
From: Casey Schaufler @ 2015-07-08 16:24 UTC (permalink / raw)
To: Paul Osmialowski, Paul Moore, James Morris, Serge E. Hallyn,
Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown, Mark Rustad,
Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
Shuah Khan, Al Viro, linux-security-module, linux-kernel,
linux-api
Cc: Karol Lewandowski, Lukasz Skalski
In-Reply-To: <1436351110-5902-6-git-send-email-p.osmialowsk@samsung.com>
On 7/8/2015 3:25 AM, Paul Osmialowski wrote:
> Originates from:
>
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: aa0885489d19be92fa41c6f0a71df28763228a40
>
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
> ipc/kdbus/bus.c | 12 ++++++++++-
> ipc/kdbus/bus.h | 3 +++
> ipc/kdbus/connection.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
> ipc/kdbus/connection.h | 4 ++++
> ipc/kdbus/domain.c | 9 ++++++++-
> ipc/kdbus/domain.h | 2 ++
> ipc/kdbus/endpoint.c | 11 ++++++++++
> ipc/kdbus/names.c | 11 ++++++++++
> ipc/kdbus/queue.c | 30 ++++++++++++++++++----------
> 9 files changed, 124 insertions(+), 12 deletions(-)
>
> diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
> index bbdf0f2..9894895 100644
> --- a/ipc/kdbus/bus.c
> +++ b/ipc/kdbus/bus.c
> @@ -22,6 +22,7 @@
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "notify.h"
> @@ -51,6 +52,7 @@ static void kdbus_bus_free(struct kdbus_node *node)
> kdbus_domain_unref(bus->domain);
> kdbus_policy_db_clear(&bus->policy_db);
> kdbus_meta_proc_unref(bus->creator_meta);
> + security_kdbus_bus_free(bus);
> kfree(bus);
> }
>
> @@ -161,6 +163,12 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
> goto exit_unref;
> }
>
> + ret = security_kdbus_bus_alloc(b);
> + if (ret) {
> + ret = -EPERM;
> + goto exit_unref;
> + }
> +
> /*
> * Bus-limits of the creator are accounted on its real UID, just like
> * all other per-user limits.
> @@ -169,11 +177,13 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
> if (IS_ERR(b->creator)) {
> ret = PTR_ERR(b->creator);
> b->creator = NULL;
> - goto exit_unref;
> + goto exit_free_security;
> }
>
> return b;
>
> +exit_free_security:
> + security_kdbus_bus_free(b);
> exit_unref:
> kdbus_node_deactivate(&b->node);
> kdbus_node_unref(&b->node);
> diff --git a/ipc/kdbus/bus.h b/ipc/kdbus/bus.h
> index 5bea5ef..03e4a54 100644
> --- a/ipc/kdbus/bus.h
> +++ b/ipc/kdbus/bus.h
> @@ -53,6 +53,7 @@ struct kdbus_user;
> * @notify_list: List of pending kernel-generated messages
> * @notify_lock: Notification list lock
> * @notify_flush_lock: Notification flushing lock
> + * @security: LSM security blob
> */
> struct kdbus_bus {
> struct kdbus_node node;
> @@ -81,6 +82,8 @@ struct kdbus_bus {
> struct list_head notify_list;
> spinlock_t notify_lock;
> struct mutex notify_flush_lock;
> +
> + void *security;
> };
>
> struct kdbus_bus *kdbus_bus_ref(struct kdbus_bus *bus);
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index 9993753..b85cdc7 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -31,6 +31,7 @@
> #include <linux/slab.h>
> #include <linux/syscalls.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "connection.h"
> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
> bool is_activator;
> bool is_monitor;
> struct kvec kvec;
> + u32 sid, len;
> + char *label;
> int ret;
>
> struct {
> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
> }
> }
>
> + security_task_getsecid(current, &sid);
> + security_secid_to_secctx(sid, &label, &len);
> + ret = security_kdbus_connect(conn, label, len);
This is horrid. It's maximally inefficient for everyone.
Change the label argument to a task, and pass current. Let
the module decide what to do with it. While Smack and SELinux
will be making the decision based on the label, you can't
count on that for all modules.
If you think you need to do *anything* using secids you are
mistaken. The only case where a secid should be used is in
legacy networking code where the maintainer has decided that
security is only worth 32 bits and can never be worth more.
> + if (ret) {
> + ret = -EPERM;
> + goto exit_unref;
> + }
> +
> if (atomic_inc_return(&conn->user->connections) > KDBUS_USER_MAX_CONN) {
> /* decremented by destructor as conn->user is valid */
> ret = -EMFILE;
> @@ -276,6 +287,7 @@ static void __kdbus_conn_free(struct kref *kref)
> kdbus_pool_free(conn->pool);
> kdbus_ep_unref(conn->ep);
> put_cred(conn->cred);
> + security_kdbus_conn_free(conn);
> kfree(conn->description);
> kfree(conn->quota);
> kfree(conn);
> @@ -1107,6 +1119,12 @@ static int kdbus_conn_reply(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
> if (ret < 0)
> goto exit;
>
> + ret = security_kdbus_talk(src, dst);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
Why not return what the module reports?
Why EPERM? EPERM means "you needed privilege".
EACCES means "you were denied access by normal means".
> +
> mutex_lock(&dst->lock);
> reply = kdbus_reply_find(src, dst, kmsg->msg.cookie_reply);
> if (reply) {
> @@ -1187,6 +1205,12 @@ static struct kdbus_reply *kdbus_conn_call(struct kdbus_conn *src,
> if (ret < 0)
> goto exit;
>
> + ret = security_kdbus_talk(src, dst);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> if (!kdbus_conn_policy_talk(src, current_cred(), dst)) {
> ret = -EPERM;
> goto exit;
> @@ -1248,6 +1272,12 @@ static int kdbus_conn_unicast(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
> if (ret < 0)
> goto exit;
>
> + ret = security_kdbus_talk(src, dst);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> if (is_signal) {
> /* like broadcasts we eavesdrop even if the msg is dropped */
> kdbus_bus_eavesdrop(bus, src, kmsg);
> @@ -1639,6 +1669,12 @@ struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, bool privileged,
> if (ret < 0)
> goto exit;
>
> + ret = security_kdbus_ep_setpolicy(c->ep->bus);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> ret = kdbus_policy_set(&c->ep->bus->policy_db, args.items,
> args.items_size, 1,
> kdbus_conn_is_policy_holder(c), c);
> @@ -1732,6 +1768,10 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp)
> if (ret != 0)
> return ret;
>
> + ret = security_kdbus_conn_info(conn);
> + if (ret)
> + return -EPERM;
> +
> /* registry must be held throughout lookup *and* collecting data */
> down_read(&bus->name_registry->rwlock);
>
> @@ -1905,6 +1945,12 @@ int kdbus_cmd_update(struct kdbus_conn *conn, void __user *argp)
> /* now that we verified the input, update the connection */
>
> if (item_policy) {
> + ret = security_kdbus_ep_setpolicy(conn->ep->bus);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> ret = kdbus_policy_set(&conn->ep->bus->policy_db, cmd->items,
> KDBUS_ITEMS_SIZE(cmd, items),
> 1, true, conn);
> @@ -1948,6 +1994,10 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
> .argc = ARRAY_SIZE(argv),
> };
>
> + ret = security_kdbus_send(conn, conn->ep->bus);
> + if (ret)
> + return -EPERM;
> +
> if (!kdbus_conn_is_ordinary(conn))
> return -EOPNOTSUPP;
>
> @@ -2044,6 +2094,10 @@ int kdbus_cmd_recv(struct kdbus_conn *conn, void __user *argp)
> .argc = ARRAY_SIZE(argv),
> };
>
> + ret = security_kdbus_recv(conn, conn->ep->bus);
> + if (ret)
> + return -EPERM;
> +
> if (!kdbus_conn_is_ordinary(conn) &&
> !kdbus_conn_is_monitor(conn) &&
> !kdbus_conn_is_activator(conn))
> diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
> index d1ffe90..1f91d39 100644
> --- a/ipc/kdbus/connection.h
> +++ b/ipc/kdbus/connection.h
> @@ -19,6 +19,7 @@
> #include <linux/kref.h>
> #include <linux/lockdep.h>
> #include <linux/path.h>
> +#include <uapi/linux/kdbus.h>
>
> #include "limits.h"
> #include "metadata.h"
> @@ -73,6 +74,7 @@ struct kdbus_kmsg;
> * @names_queue_list: Well-known names this connection waits for
> * @privileged: Whether this connection is privileged on the bus
> * @faked_meta: Whether the metadata was faked on HELLO
> + * @security: LSM security blob
> */
> struct kdbus_conn {
> struct kref kref;
> @@ -113,6 +115,8 @@ struct kdbus_conn {
>
> bool privileged:1;
> bool faked_meta:1;
> +
> + void *security;
> };
>
> struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn);
> diff --git a/ipc/kdbus/domain.c b/ipc/kdbus/domain.c
> index ac9f760..da9cdab 100644
> --- a/ipc/kdbus/domain.c
> +++ b/ipc/kdbus/domain.c
> @@ -20,6 +20,7 @@
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "domain.h"
> @@ -73,6 +74,7 @@ static void kdbus_domain_free(struct kdbus_node *node)
> put_user_ns(domain->user_namespace);
> ida_destroy(&domain->user_ida);
> idr_destroy(&domain->user_idr);
> + security_kdbus_domain_free(domain);
> kfree(domain);
> }
>
> @@ -104,6 +106,10 @@ struct kdbus_domain *kdbus_domain_new(unsigned int access)
> idr_init(&d->user_idr);
> ida_init(&d->user_ida);
>
> + ret = security_kdbus_domain_alloc(d);
> + if (ret)
> + return ERR_PTR(-EPERM);
> +
> /* Pin user namespace so we can guarantee domain-unique bus * names. */
> d->user_namespace = get_user_ns(current_user_ns());
>
> @@ -116,6 +122,7 @@ struct kdbus_domain *kdbus_domain_new(unsigned int access)
> exit_unref:
> kdbus_node_deactivate(&d->node);
> kdbus_node_unref(&d->node);
> + security_kdbus_domain_free(d);
> return ERR_PTR(ret);
> }
>
> @@ -264,7 +271,7 @@ static void __kdbus_user_free(struct kref *kref)
> if (uid_valid(user->uid))
> idr_remove(&user->domain->user_idr, __kuid_val(user->uid));
> mutex_unlock(&user->domain->lock);
> -
> + security_kdbus_domain_free(user->domain);
> kdbus_domain_unref(user->domain);
> kfree(user);
> }
> diff --git a/ipc/kdbus/domain.h b/ipc/kdbus/domain.h
> index 447a2bd..3db06d8 100644
> --- a/ipc/kdbus/domain.h
> +++ b/ipc/kdbus/domain.h
> @@ -31,6 +31,7 @@
> * @user_ida: Set of all users to compute small indices
> * @user_namespace: User namespace, pinned at creation time
> * @dentry: Root dentry of VFS mount (don't use outside of kdbusfs)
> + * @security: LSM security blob
> */
> struct kdbus_domain {
> struct kdbus_node node;
> @@ -40,6 +41,7 @@ struct kdbus_domain {
> struct ida user_ida;
> struct user_namespace *user_namespace;
> struct dentry *dentry;
> + void *security;
> };
>
> /**
> diff --git a/ipc/kdbus/endpoint.c b/ipc/kdbus/endpoint.c
> index 9a95a5e..380228f 100644
> --- a/ipc/kdbus/endpoint.c
> +++ b/ipc/kdbus/endpoint.c
> @@ -21,6 +21,7 @@
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "connection.h"
> @@ -122,6 +123,12 @@ struct kdbus_ep *kdbus_ep_new(struct kdbus_bus *bus, const char *name,
> kdbus_policy_db_init(&e->policy_db);
> e->bus = kdbus_bus_ref(bus);
>
> + ret = security_kdbus_ep_create(bus);
> + if (ret) {
> + ret = -EPERM;
> + goto exit_unref;
> + }
> +
> ret = kdbus_node_link(&e->node, &bus->node, name);
> if (ret < 0)
> goto exit_unref;
> @@ -265,6 +272,10 @@ int kdbus_cmd_ep_update(struct kdbus_ep *ep, void __user *argp)
> .argc = ARRAY_SIZE(argv),
> };
>
> + ret = security_kdbus_ep_setpolicy(ep->bus);
> + if (ret)
> + return -EPERM;
> +
> ret = kdbus_args_parse(&args, argp, &cmd);
> if (ret != 0)
> return ret;
> diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
> index d77ee08..dd20bea 100644
> --- a/ipc/kdbus/names.c
> +++ b/ipc/kdbus/names.c
> @@ -24,6 +24,7 @@
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "bus.h"
> #include "connection.h"
> @@ -503,6 +504,12 @@ int kdbus_cmd_name_acquire(struct kdbus_conn *conn, void __user *argp)
> goto exit;
> }
>
> + ret = security_kdbus_name_acquire(conn, item_name);
> + if (ret) {
> + ret = -EPERM;
> + goto exit;
> + }
> +
> /*
> * Do atomic_inc_return here to reserve our slot, then decrement
> * it before returning.
> @@ -724,6 +731,10 @@ int kdbus_cmd_list(struct kdbus_conn *conn, void __user *argp)
> if (ret != 0)
> return ret;
>
> + ret = security_kdbus_name_list(conn->ep->bus);
> + if (ret)
> + return -EPERM;
> +
> /* lock order: domain -> bus -> ep -> names -> conn */
> down_read(®->rwlock);
> down_read(&conn->ep->bus->conn_rwlock);
> diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
> index 25bb3ad..9872fb4 100644
> --- a/ipc/kdbus/queue.c
> +++ b/ipc/kdbus/queue.c
> @@ -28,6 +28,7 @@
> #include <linux/slab.h>
> #include <linux/syscalls.h>
> #include <linux/uio.h>
> +#include <linux/security.h>
>
> #include "util.h"
> #include "domain.h"
> @@ -514,12 +515,17 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
>
> for (i = 0; i < res->fds_count; i++) {
> if (install_fds) {
> - fds[i] = get_unused_fd_flags(O_CLOEXEC);
> - if (fds[i] >= 0)
> - fd_install(fds[i],
> - get_file(res->fds[i]));
> - else
> + if (security_file_receive(res->fds[i])) {
> + fds[i] = -1;
> incomplete_fds = true;
> + } else {
> + fds[i] = get_unused_fd_flags(O_CLOEXEC);
> + if (fds[i] >= 0)
> + fd_install(fds[i],
> + get_file(res->fds[i]));
> + else
> + incomplete_fds = true;
> + }
> } else {
> fds[i] = -1;
> }
> @@ -557,13 +563,17 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
> m.fd = -1;
>
> if (install_fds) {
> - m.fd = get_unused_fd_flags(O_CLOEXEC);
> - if (m.fd < 0) {
> - m.fd = -1;
> + if (security_file_receive(d->memfd.file)) {
> incomplete_fds = true;
> } else {
> - fd_install(m.fd,
> - get_file(d->memfd.file));
> + m.fd = get_unused_fd_flags(O_CLOEXEC);
> + if (m.fd < 0) {
> + m.fd = -1;
> + incomplete_fds = true;
> + } else {
> + fd_install(m.fd,
> + get_file(d->memfd.file));
> + }
> }
> }
>
^ permalink raw reply
* Re: [RFC 4/8] lsm: smack: smack callbacks for kdbus security hooks
From: Casey Schaufler @ 2015-07-08 16:38 UTC (permalink / raw)
To: Stephen Smalley, Paul Osmialowski, Paul Moore, James Morris,
Serge E. Hallyn, Kees Cook, Tetsuo Handa, Neil Brown, Mark Rustad,
Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
Shuah Khan, Al Viro, linux-security-module, linux-kernel,
linux-api
Cc: Karol Lewandowski, Lukasz Skalski
In-Reply-To: <559D28DE.4070406@tycho.nsa.gov>
On 7/8/2015 6:42 AM, Stephen Smalley wrote:
> On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
>> This adds implementation of three smack callbacks sitting behind kdbus
>> security hooks as proposed by Karol Lewandowski.
>>
>> Originates from:
>>
>> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
>> commit: fc3505d058c001fe72a6f66b833e0be5b2d118f3
>>
>> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
>> commit: 103c26fd27d1ec8c32d85dd3d85681f936ac66fb
>>
>> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
>> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
>> ---
>> security/smack/smack_lsm.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 68 insertions(+)
>>
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index a143328..033b756 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -41,6 +41,7 @@
>> #include <linux/msg.h>
>> #include <linux/shm.h>
>> #include <linux/binfmts.h>
>> +#include <kdbus/connection.h>
>> #include "smack.h"
>>
>> #define TRANS_TRUE "TRUE"
>> @@ -3336,6 +3337,69 @@ static int smack_setprocattr(struct task_struct *p, char *name,
>> }
>>
>> /**
>> + * smack_kdbus_connect - Set the security blob for a KDBus connection
>> + * @conn: the connection
>> + * @secctx: smack label
>> + * @seclen: smack label length
>> + *
>> + * Returns 0
>> + */
>> +static int smack_kdbus_connect(struct kdbus_conn *conn,
>> + const char *secctx, u32 seclen)
>> +{
>> + struct smack_known *skp;
>> +
>> + if (secctx && seclen > 0)
>> + skp = smk_import_entry(secctx, seclen);
>> + else
>> + skp = smk_of_current();
>> + conn->security = skp;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * smack_kdbus_conn_free - Clear the security blob for a KDBus connection
>> + * @conn: the connection
>> + *
>> + * Clears the blob pointer
>> + */
>> +static void smack_kdbus_conn_free(struct kdbus_conn *conn)
>> +{
>> + conn->security = NULL;
>> +}
>> +
>> +/**
>> + * smack_kdbus_talk - Smack access on KDBus
>> + * @src: source kdbus connection
>> + * @dst: destination kdbus connection
>> + *
>> + * Return 0 if a subject with the smack of sock could access
>> + * an object with the smack of other, otherwise an error code
>> + */
>> +static int smack_kdbus_talk(const struct kdbus_conn *src,
>> + const struct kdbus_conn *dst)
>> +{
>> + struct smk_audit_info ad;
>> + struct smack_known *sskp = src->security;
>> + struct smack_known *dskp = dst->security;
>> + int ret;
>> +
>> + BUG_ON(sskp == NULL);
>> + BUG_ON(dskp == NULL);
>> +
>> + if (smack_privileged(CAP_MAC_OVERRIDE))
>> + return 0;
>> +
>> + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NONE);
>> +
>> + ret = smk_access(sskp, dskp, MAY_WRITE, &ad);
>> + if (ret)
>> + return ret;
>> + return 0;
>> +}
>> +
>> +/**
>> * smack_unix_stream_connect - Smack access on UDS
>> * @sock: one sock
>> * @other: the other sock
>> @@ -4393,6 +4457,10 @@ struct security_hook_list smack_hooks[] = {
>> LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
>> LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
>> LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx),
>> +
>> + LSM_HOOK_INIT(kdbus_connect, smack_kdbus_connect),
>> + LSM_HOOK_INIT(kdbus_conn_free, smack_kdbus_conn_free),
>> + LSM_HOOK_INIT(kdbus_talk, smack_kdbus_talk),
>> };
> If Smack only truly needs 3 hooks, then it begs the question of why
> there are so many other hooks defined. Are the other hooks just to
> support finer-grained distinctions, or is Smack's coverage incomplete?
I haven't been following kdbus closely for a while, but the original
intent for Smack and kdbus was that it Smack controls would be on the
objects involved, and that to accomplish that only a small number of
hooks would be necessary. After all, Smack uses fewer hooks than SELinux
on other things. I do agree that without a user there is no point in
having hooks. If SELinux requires the other hooks we might want to
hold off on asking for the hooks until the SELinux implementation is
exposed. I also think that AppArmor should be examined as a potential
user of the hooks, just to make sure the hooks aren't excessively
oriented toward subject/object based security modules.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [RFC 2/8] lsm: smack: Make ipc/kdbus includes visible so smack callbacks could see them
From: Daniel Mack @ 2015-07-08 16:43 UTC (permalink / raw)
To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
Serge E. Hallyn, Kees Cook, Tetsuo Handa, Stephen Smalley,
Neil Brown, Mark Rustad, Greg Kroah-Hartman, David Herrmann,
Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
linux-kernel, linux-api
Cc: Karol Lewandowski, Lukasz Skalski
In-Reply-To: <1436351110-5902-3-git-send-email-p.osmialowsk@samsung.com>
On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
> security/smack/Makefile | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/security/smack/Makefile b/security/smack/Makefile
> index ee2ebd5..bd6927c 100644
> --- a/security/smack/Makefile
> +++ b/security/smack/Makefile
> @@ -6,3 +6,5 @@ obj-$(CONFIG_SECURITY_SMACK) := smack.o
>
> smack-y := smack_lsm.o smack_access.o smackfs.o
> smack-$(CONFIG_SECURITY_SMACK_NETFILTER) += smack_netfilter.o
> +
> +ccflags-y += -Iipc
>
I would really like to avoid exposing the internal structures of kdbus
to LSM modules. This is going to get messy if any internals change in
the future.
The callbacks for smack currently only use the passed structure to
access the opaque 'security' pointer. Can't you just pass that directly
(as void**), along with other parameters the LSMs needs to know about?
Thanks,
Daniel
^ permalink raw reply
* Re: [RFC 0/8] Introduce LSM to KDBUS
From: Casey Schaufler @ 2015-07-08 16:46 UTC (permalink / raw)
To: Paul Osmialowski, Paul Moore, James Morris, Serge E. Hallyn,
Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown, Mark Rustad,
Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
Shuah Khan, Al Viro, linux-security-module, linux-kernel,
linux-api
Cc: Karol Lewandowski, Lukasz Skalski
In-Reply-To: <1436351110-5902-1-git-send-email-p.osmialowsk@samsung.com>
On 7/8/2015 3:25 AM, Paul Osmialowski wrote:
> This patchset partially summarizes effects of collective work by
> Karol Lewandowski and Paul Moore towards introduction of LSM into KDBUS.
>
> These patches originate from following git repositories:
>
> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
>
> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
>
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
>
> Since kdbus made its way to linux-next tree, I was kindly asked by
> Karol Lewandowski to fit his work into the current kdbus code existing
> there.
>
> As both kdbus and security related code changed a bit, so are my changes
> quite substantial in places.
>
> Note that SELinux kdbus access control patches are absent - only SMACK part
> of original work is included.
Patches 2 and 3 need to be reversed. You can't add the Smack hooks
until you've added the infrastructure for them.
My comments should in no way be construed as an endorsement of kdbus.
>
> I've also made some changes to kdbus test suite. In order to see LSM hooks
> in action we need to be able to run tests from different executable
> binaries holding different security labels.
>
> Therefore I added ability to select execution of particular test by
> executed binary name. This is essential for running newly added 'send' test
> which should communicate with 'daemon' test running in another process.
>
> Karol Lewandowski (1):
> lsm: make security_file_receive available for external modules
>
> Paul Osmialowski (7):
> lsm: smack: Make ipc/kdbus includes visible so smack callbacks could
> see them
> lsm: kdbus security hooks
> lsm: smack: smack callbacks for kdbus security hooks
> kdbus: use LSM hooks in kdbus code
> kdbus: TEST_CREATE_CONN now does no depend on TEST_CREATE_BUS
> kdbus: selftests extended
> kdbus: Ability to run kdbus test by executable binary name
>
> include/linux/lsm_hooks.h | 67 +++++++++++++
> include/linux/security.h | 99 +++++++++++++++++++
> ipc/kdbus/bus.c | 12 ++-
> ipc/kdbus/bus.h | 3 +
> ipc/kdbus/connection.c | 54 +++++++++++
> ipc/kdbus/connection.h | 4 +
> ipc/kdbus/domain.c | 9 +-
> ipc/kdbus/domain.h | 2 +
> ipc/kdbus/endpoint.c | 11 +++
> ipc/kdbus/names.c | 11 +++
> ipc/kdbus/queue.c | 30 ++++--
> security/security.c | 118 +++++++++++++++++++++++
> security/smack/Makefile | 2 +
> security/smack/smack_lsm.c | 68 +++++++++++++
> tools/testing/selftests/kdbus/Makefile | 1 +
> tools/testing/selftests/kdbus/kdbus-test.c | 37 ++++++-
> tools/testing/selftests/kdbus/kdbus-test.h | 1 +
> tools/testing/selftests/kdbus/kdbus-util.c | 37 ++++---
> tools/testing/selftests/kdbus/kdbus-util.h | 2 +-
> tools/testing/selftests/kdbus/test-activator.c | 20 ++--
> tools/testing/selftests/kdbus/test-chat.c | 6 +-
> tools/testing/selftests/kdbus/test-connection.c | 8 +-
> tools/testing/selftests/kdbus/test-fd.c | 2 +-
> tools/testing/selftests/kdbus/test-message.c | 69 ++++++++-----
> tools/testing/selftests/kdbus/test-metadata-ns.c | 10 +-
> tools/testing/selftests/kdbus/test-monitor.c | 9 +-
> tools/testing/selftests/kdbus/test-policy-ns.c | 8 +-
> tools/testing/selftests/kdbus/test-policy-priv.c | 48 +++++----
> tools/testing/selftests/kdbus/test-send.c | 84 ++++++++++++++++
> tools/testing/selftests/kdbus/test-sync.c | 2 +-
> tools/testing/selftests/kdbus/test-timeout.c | 2 +-
> 31 files changed, 732 insertions(+), 104 deletions(-)
> create mode 100644 tools/testing/selftests/kdbus/test-send.c
>
^ permalink raw reply
* Re: [PATCH V3 0/5] Allow user to request memory to be locked on page fault
From: Andrew Morton @ 2015-07-08 17:00 UTC (permalink / raw)
To: Eric B Munson
Cc: Shuah Khan, Michal Hocko, Michael Kerrisk, Vlastimil Babka,
linux-alpha, linux-kernel, linux-mips, linux-parisc, linuxppc-dev,
sparclinux, linux-xtensa, linux-mm, linux-arch, linux-api
In-Reply-To: <20150708132302.GB4669@akamai.com>
On Wed, 8 Jul 2015 09:23:02 -0400 Eric B Munson <emunson@akamai.com> wrote:
> > I don't know whether these syscalls should be documented via new
> > manpages, or if we should instead add them to the existing
> > mlock/munlock/mlockall manpages. Michael, could you please advise?
> >
>
> Thanks for adding the series. I owe you several updates (getting the
> new syscall right for all architectures and a set of tests for the new
> syscalls). Would you prefer a new pair of patches or I update this set?
It doesn't matter much. I guess a full update will be more convenient
at your end.
^ permalink raw reply
* Re: [PATCH 0/24] kernel: add a netlink interface to get information about processes (v2)
From: Andy Lutomirski @ 2015-07-08 17:39 UTC (permalink / raw)
To: Andrew Vagin
Cc: Andrey Vagin, linux-kernel@vger.kernel.org, Linux API,
Oleg Nesterov, Andrew Morton, Cyrill Gorcunov, Pavel Emelyanov,
Roger Luethi, Arnd Bergmann, Arnaldo Carvalho de Melo,
David Ahern, Pavel Odintsov
In-Reply-To: <20150708161022.GA1705@odin.com>
On Wed, Jul 8, 2015 at 9:10 AM, Andrew Vagin <avagin@odin.com> wrote:
> On Tue, Jul 07, 2015 at 08:56:37AM -0700, Andy Lutomirski wrote:
>> On Tue, Jul 7, 2015 at 8:43 AM, Andrew Vagin <avagin@odin.com> wrote:
>> > On Mon, Jul 06, 2015 at 10:10:32AM -0700, Andy Lutomirski wrote:
>> >> On Mon, Jul 6, 2015 at 1:47 AM, Andrey Vagin <avagin@openvz.org> wrote:
>> >> > Currently we use the proc file system, where all information are
>> >> > presented in text files, what is convenient for humans. But if we need
>> >> > to get information about processes from code (e.g. in C), the procfs
>> >> > doesn't look so cool.
>> >> >
>> >> > From code we would prefer to get information in binary format and to be
>> >> > able to specify which information and for which tasks are required. Here
>> >> > is a new interface with all these features, which is called task_diag.
>> >> > In addition it's much faster than procfs.
>> >> >
>> >> > task_diag is based on netlink sockets and looks like socket-diag, which
>> >> > is used to get information about sockets.
>> >>
>> >> I think I like this in principle, but I have can see a few potential
>> >> problems with using netlink for this:
>> >>
>> >> 1. Netlink very naturally handles net namespaces, but it doesn't
>> >> naturally handle any other kind of namespace. In fact, the taskstats
>> >> code that you're building on has highly broken user and pid namespace
>> >> support. (Look for some obviously useless init_user_ns and
>> >> init_pid_ns references. But that's only the obvious problem. That
>> >> code calls current_user_ns() and task_active_pid_ns(current) from
>> >> .doit, which is, in turn, called from sys_write, and looking at
>> >> current's security state from sys_write is a big no-no.)
>> >>
>> >> You could partially fix it by looking at f_cred's namespaces, but that
>> >> would be a change of what it means to create a netlink socket, and I'm
>> >> not sure that's a good idea.
>> >
>> > If I don't miss something, all problems around pidns and userns are
>> > related with multicast functionality. task_diag is using
>> > request/response scheme and doesn't send multicast packets.
>>
>> It has nothing to do with multicast. task_diag needs to know what
>> pidns and userns to use for a request, but netlink isn't set up to
>> give you any reasonably way to do that. A netlink socket is
>> fundamentally tied to a *net* ns (it's a socket, after all). But you
>> can send it requests using write(2), and calling current_user_ns()
>> from write(2) is bad. There's a long history of bugs and
>> vulnerabilities related to thinking that current_cred() and similar
>> are acceptable things to use in write(2) implementations.
>>
>
> As far as I understand, socket_diag doesn't have this problem, becaus
> each socket has a link on a namespace where it was created.
>
> What if we will pin the current pidns and credentials to a task_diag
> socket in a moment when it's created.
That's certainly doable. OTOH, if anything does:
socket(AF_NETLINK, ...);
unshare(CLONE_PID);
fork();
then they now have a (minor) security problem.
--Andy
^ permalink raw reply
* Re: [PATCH -mm v6 0/6] idle memory tracking
From: Vladimir Davydov @ 2015-07-08 17:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Minchan Kim, Raghavendra K T, Johannes Weiner, Michal Hocko,
Greg Thelen, Michel Lespinasse, David Rientjes, Pavel Emelyanov,
Cyrill Gorcunov, Jonathan Corbet, linux-api, linux-doc, linux-mm,
cgroups, linux-kernel
In-Reply-To: <cover.1434102076.git.vdavydov@parallels.com>
Hi,
Any comments, thoughts, proposals regarding this patch? Any chance for
it to get merged?
Thanks,
Vladimir
On Fri, Jun 12, 2015 at 12:52:20PM +0300, Vladimir Davydov wrote:
> Hi,
>
> This patch set introduces a new user API for tracking user memory pages
> that have not been used for a given period of time. The purpose of this
> is to provide the userspace with the means of tracking a workload's
> working set, i.e. the set of pages that are actively used by the
> workload. Knowing the working set size can be useful for partitioning
> the system more efficiently, e.g. by tuning memory cgroup limits
> appropriately, or for job placement within a compute cluster.
>
> ---- USE CASES ----
>
> The unified cgroup hierarchy has memory.low and memory.high knobs, which
> are defined as the low and high boundaries for the workload working set
> size. However, the working set size of a workload may be unknown or
> change in time. With this patch set, one can periodically estimate the
> amount of memory unused by each cgroup and tune their memory.low and
> memory.high parameters accordingly, therefore optimizing the overall
> memory utilization.
>
> Another use case is balancing workloads within a compute cluster.
> Knowing how much memory is not really used by a workload unit may help
> take a more optimal decision when considering migrating the unit to
> another node within the cluster.
>
> Also, as noted by Minchan, this would be useful for per-process reclaim
> (https://lwn.net/Articles/545668/). With idle tracking, we could reclaim idle
> pages only by smart user memory manager.
>
> ---- USER API ----
>
> The user API consists of two new proc files:
>
> * /proc/kpageidle. This file implements a bitmap where each bit corresponds
> to a page, indexed by PFN. When the bit is set, the corresponding page is
> idle. A page is considered idle if it has not been accessed since it was
> marked idle. To mark a page idle one should set the bit corresponding to the
> page by writing to the file. A value written to the file is OR-ed with the
> current bitmap value. Only user memory pages can be marked idle, for other
> page types input is silently ignored. Writing to this file beyond max PFN
> results in the ENXIO error. Only available when CONFIG_IDLE_PAGE_TRACKING is
> set.
>
> This file can be used to estimate the amount of pages that are not
> used by a particular workload as follows:
>
> 1. mark all pages of interest idle by setting corresponding bits in the
> /proc/kpageidle bitmap
> 2. wait until the workload accesses its working set
> 3. read /proc/kpageidle and count the number of bits set
>
> * /proc/kpagecgroup. This file contains a 64-bit inode number of the
> memory cgroup each page is charged to, indexed by PFN. Only available when
> CONFIG_MEMCG is set.
>
> This file can be used to find all pages (including unmapped file
> pages) accounted to a particular cgroup. Using /proc/kpageidle, one
> can then estimate the cgroup working set size.
>
> For an example of using these files for estimating the amount of unused
> memory pages per each memory cgroup, please see the script attached
> below.
>
> ---- REASONING ----
>
> The reason to introduce the new user API instead of using
> /proc/PID/{clear_refs,smaps} is that the latter has two serious
> drawbacks:
>
> - it does not count unmapped file pages
> - it affects the reclaimer logic
>
> The new API attempts to overcome them both. For more details on how it
> is achieved, please see the comment to patch 5.
>
> ---- CHANGE LOG ----
>
> Changes in v6:
>
> - Split the patch introducing page_cgroup_ino helper to ease review.
> - Rebase on top of v4.1-rc7-mmotm-2015-06-09-16-55
>
> Changes in v5:
>
> - Fix possible race between kpageidle_clear_pte_refs() and
> __page_set_anon_rmap() by checking that a page is on an LRU list
> under zone->lru_lock (Minchan).
> - Export idle flag via /proc/kpageflags (Minchan).
> - Rebase on top of 4.1-rc3.
>
> Changes in v4:
>
> This iteration primarily addresses Minchan's comments to v3:
>
> - Implement /proc/kpageidle as a bitmap instead of using u64 per each page,
> because there does not seem to be any future uses for the other 63 bits.
> - Do not double-increase pra->referenced in page_referenced_one() if the page
> was young and referenced recently.
> - Remove the pointless (page_count == 0) check from kpageidle_get_page().
> - Rename kpageidle_clear_refs() to kpageidle_clear_pte_refs().
> - Improve comments to kpageidle-related functions.
> - Rebase on top of 4.1-rc2.
>
> Note it does not address Minchan's concern of possible __page_set_anon_rmap vs
> page_referenced race (see https://lkml.org/lkml/2015/5/3/220) since it is still
> unclear if this race can really happen (see https://lkml.org/lkml/2015/5/4/160)
>
> Changes in v3:
>
> - Enable CONFIG_IDLE_PAGE_TRACKING for 32 bit. Since this feature
> requires two extra page flags and there is no space for them on 32
> bit, page ext is used (thanks to Minchan Kim).
> - Minor code cleanups and comments improved.
> - Rebase on top of 4.1-rc1.
>
> Changes in v2:
>
> - The main difference from v1 is the API change. In v1 the user can
> only set the idle flag for all pages at once, and for clearing the
> Idle flag on pages accessed via page tables /proc/PID/clear_refs
> should be used.
> The main drawback of the v1 approach, as noted by Minchan, is that on
> big machines setting the idle flag for each pages can result in CPU
> bursts, which would be especially frustrating if the user only wanted
> to estimate the amount of idle pages for a particular process or VMA.
> With the new API a more fine-grained approach is possible: one can
> read a process's /proc/PID/pagemap and set/check the Idle flag only
> for those pages of the process's address space he or she is
> interested in.
> Another good point about the v2 API is that it is possible to limit
> /proc/kpage* scanning rate when the user wants to estimate the total
> number of idle pages, which is unachievable with the v1 approach.
> - Make /proc/kpagecgroup return the ino of the closest online ancestor
> in case the cgroup a page is charged to is offline.
> - Fix /proc/PID/clear_refs not clearing Young page flag.
> - Rebase on top of v4.0-rc6-mmotm-2015-04-01-14-54
>
> v4: https://lkml.org/lkml/2015/5/7/580
> v3: https://lkml.org/lkml/2015/4/28/224
> v2: https://lkml.org/lkml/2015/4/7/260
> v1: https://lkml.org/lkml/2015/3/18/794
>
> ---- PATCH SET STRUCTURE ----
>
> The patch set is organized as follows:
>
> - patch 1 adds page_cgroup_ino() helper for the sake of
> /proc/kpagecgroup and patches 2-3 do related cleanup
> - patch 4 adds /proc/kpagecgroup, which reports cgroup ino each page is
> charged to
> - patch 5 implements the idle page tracking feature, including the
> userspace API, /proc/kpageidle
> - patch 6 exports idle flag via /proc/kpageflags
>
> ---- SIMILAR WORKS ----
>
> Originally, the patch for tracking idle memory was proposed back in 2011
> by Michel Lespinasse (see http://lwn.net/Articles/459269/). The main
> difference between Michel's patch and this one is that Michel
> implemented a kernel space daemon for estimating idle memory size per
> cgroup while this patch only provides the userspace with the minimal API
> for doing the job, leaving the rest up to the userspace. However, they
> both share the same idea of Idle/Young page flags to avoid affecting the
> reclaimer logic.
>
> ---- SCRIPT FOR COUNTING IDLE PAGES PER CGROUP ----
> #! /usr/bin/python
> #
>
> import os
> import stat
> import errno
> import struct
>
> CGROUP_MOUNT = "/sys/fs/cgroup/memory"
> BUFSIZE = 8 * 1024 # must be multiple of 8
>
>
> def set_idle():
> f = open("/proc/kpageidle", "wb", BUFSIZE)
> while True:
> try:
> f.write(struct.pack("Q", pow(2, 64) - 1))
> except IOError as err:
> if err.errno == errno.ENXIO:
> break
> raise
> f.close()
>
>
> def count_idle():
> f_flags = open("/proc/kpageflags", "rb", BUFSIZE)
> f_cgroup = open("/proc/kpagecgroup", "rb", BUFSIZE)
> f_idle = open("/proc/kpageidle", "rb", BUFSIZE)
>
> pfn = 0
> nr_idle = {}
> while True:
> s = f_flags.read(8)
> if not s:
> break
>
> flags, = struct.unpack('Q', s)
> cgino, = struct.unpack('Q', f_cgroup.read(8))
>
> bit = pfn % 64
> if not bit:
> idle_bitmap, = struct.unpack('Q', f_idle.read(8))
>
> idle = idle_bitmap >> bit & 1
> pfn += 1
>
> unevictable = flags >> 18 & 1
> huge = flags >> 22 & 1
>
> if idle and not unevictable:
> nr_idle[cgino] = nr_idle.get(cgino, 0) + (512 if huge else 1)
>
> f_flags.close()
> f_cgroup.close()
> f_idle.close()
> return nr_idle
>
>
> print "Setting the idle flag for each page..."
> set_idle()
>
> raw_input("Wait until the workload accesses its working set, then press Enter")
>
> print "Counting idle pages..."
> nr_idle = count_idle()
>
> for dir, subdirs, files in os.walk(CGROUP_MOUNT):
> ino = os.stat(dir)[stat.ST_INO]
> print dir + ": " + str(nr_idle.get(ino, 0) * 4) + " KB"
> ---- END SCRIPT ----
>
> Comments are more than welcome.
>
> Thanks,
>
> Vladimir Davydov (6):
> memcg: add page_cgroup_ino helper
> hwpoison: use page_cgroup_ino for filtering by memcg
> memcg: zap try_get_mem_cgroup_from_page
> proc: add kpagecgroup file
> proc: add kpageidle file
> proc: export idle flag via kpageflags
>
> Documentation/vm/pagemap.txt | 22 +++-
> fs/proc/page.c | 234 +++++++++++++++++++++++++++++++++
> fs/proc/task_mmu.c | 4 +-
> include/linux/memcontrol.h | 7 +-
> include/linux/mm.h | 88 +++++++++++++
> include/linux/page-flags.h | 9 ++
> include/linux/page_ext.h | 4 +
> include/uapi/linux/kernel-page-flags.h | 1 +
> mm/Kconfig | 12 ++
> mm/debug.c | 4 +
> mm/hwpoison-inject.c | 5 +-
> mm/memcontrol.c | 71 +++++-----
> mm/memory-failure.c | 16 +--
> mm/page_ext.c | 3 +
> mm/rmap.c | 8 ++
> mm/swap.c | 2 +
> 16 files changed, 428 insertions(+), 62 deletions(-)
>
> --
> 2.1.4
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3] ipc: Modify message queue accounting to not take kernel data structures into account
From: Doug Ledford @ 2015-07-08 19:17 UTC (permalink / raw)
To: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Davidlohr Bueso
Cc: Marcus Gelderie, lkml, David Howells, Alexander Viro, John Duffy,
Arto Bendiken, Linux API, Andrew Morton
In-Reply-To: <CAKgNAkjy-+2TkN=0Fe11bVea4q6uLcUx=++Mf1eFxhmPmZoc9w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4999 bytes --]
On 07/07/2015 09:01 AM, Michael Kerrisk (man-pages) wrote:
> Hi David,
>
> On 7 July 2015 at 07:16, Davidlohr Bueso <dave-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org> wrote:
>> On Mon, 2015-07-06 at 17:49 +0200, Marcus Gelderie wrote:
>>> A while back, the message queue implementation in the kernel was
>>> improved to use btrees to speed up retrieval of messages (commit
>>> d6629859b36). The patch introducing the improved kernel handling of
>>> message queues (using btrees) has, as a by-product, changed the
>>> meaning of the QSIZE field in the pseudo-file created for the queue.
>>> Before, this field reflected the size of the user-data in the queue.
>>> Since, it also takes kernel data structures into account. For
>>> example, if 13 bytes of user data are in the queue, on my machine the
>>> file reports a size of 61 bytes.
>>>
>>> There was some discussion on this topic before (for example
>>> https://lkml.org/lkml/2014/10/1/115). Commenting on a th lkml, Michael
>>> Kerrisk gave the following background (https://lkml.org/lkml/2015/6/16/74):
>>>
>>> The pseudofiles in the mqueue filesystem (usually mounted at
>>> /dev/mqueue) expose fields with metadata describing a message
>>> queue. One of these fields, QSIZE, as originally implemented,
>>> showed the total number of bytes of user data in all messages in
>>> the message queue, and this feature was documented from the
>>> beginning in the mq_overview(7) page. In 3.5, some other (useful)
>>> work happened to break the user-space API in a couple of places,
>>> including the value exposed via QSIZE, which now includes a measure
>>> of kernel overhead bytes for the queue, a figure that renders QSIZE
>>> useless for its original purpose, since there's no way to deduce
>>> the number of overhead bytes consumed by the implementation.
>>> (The other user-space breakage was subsequently fixed.)
>>
>> Michael, this breakage was never finally documented in the manpage,
>> right?
>
> Right.
>
>> I took a look and there is no mention, but it was a quick look.
>> It's just that if this patch goes in, I'd hate ending up with something
>> like this in the manpage:
>>
>> as of 3.5
>> <accounts for kernel overhead>
>>
>> as of 4.3
>> <behavior reverted back to not include kernel overhead... *sigh*>
>>
>> If there are changes to be made to the manpage, it should probably be
>> posted with this patch, methinks.
>
> I think something like the above will have to end up in the man page.
> The only thing I'd add is that the fix also has gone to -stable
> kernels. At least: I think this patch should also be marked for
> -stable. I'll take care of the man page updates as the patch goes
> through.
>
>>> This patch removes the accounting of kernel data structures in the
>>> queue. Reporting the size of these data-structures in the QSIZE field
>>> was a breaking change (see Michael's comment above). Without the QSIZE
>>> field reporting the total size of user-data in the queue, there is no
>>> way to deduce this number.
>>>
>>> It should be noted that the resource limit RLIMIT_MSGQUEUE is counted
>>> against the worst-case size of the queue (in both the old and the new
>>> implementation). Therefore, the kernel overhead accounting in QSIZE is
>>> not necessary to help the user understand the limitations RLIMIT imposes
>>> on the processes.
>>
>> Also, I would suggest adding some comment in struct mqueue_inode_info
>> for future reference, ie:
>>
>> - unsigned long qsize; /* size of queue in memory (sum of all msgs) */
>> + /*
>> + * Size of queue in memory (sum of all msgs). Accounts for
>> + * only userspace overhead; ignoring any in-kernel rbtree nodes.
>> + */
>> + unsigned long qsize;
>>
>> But no big deal in any case.
>>
>> I think this is the right approach,
>
> Me too.
>
>> but would still like to know if Doug
>> has any concerns about it.
>
> Looking on gmane, Doug does not appear to have been active on any
> lists since late May! Not sure why.
I responded yesterday in the v2 patch thread I believe. In any case, I
think this patch is fine, and can go to stable. This patch doesn't
actually change the math related to the rlimit checks (which is the main
thing I wanted to correct in my original patches), instead it corrects a
mistake I made. At the time, I mistakenly thought that the qsize
included the current message data total + the struct msg_msg size total.
It didn't, it was just the current user data total. I added the rbtree
nodes in order to keep the total accurate but I shouldn't have added the
rbtree nodes to this total because none of the other kernel usage was
previously included.
Acked-by: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH V3 3/5] mm: mlock: Introduce VM_LOCKONFAULT and add mlock flags to enable it
From: Jonathan Corbet @ 2015-07-08 19:23 UTC (permalink / raw)
To: Eric B Munson
Cc: Andrew Morton, Michal Hocko, Vlastimil Babka, linux-alpha,
linux-kernel, linux-mips, linux-parisc, linuxppc-dev, sparclinux,
linux-xtensa, linux-mm, linux-arch, linux-api
In-Reply-To: <1436288623-13007-4-git-send-email-emunson@akamai.com>
On Tue, 7 Jul 2015 13:03:41 -0400
Eric B Munson <emunson@akamai.com> wrote:
> This patch introduces the ability to request that pages are not
> pre-faulted, but are placed on the unevictable LRU when they are finally
> faulted in. This can be done area at a time via the
> mlock2(MLOCK_ONFAULT) or the mlockall(MCL_ONFAULT) system calls. These
> calls can be undone via munlock2(MLOCK_ONFAULT) or
> munlockall2(MCL_ONFAULT).
Quick, possibly dumb question: I've been beating my head against these for
a little bit, and I can't figure out what's supposed to happen in this
case:
mlock2(addr, len, MLOCK_ONFAULT);
munlock2(addr, len, MLOCK_LOCKED);
It looks to me like it will clear VM_LOCKED without actually unlocking any
pages. Is that the intended result?
Thanks,
jon
^ permalink raw reply
* Re: [PATCH v3] ipc: Modify message queue accounting to not take kernel data structures into account
From: Michael Kerrisk (man-pages) @ 2015-07-08 19:53 UTC (permalink / raw)
To: Doug Ledford
Cc: Davidlohr Bueso, Marcus Gelderie, lkml, David Howells,
Alexander Viro, John Duffy, Arto Bendiken, Linux API,
Andrew Morton
In-Reply-To: <559D7760.1020909-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On 8 July 2015 at 21:17, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 07/07/2015 09:01 AM, Michael Kerrisk (man-pages) wrote:
>> Hi David,
>>
>> On 7 July 2015 at 07:16, Davidlohr Bueso <dave-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org> wrote:
>>> On Mon, 2015-07-06 at 17:49 +0200, Marcus Gelderie wrote:
>>>> A while back, the message queue implementation in the kernel was
>>>> improved to use btrees to speed up retrieval of messages (commit
>>>> d6629859b36). The patch introducing the improved kernel handling of
>>>> message queues (using btrees) has, as a by-product, changed the
>>>> meaning of the QSIZE field in the pseudo-file created for the queue.
>>>> Before, this field reflected the size of the user-data in the queue.
>>>> Since, it also takes kernel data structures into account. For
>>>> example, if 13 bytes of user data are in the queue, on my machine the
>>>> file reports a size of 61 bytes.
>>>>
>>>> There was some discussion on this topic before (for example
>>>> https://lkml.org/lkml/2014/10/1/115). Commenting on a th lkml, Michael
>>>> Kerrisk gave the following background (https://lkml.org/lkml/2015/6/16/74):
>>>>
>>>> The pseudofiles in the mqueue filesystem (usually mounted at
>>>> /dev/mqueue) expose fields with metadata describing a message
>>>> queue. One of these fields, QSIZE, as originally implemented,
>>>> showed the total number of bytes of user data in all messages in
>>>> the message queue, and this feature was documented from the
>>>> beginning in the mq_overview(7) page. In 3.5, some other (useful)
>>>> work happened to break the user-space API in a couple of places,
>>>> including the value exposed via QSIZE, which now includes a measure
>>>> of kernel overhead bytes for the queue, a figure that renders QSIZE
>>>> useless for its original purpose, since there's no way to deduce
>>>> the number of overhead bytes consumed by the implementation.
>>>> (The other user-space breakage was subsequently fixed.)
>>>
>>> Michael, this breakage was never finally documented in the manpage,
>>> right?
>>
>> Right.
>>
>>> I took a look and there is no mention, but it was a quick look.
>>> It's just that if this patch goes in, I'd hate ending up with something
>>> like this in the manpage:
>>>
>>> as of 3.5
>>> <accounts for kernel overhead>
>>>
>>> as of 4.3
>>> <behavior reverted back to not include kernel overhead... *sigh*>
>>>
>>> If there are changes to be made to the manpage, it should probably be
>>> posted with this patch, methinks.
>>
>> I think something like the above will have to end up in the man page.
>> The only thing I'd add is that the fix also has gone to -stable
>> kernels. At least: I think this patch should also be marked for
>> -stable. I'll take care of the man page updates as the patch goes
>> through.
>>
>>>> This patch removes the accounting of kernel data structures in the
>>>> queue. Reporting the size of these data-structures in the QSIZE field
>>>> was a breaking change (see Michael's comment above). Without the QSIZE
>>>> field reporting the total size of user-data in the queue, there is no
>>>> way to deduce this number.
>>>>
>>>> It should be noted that the resource limit RLIMIT_MSGQUEUE is counted
>>>> against the worst-case size of the queue (in both the old and the new
>>>> implementation). Therefore, the kernel overhead accounting in QSIZE is
>>>> not necessary to help the user understand the limitations RLIMIT imposes
>>>> on the processes.
>>>
>>> Also, I would suggest adding some comment in struct mqueue_inode_info
>>> for future reference, ie:
>>>
>>> - unsigned long qsize; /* size of queue in memory (sum of all msgs) */
>>> + /*
>>> + * Size of queue in memory (sum of all msgs). Accounts for
>>> + * only userspace overhead; ignoring any in-kernel rbtree nodes.
>>> + */
>>> + unsigned long qsize;
>>>
>>> But no big deal in any case.
>>>
>>> I think this is the right approach,
>>
>> Me too.
>>
>>> but would still like to know if Doug
>>> has any concerns about it.
>>
>> Looking on gmane, Doug does not appear to have been active on any
>> lists since late May! Not sure why.
>
> I responded yesterday in the v2 patch thread I believe. In any case, I
> think this patch is fine, and can go to stable. This patch doesn't
> actually change the math related to the rlimit checks (which is the main
> thing I wanted to correct in my original patches), instead it corrects a
> mistake I made. At the time, I mistakenly thought that the qsize
> included the current message data total + the struct msg_msg size total.
> It didn't, it was just the current user data total. I added the rbtree
> nodes in order to keep the total accurate but I shouldn't have added the
> rbtree nodes to this total because none of the other kernel usage was
> previously included.
>
> Acked-by: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
+
Acked-by: Michael Kerrisk <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [RFC 4/8] lsm: smack: smack callbacks for kdbus security hooks
From: Paul Moore @ 2015-07-08 20:07 UTC (permalink / raw)
To: Casey Schaufler
Cc: Stephen Smalley, Paul Osmialowski, James Morris, Serge E. Hallyn,
Kees Cook, Tetsuo Handa, Neil Brown, Mark Rustad,
Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
Shuah Khan, Al Viro, linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Karol Lewandowski,
Lukasz Skalski
In-Reply-To: <559D5201.6060400-iSGtlc1asvQWG2LlvL+J4A@public.gmane.org>
On Wednesday, July 08, 2015 09:38:25 AM Casey Schaufler wrote:
> On 7/8/2015 6:42 AM, Stephen Smalley wrote:
> > On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
...
> > If Smack only truly needs 3 hooks, then it begs the question of why
> > there are so many other hooks defined. Are the other hooks just to
> > support finer-grained distinctions, or is Smack's coverage incomplete?
>
> I haven't been following kdbus closely for a while, but the original
> intent for Smack and kdbus was that it Smack controls would be on the
> objects involved, and that to accomplish that only a small number of
> hooks would be necessary. After all, Smack uses fewer hooks than SELinux
> on other things. I do agree that without a user there is no point in
> having hooks. If SELinux requires the other hooks we might want to
> hold off on asking for the hooks until the SELinux implementation is
> exposed. I also think that AppArmor should be examined as a potential
> user of the hooks, just to make sure the hooks aren't excessively
> oriented toward subject/object based security modules.
In Paul O.'s defense, we did have some discussion about the reasons for these
hooks, although that seems like ages ago and I would need to dig through the
archives (my inbox?) to find the reasoning for each.
However, I don't remember being very comfortable with the hooks back them
largely due to uncertainty about how we were treating kdbus with respect to
subjects/objects. I think it's worth restarting that discussion now before we
nit pick the patches themselves.
--
paul moore
security @ redhat
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox