Subject: [PATCH] p9caps: add Plan9 capability devices

From: "Enrico Weigelt, metux IT consult" <[email protected]>

This driver implements the Plan9 capability devices, used for
switching user id via capability tokens.

https://9p.io/sys/doc/auth.html
---
drivers/staging/Kconfig | 2 +
drivers/staging/Makefile | 1 +
drivers/staging/p9caps/Kconfig | 11 ++
drivers/staging/p9caps/Makefile | 1 +
drivers/staging/p9caps/p9caps.c | 371 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 386 insertions(+)
create mode 100644 drivers/staging/p9caps/Kconfig
create mode 100644 drivers/staging/p9caps/Makefile
create mode 100644 drivers/staging/p9caps/p9caps.c

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 554683912cff..23f325339fe8 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -118,4 +118,6 @@ source "drivers/staging/vboxvideo/Kconfig"

source "drivers/staging/pi433/Kconfig"

+source "drivers/staging/p9caps/Kconfig"
+
endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 6e536020029a..eccdf4643453 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -3,6 +3,7 @@

obj-y += media/
obj-y += typec/
+obj-$(CONFIG_PLAN9CAPS) += p9caps/
obj-$(CONFIG_IRDA) += irda/net/
obj-$(CONFIG_IRDA) += irda/drivers/
obj-$(CONFIG_PRISM2_USB) += wlan-ng/
diff --git a/drivers/staging/p9caps/Kconfig b/drivers/staging/p9caps/Kconfig
new file mode 100644
index 000000000000..455c3fa726ff
--- /dev/null
+++ b/drivers/staging/p9caps/Kconfig
@@ -0,0 +1,11 @@
+config PLAN9CAPS
+ tristate "Plan 9 capability device"
+ default n
+ select CRYPTO_HMAC
+ select CRYPTO_SHA1
+ help
+ This module implements the Plan 9 capability devices
+ /dev/caphash and /dev/capuse
+
+ To compile this driver as a module, choose
+ M here: the module will be called p9auth.
diff --git a/drivers/staging/p9caps/Makefile b/drivers/staging/p9caps/Makefile
new file mode 100644
index 000000000000..67d38099a249
--- /dev/null
+++ b/drivers/staging/p9caps/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_PLAN9CAPS) += p9caps.o
diff --git a/drivers/staging/p9caps/p9caps.c b/drivers/staging/p9caps/p9caps.c
new file mode 100644
index 000000000000..4c5c94dc1893
--- /dev/null
+++ b/drivers/staging/p9caps/p9caps.c
@@ -0,0 +1,371 @@
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/errno.h>
+#include <linux/fcntl.h>
+#include <linux/cdev.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/string.h>
+#include <linux/scatterlist.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/user_namespace.h>
+#include <linux/mutex.h>
+#include <crypto/hash.h>
+#include <crypto/sha.h>
+
+/*
+ * Plan9 /dev/caphash and /dev/capuse device
+ *
+ * 2DO: - caphash should only allow one process (per userns)
+ * - support textual user names
+ * - invalidate old caps
+ */
+
+#define DEVICE_CAPUSE "/dev/capuse"
+#define DEVICE_CAPHASH "/dev/caphash"
+
+#define MODNAME "p9cap"
+
+struct caphash_entry {
+ struct list_head list;
+ struct user_namespace *user_ns;
+ char data[SHA1_DIGEST_SIZE];
+};
+
+struct caphash_writer {
+ struct list_head list;
+ struct user_namespace *user_ns;
+};
+
+static dev_t caphash_devid = 0;
+static dev_t capuse_devid = 0;
+
+static LIST_HEAD(caphash_entries);
+static LIST_HEAD(caphash_writers);
+
+static DEFINE_MUTEX(p9cap_lock);
+
+struct crypto_ahash *p9cap_tfm = NULL;
+
+static int caphash_open(struct inode *inode, struct file *filp)
+{
+ struct caphash_writer *tmp = NULL;
+ struct user_namespace *user_ns = current_user_ns();
+ int retval = 0;
+ struct list_head *pos, *q;
+
+ /* make sure only one instance per namespace can be opened */
+ mutex_lock(&p9cap_lock);
+
+ list_for_each_safe(pos, q, &(caphash_writers)) {
+ tmp = list_entry(pos, struct caphash_writer, list);
+ if (tmp->user_ns == user_ns) {
+ printk(KERN_ERR DEVICE_CAPHASH ": already locked in this namespace\n");
+ retval = -EBUSY;
+ goto out;
+ }
+ }
+
+ if (!(tmp = kzalloc(sizeof(struct caphash_writer), GFP_KERNEL))) {
+ retval = -ENOMEM;
+ goto out;
+ }
+
+ tmp->user_ns = get_user_ns(user_ns);
+ list_add(&(tmp->list), &caphash_writers);
+
+out:
+ mutex_unlock(&p9cap_lock);
+ return retval;
+}
+
+static int caphash_release(struct inode *inode, struct file *filp)
+{
+ int retval = 0;
+ struct user_namespace *user_ns = current_user_ns();
+ struct list_head *pos, *q;
+ struct caphash_entry *tmp;
+
+ mutex_lock(&p9cap_lock);
+
+ list_for_each_safe(pos, q, &(caphash_writers)) {
+ tmp = list_entry(pos, struct caphash_entry, list);
+ if (tmp->user_ns == user_ns) {
+ list_del(pos);
+ kfree(tmp);
+ goto out;
+ }
+ }
+
+out:
+ mutex_unlock(&p9cap_lock);
+ return retval;
+}
+
+static ssize_t caphash_write(struct file *filp, const char __user *buf,
+ size_t count, loff_t *f_pos)
+{
+ struct caphash_entry *ent;
+
+ if (count > SHA1_DIGEST_SIZE) {
+ printk(KERN_ERR DEVICE_CAPHASH ": too large: %d\n", count);
+ return -E2BIG;
+ }
+
+ if (!(ent = kzalloc(sizeof(struct caphash_entry), GFP_KERNEL)))
+ return -ENOMEM;
+
+ if (copy_from_user(&(ent->data), buf, count)) {
+ kfree(ent);
+ return -EFAULT;
+ }
+
+ ent->user_ns = get_user_ns(current_user_ns());
+
+ mutex_lock(&p9cap_lock);
+ list_add(&(ent->list), &caphash_entries);
+ mutex_unlock(&p9cap_lock);
+
+ return count;
+}
+
+/* called w/ lock held. we can releave this by allocating tfm locally */
+static ssize_t hash(const char *src, const char* dst, const char *key, u8 *result)
+{
+ struct scatterlist sg;
+ struct ahash_request *req;
+ int retval;
+ char *text = NULL;
+ size_t text_len;
+ int digest_len;
+ u8* digest = NULL;
+
+ text_len = strlen(src)+strlen(dst)+1; /* src@dst\0 */
+ digest_len = crypto_ahash_reqsize(p9cap_tfm);
+
+ digest = kzalloc(digest_len, GFP_KERNEL);
+ text = kzalloc(text_len+1, GFP_KERNEL);
+
+ if (!digest || !text) {
+ retval = -ENOMEM;
+ goto out;
+ }
+
+ if (!(req = ahash_request_alloc(p9cap_tfm, GFP_KERNEL))) {
+ printk(KERN_ERR MODNAME ": failed to alloc ahash_request\n");
+ retval = -ENOMEM;
+ goto out;
+ }
+
+ snprintf(text, text_len+1, "%s@%s", src, dst);
+ sg_set_buf(&sg, text, text_len);
+
+ ahash_request_set_callback(req, 0, NULL, NULL);
+ ahash_request_set_crypt(req, &sg, digest, text_len);
+
+ if ((retval = crypto_ahash_setkey(p9cap_tfm, key, strlen(key)))) {
+ printk(KERN_ERR MODNAME ": crypto_ahash_setkey() failed ret=%d\n", retval);
+ goto out;
+ }
+
+ if ((retval = crypto_ahash_digest(req))) {
+ printk(KERN_ERR MODNAME ": digest() failed ret=%d\n", retval);
+ goto out;
+ }
+
+ memcpy(result, digest, SHA1_DIGEST_SIZE);
+
+out:
+ kfree(text);
+ kfree(digest);
+
+ return 0;
+}
+
+static inline kuid_t convert_uid(const char* uname)
+{
+ return make_kuid(current_user_ns(), simple_strtol(uname, NULL, 0));
+}
+
+static ssize_t switch_uid(const char *src_uname, const char *dst_uname)
+{
+ struct cred *creds = prepare_creds();
+
+ kuid_t src_uid = convert_uid(src_uname);
+ kuid_t dst_uid = convert_uid(dst_uname);
+
+ if (!uid_eq(src_uid, current_uid())) {
+ printk(KERN_INFO DEVICE_CAPUSE ": src uid mismatch\n");
+ return -EPERM;
+ }
+
+ if (!(creds = prepare_creds()))
+ return -ENOMEM;
+
+ creds->uid = dst_uid;
+ creds->euid = dst_uid;
+
+ printk(KERN_INFO DEVICE_CAPUSE ": switching from kuid %d to %d\n", src_uid.val, dst_uid.val);
+ return commit_creds(creds);
+}
+
+static ssize_t try_switch(const char* src_uname, const char* dst_uname, const u8* hashval)
+{
+ struct list_head *pos;
+ list_for_each(pos, &(caphash_entries)) {
+ struct caphash_entry *tmp = list_entry(pos, struct caphash_entry, list);
+ if ((0 == memcmp(hashval, tmp->data, SHA1_DIGEST_SIZE)) &&
+ (tmp->user_ns == current_user_ns())) {
+
+ int retval;
+
+ if ((retval = switch_uid(src_uname, dst_uname))) {
+ printk(KERN_INFO DEVICE_CAPUSE ": uid switch failed\n");
+ return retval;
+ }
+
+ tmp = list_entry(pos, struct caphash_entry, list);
+ list_del(pos);
+ put_user_ns(tmp->user_ns);
+ kfree(tmp);
+
+ return 0;
+ }
+ }
+
+ printk(KERN_INFO DEVICE_CAPUSE ": cap not found\n");
+
+ return -ENOENT;
+}
+
+static ssize_t capuse_write(struct file *filp, const char __user *buf,
+ size_t count, loff_t *f_pos)
+{
+ ssize_t retval = count;
+ char *rand_str, *src_uname, *dst_uname;
+ u8 hashval[SHA1_DIGEST_SIZE] = { 0 };
+ char *cmdbuf;
+
+ if (!(cmdbuf = kzalloc(count, GFP_KERNEL)))
+ return -ENOMEM;
+
+ if (copy_from_user(cmdbuf, buf, count)) {
+ retval = -EFAULT;
+ goto out_free;
+ }
+
+ {
+ char *walk = cmdbuf;
+ src_uname = strsep(&walk, "@");
+ dst_uname = strsep(&walk, "@");
+ rand_str = walk;
+ if (!src_uname || !dst_uname || !rand_str) {
+ retval = -EINVAL;
+ goto out_free;
+ }
+ }
+
+ mutex_lock(&p9cap_lock);
+
+ if ((retval = hash(src_uname, dst_uname, rand_str, hashval)))
+ goto out_unlock;
+
+ if ((retval = try_switch(src_uname, dst_uname, hashval)))
+ goto out_unlock;
+
+ retval = count;
+
+out_unlock:
+ mutex_unlock(&p9cap_lock);
+
+out_free:
+ kfree(cmdbuf);
+ return retval;
+}
+
+static const struct file_operations p9cap_caphash_fops = {
+ .owner = THIS_MODULE,
+ .write = caphash_write,
+ .open = caphash_open,
+ .release = caphash_release,
+};
+
+static const struct file_operations p9cap_capuse_fops = {
+ .owner = THIS_MODULE,
+ .write = capuse_write,
+};
+
+static struct cdev p9cap_dev_caphash;
+static struct cdev p9cap_dev_capuse;
+
+static int p9cap_clear(void)
+{
+ struct caphash_entry *tmp;
+ struct list_head *pos, *q;
+
+ list_for_each_safe(pos, q, &(caphash_entries)) {
+ tmp = list_entry(pos, struct caphash_entry, list);
+ list_del(pos);
+ kfree(tmp);
+ }
+
+ return 0;
+}
+
+static void p9cap_cleanup_module(void)
+{
+ p9cap_clear();
+
+ cdev_del(&p9cap_dev_caphash);
+ cdev_del(&p9cap_dev_capuse);
+
+ unregister_chrdev_region(caphash_devid, 1);
+ unregister_chrdev_region(capuse_devid, 1);
+
+ if (p9cap_tfm)
+ crypto_free_ahash(p9cap_tfm);
+}
+
+static int p9cap_init_module(void)
+{
+ int retval;
+
+ p9cap_tfm = crypto_alloc_ahash("hmac(sha1)", 0, CRYPTO_ALG_ASYNC);
+ if (IS_ERR(p9cap_tfm)) {
+ retval = -PTR_ERR(p9cap_tfm);
+ printk(KERN_ERR MODNAME ": failed to load transform for hmac(sha1): %d\n", retval);
+ goto fail;
+ }
+
+ if ((retval = alloc_chrdev_region(&caphash_devid, 0, 1, DEVICE_CAPHASH)))
+ goto fail;
+
+ if ((retval = alloc_chrdev_region(&capuse_devid, 0, 1, DEVICE_CAPUSE)))
+ goto fail;
+
+ cdev_init(&p9cap_dev_caphash, &p9cap_caphash_fops);
+ p9cap_dev_caphash.owner = THIS_MODULE;
+ if ((retval = cdev_add(&p9cap_dev_caphash, caphash_devid, 1)))
+ printk(KERN_ERR MODNAME ": failed adding " DEVICE_CAPHASH ": %d\n", retval);
+
+ cdev_init(&p9cap_dev_capuse, &p9cap_capuse_fops);
+ p9cap_dev_capuse.owner = THIS_MODULE;
+ if ((retval = cdev_add(&p9cap_dev_capuse, capuse_devid, 1)))
+ printk(KERN_ERR MODNAME ": failed adding " DEVICE_CAPUSE ": %d\n", retval);
+
+ return 0;
+
+fail:
+ p9cap_cleanup_module();
+ return retval;
+}
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <[email protected]>");
+MODULE_LICENSE("GPLv3");
+
+module_init(p9cap_init_module);
+module_exit(p9cap_cleanup_module);
--
2.11.0



2018-02-10 17:55:38

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] p9caps: add Plan9 capability devices

Hi,

On 02/10/2018 08:58 AM, Enrico Weigelt, metux IT consult wrote:
> From: "Enrico Weigelt, metux IT consult" <[email protected]>
>
> This driver implements the Plan9 capability devices, used for
> switching user id via capability tokens.
>
> https://9p.io/sys/doc/auth.html
> ---
> drivers/staging/Kconfig | 2 +
> drivers/staging/Makefile | 1 +
> drivers/staging/p9caps/Kconfig | 11 ++
> drivers/staging/p9caps/Makefile | 1 +
> drivers/staging/p9caps/p9caps.c | 371 ++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 386 insertions(+)
> create mode 100644 drivers/staging/p9caps/Kconfig
> create mode 100644 drivers/staging/p9caps/Makefile
> create mode 100644 drivers/staging/p9caps/p9caps.c
>
> diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> index 554683912cff..23f325339fe8 100644
> --- a/drivers/staging/Kconfig
> +++ b/drivers/staging/Kconfig
> @@ -118,4 +118,6 @@ source "drivers/staging/vboxvideo/Kconfig"
>
> source "drivers/staging/pi433/Kconfig"
>
> +source "drivers/staging/p9caps/Kconfig"
> +
> endif # STAGING
> diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> index 6e536020029a..eccdf4643453 100644
> --- a/drivers/staging/Makefile
> +++ b/drivers/staging/Makefile
> @@ -3,6 +3,7 @@
>
> obj-y += media/
> obj-y += typec/
> +obj-$(CONFIG_PLAN9CAPS) += p9caps/
> obj-$(CONFIG_IRDA) += irda/net/
> obj-$(CONFIG_IRDA) += irda/drivers/
> obj-$(CONFIG_PRISM2_USB) += wlan-ng/
> diff --git a/drivers/staging/p9caps/Kconfig b/drivers/staging/p9caps/Kconfig
> new file mode 100644
> index 000000000000..455c3fa726ff
> --- /dev/null
> +++ b/drivers/staging/p9caps/Kconfig
> @@ -0,0 +1,11 @@
> +config PLAN9CAPS
> + tristate "Plan 9 capability device"
> + default n
> + select CRYPTO_HMAC
> + select CRYPTO_SHA1
> + help
> + This module implements the Plan 9 capability devices
> + /dev/caphash and /dev/capuse
> +
> + To compile this driver as a module, choose
> + M here: the module will be called p9auth.

Just below here (Makefile), it's called p9caps, not p9auth.

> diff --git a/drivers/staging/p9caps/Makefile b/drivers/staging/p9caps/Makefile
> new file mode 100644
> index 000000000000..67d38099a249
> --- /dev/null
> +++ b/drivers/staging/p9caps/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_PLAN9CAPS) += p9caps.o
> diff --git a/drivers/staging/p9caps/p9caps.c b/drivers/staging/p9caps/p9caps.c
> new file mode 100644
> index 000000000000..4c5c94dc1893
> --- /dev/null
> +++ b/drivers/staging/p9caps/p9caps.c
> @@ -0,0 +1,371 @@
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/errno.h>
> +#include <linux/fcntl.h>
> +#include <linux/cdev.h>
> +#include <linux/list.h>
> +#include <linux/mm.h>
> +#include <linux/string.h>
> +#include <linux/scatterlist.h>
> +#include <linux/cred.h>
> +#include <linux/err.h>
> +#include <linux/user_namespace.h>
> +#include <linux/mutex.h>
> +#include <crypto/hash.h>
> +#include <crypto/sha.h>
> +
> +/*
> + * Plan9 /dev/caphash and /dev/capuse device
> + *
> + * 2DO: - caphash should only allow one process (per userns)
> + * - support textual user names
> + * - invalidate old caps
> + */
> +
> +#define DEVICE_CAPUSE "/dev/capuse"
> +#define DEVICE_CAPHASH "/dev/caphash"
> +
> +#define MODNAME "p9cap"

p9caps ?

> +
> +struct caphash_entry {
> + struct list_head list;
> + struct user_namespace *user_ns;
> + char data[SHA1_DIGEST_SIZE];
> +};
> +
> +struct caphash_writer {
> + struct list_head list;
> + struct user_namespace *user_ns;
> +};
> +
> +static dev_t caphash_devid = 0;
> +static dev_t capuse_devid = 0;
> +
> +static LIST_HEAD(caphash_entries);
> +static LIST_HEAD(caphash_writers);
> +
> +static DEFINE_MUTEX(p9cap_lock);
> +
> +struct crypto_ahash *p9cap_tfm = NULL;
> +
> +static int caphash_open(struct inode *inode, struct file *filp)
> +{
> + struct caphash_writer *tmp = NULL;
> + struct user_namespace *user_ns = current_user_ns();
> + int retval = 0;
> + struct list_head *pos, *q;
> +
> + /* make sure only one instance per namespace can be opened */
> + mutex_lock(&p9cap_lock);
> +
> + list_for_each_safe(pos, q, &(caphash_writers)) {
> + tmp = list_entry(pos, struct caphash_writer, list);
> + if (tmp->user_ns == user_ns) {
> + printk(KERN_ERR DEVICE_CAPHASH ": already locked in this namespace\n");
> + retval = -EBUSY;
> + goto out;
> + }
> + }
> +
> + if (!(tmp = kzalloc(sizeof(struct caphash_writer), GFP_KERNEL))) {
> + retval = -ENOMEM;
> + goto out;
> + }
> +
> + tmp->user_ns = get_user_ns(user_ns);
> + list_add(&(tmp->list), &caphash_writers);
> +
> +out:
> + mutex_unlock(&p9cap_lock);
> + return retval;
> +}
> +
> +static int caphash_release(struct inode *inode, struct file *filp)
> +{
> + int retval = 0;
> + struct user_namespace *user_ns = current_user_ns();
> + struct list_head *pos, *q;
> + struct caphash_entry *tmp;
> +
> + mutex_lock(&p9cap_lock);
> +
> + list_for_each_safe(pos, q, &(caphash_writers)) {
> + tmp = list_entry(pos, struct caphash_entry, list);
> + if (tmp->user_ns == user_ns) {
> + list_del(pos);
> + kfree(tmp);
> + goto out;
> + }
> + }
> +
> +out:
> + mutex_unlock(&p9cap_lock);
> + return retval;
> +}
> +
> +static ssize_t caphash_write(struct file *filp, const char __user *buf,
> + size_t count, loff_t *f_pos)
> +{
> + struct caphash_entry *ent;
> +
> + if (count > SHA1_DIGEST_SIZE) {
> + printk(KERN_ERR DEVICE_CAPHASH ": too large: %d\n", count);
> + return -E2BIG;
> + }
> +
> + if (!(ent = kzalloc(sizeof(struct caphash_entry), GFP_KERNEL)))
> + return -ENOMEM;
> +
> + if (copy_from_user(&(ent->data), buf, count)) {
> + kfree(ent);
> + return -EFAULT;
> + }
> +
> + ent->user_ns = get_user_ns(current_user_ns());
> +
> + mutex_lock(&p9cap_lock);
> + list_add(&(ent->list), &caphash_entries);
> + mutex_unlock(&p9cap_lock);
> +
> + return count;
> +}
> +
> +/* called w/ lock held. we can releave this by allocating tfm locally */

releave? relieve?

> +static ssize_t hash(const char *src, const char* dst, const char *key, u8 *result)
> +{
> + struct scatterlist sg;
> + struct ahash_request *req;
> + int retval;
> + char *text = NULL;
> + size_t text_len;
> + int digest_len;
> + u8* digest = NULL;
> +
> + text_len = strlen(src)+strlen(dst)+1; /* src@dst\0 */

Does one of src/dst already contain the @ sign? If not, I think this needs + 2
instead of +1.
Reading more below, + 2 seems to be needed. (1 for @, 1 for \0)

> + digest_len = crypto_ahash_reqsize(p9cap_tfm);
> +
> + digest = kzalloc(digest_len, GFP_KERNEL);
> + text = kzalloc(text_len+1, GFP_KERNEL);
> +
> + if (!digest || !text) {
> + retval = -ENOMEM;
> + goto out;
> + }
> +
> + if (!(req = ahash_request_alloc(p9cap_tfm, GFP_KERNEL))) {
> + printk(KERN_ERR MODNAME ": failed to alloc ahash_request\n");
> + retval = -ENOMEM;
> + goto out;
> + }
> +
> + snprintf(text, text_len+1, "%s@%s", src, dst);
> + sg_set_buf(&sg, text, text_len);
> +
> + ahash_request_set_callback(req, 0, NULL, NULL);
> + ahash_request_set_crypt(req, &sg, digest, text_len);
> +
> + if ((retval = crypto_ahash_setkey(p9cap_tfm, key, strlen(key)))) {
> + printk(KERN_ERR MODNAME ": crypto_ahash_setkey() failed ret=%d\n", retval);
> + goto out;
> + }
> +
> + if ((retval = crypto_ahash_digest(req))) {
> + printk(KERN_ERR MODNAME ": digest() failed ret=%d\n", retval);
> + goto out;
> + }
> +
> + memcpy(result, digest, SHA1_DIGEST_SIZE);
> +
> +out:
> + kfree(text);
> + kfree(digest);
> +
> + return 0;
> +}
> +
> +static inline kuid_t convert_uid(const char* uname)
> +{
> + return make_kuid(current_user_ns(), simple_strtol(uname, NULL, 0));
> +}
> +
> +static ssize_t switch_uid(const char *src_uname, const char *dst_uname)
> +{
> + struct cred *creds = prepare_creds();
> +
> + kuid_t src_uid = convert_uid(src_uname);
> + kuid_t dst_uid = convert_uid(dst_uname);
> +
> + if (!uid_eq(src_uid, current_uid())) {
> + printk(KERN_INFO DEVICE_CAPUSE ": src uid mismatch\n");
> + return -EPERM;
> + }
> +
> + if (!(creds = prepare_creds()))
> + return -ENOMEM;
> +
> + creds->uid = dst_uid;
> + creds->euid = dst_uid;
> +
> + printk(KERN_INFO DEVICE_CAPUSE ": switching from kuid %d to %d\n", src_uid.val, dst_uid.val);
> + return commit_creds(creds);
> +}
> +
> +static ssize_t try_switch(const char* src_uname, const char* dst_uname, const u8* hashval)
> +{
> + struct list_head *pos;
> + list_for_each(pos, &(caphash_entries)) {
> + struct caphash_entry *tmp = list_entry(pos, struct caphash_entry, list);
> + if ((0 == memcmp(hashval, tmp->data, SHA1_DIGEST_SIZE)) &&
> + (tmp->user_ns == current_user_ns())) {
> +
> + int retval;
> +
> + if ((retval = switch_uid(src_uname, dst_uname))) {
> + printk(KERN_INFO DEVICE_CAPUSE ": uid switch failed\n");
> + return retval;
> + }
> +
> + tmp = list_entry(pos, struct caphash_entry, list);
> + list_del(pos);
> + put_user_ns(tmp->user_ns);
> + kfree(tmp);
> +
> + return 0;
> + }
> + }
> +
> + printk(KERN_INFO DEVICE_CAPUSE ": cap not found\n");
> +
> + return -ENOENT;
> +}
> +
> +static ssize_t capuse_write(struct file *filp, const char __user *buf,
> + size_t count, loff_t *f_pos)
> +{
> + ssize_t retval = count;
> + char *rand_str, *src_uname, *dst_uname;
> + u8 hashval[SHA1_DIGEST_SIZE] = { 0 };
> + char *cmdbuf;
> +
> + if (!(cmdbuf = kzalloc(count, GFP_KERNEL)))
> + return -ENOMEM;
> +
> + if (copy_from_user(cmdbuf, buf, count)) {
> + retval = -EFAULT;
> + goto out_free;
> + }
> +
> + {
> + char *walk = cmdbuf;
> + src_uname = strsep(&walk, "@");
> + dst_uname = strsep(&walk, "@");
> + rand_str = walk;
> + if (!src_uname || !dst_uname || !rand_str) {
> + retval = -EINVAL;
> + goto out_free;
> + }
> + }
> +
> + mutex_lock(&p9cap_lock);
> +
> + if ((retval = hash(src_uname, dst_uname, rand_str, hashval)))
> + goto out_unlock;
> +
> + if ((retval = try_switch(src_uname, dst_uname, hashval)))
> + goto out_unlock;
> +
> + retval = count;
> +
> +out_unlock:
> + mutex_unlock(&p9cap_lock);
> +
> +out_free:
> + kfree(cmdbuf);
> + return retval;
> +}
> +
> +static const struct file_operations p9cap_caphash_fops = {
> + .owner = THIS_MODULE,
> + .write = caphash_write,
> + .open = caphash_open,
> + .release = caphash_release,
> +};
> +
> +static const struct file_operations p9cap_capuse_fops = {
> + .owner = THIS_MODULE,
> + .write = capuse_write,
> +};
> +
> +static struct cdev p9cap_dev_caphash;
> +static struct cdev p9cap_dev_capuse;
> +
> +static int p9cap_clear(void)
> +{
> + struct caphash_entry *tmp;
> + struct list_head *pos, *q;
> +
> + list_for_each_safe(pos, q, &(caphash_entries)) {
> + tmp = list_entry(pos, struct caphash_entry, list);
> + list_del(pos);
> + kfree(tmp);
> + }
> +
> + return 0;
> +}
> +
> +static void p9cap_cleanup_module(void)
> +{
> + p9cap_clear();
> +
> + cdev_del(&p9cap_dev_caphash);
> + cdev_del(&p9cap_dev_capuse);
> +
> + unregister_chrdev_region(caphash_devid, 1);
> + unregister_chrdev_region(capuse_devid, 1);
> +
> + if (p9cap_tfm)
> + crypto_free_ahash(p9cap_tfm);
> +}
> +
> +static int p9cap_init_module(void)
> +{
> + int retval;
> +
> + p9cap_tfm = crypto_alloc_ahash("hmac(sha1)", 0, CRYPTO_ALG_ASYNC);
> + if (IS_ERR(p9cap_tfm)) {
> + retval = -PTR_ERR(p9cap_tfm);
> + printk(KERN_ERR MODNAME ": failed to load transform for hmac(sha1): %d\n", retval);

pr_err() would be useful here and other places. [pr_warn(), etc.]
see <linux/printk.h>

> + goto fail;
> + }
> +
> + if ((retval = alloc_chrdev_region(&caphash_devid, 0, 1, DEVICE_CAPHASH)))
> + goto fail;
> +
> + if ((retval = alloc_chrdev_region(&capuse_devid, 0, 1, DEVICE_CAPUSE)))
> + goto fail;
> +
> + cdev_init(&p9cap_dev_caphash, &p9cap_caphash_fops);
> + p9cap_dev_caphash.owner = THIS_MODULE;
> + if ((retval = cdev_add(&p9cap_dev_caphash, caphash_devid, 1)))
> + printk(KERN_ERR MODNAME ": failed adding " DEVICE_CAPHASH ": %d\n", retval);
> +
> + cdev_init(&p9cap_dev_capuse, &p9cap_capuse_fops);
> + p9cap_dev_capuse.owner = THIS_MODULE;
> + if ((retval = cdev_add(&p9cap_dev_capuse, capuse_devid, 1)))
> + printk(KERN_ERR MODNAME ": failed adding " DEVICE_CAPUSE ": %d\n", retval);
> +
> + return 0;
> +
> +fail:
> + p9cap_cleanup_module();
> + return retval;
> +}
> +
> +MODULE_AUTHOR("Enrico Weigelt, metux IT consult <[email protected]>");
> +MODULE_LICENSE("GPLv3");

<linux/module.h> does not list GPLv3 as being one of the acceptable licenses
for the Linux kernel.

> +
> +module_init(p9cap_init_module);
> +module_exit(p9cap_cleanup_module);
>


--
~Randy

2018-02-10 18:04:51

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] p9caps: add Plan9 capability devices

On Sat, Feb 10, 2018 at 04:58:45PM +0000, Enrico Weigelt, metux IT consult wrote:
> From: "Enrico Weigelt, metux IT consult" <[email protected]>

> +MODULE_LICENSE("GPLv3");

... which is incompatible with GPLv2. I'm not even going to look at that
thing - same as with proprietary code. You CAN'T combine any derivatives
of that code with the kernel. Not without GPL violation.

And yes, there is GPLv2-only code all over the place in Linux VFS (at
least), with at least some of the copyright holders not going to permit
violations of clause 6. Not going to happen.

If you want it in the kernel, you need a GPLv2-compatible license. GPLv3
does not qualify, and while it can be combined with GPLv2+ (yielding GPLv3
for combination), the kernel is not GPLv2+ - some parts are, but GPLv2-only
ones render the whole thing GPLv2-only.

Subject: p9caps: add Plan9 capability devices


v2 of the p9caps patch

Subject: [PATCH] p9caps: add Plan9 capability devices

From: "Enrico Weigelt, metux IT consult" <[email protected]>

This driver implements the Plan9 capability devices, used for
switching user id via capability tokens.

https://9p.io/sys/doc/auth.html
---
drivers/staging/Kconfig | 2 +
drivers/staging/Makefile | 1 +
drivers/staging/p9caps/Kconfig | 11 ++
drivers/staging/p9caps/Makefile | 1 +
drivers/staging/p9caps/p9caps.c | 369 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 384 insertions(+)
create mode 100644 drivers/staging/p9caps/Kconfig
create mode 100644 drivers/staging/p9caps/Makefile
create mode 100644 drivers/staging/p9caps/p9caps.c

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 554683912cff..23f325339fe8 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -118,4 +118,6 @@ source "drivers/staging/vboxvideo/Kconfig"

source "drivers/staging/pi433/Kconfig"

+source "drivers/staging/p9caps/Kconfig"
+
endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 6e536020029a..eccdf4643453 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -3,6 +3,7 @@

obj-y += media/
obj-y += typec/
+obj-$(CONFIG_PLAN9CAPS) += p9caps/
obj-$(CONFIG_IRDA) += irda/net/
obj-$(CONFIG_IRDA) += irda/drivers/
obj-$(CONFIG_PRISM2_USB) += wlan-ng/
diff --git a/drivers/staging/p9caps/Kconfig b/drivers/staging/p9caps/Kconfig
new file mode 100644
index 000000000000..b909daaa79ce
--- /dev/null
+++ b/drivers/staging/p9caps/Kconfig
@@ -0,0 +1,11 @@
+config PLAN9CAPS
+ tristate "Plan 9 capability device"
+ default n
+ select CRYPTO_HMAC
+ select CRYPTO_SHA1
+ help
+ This module implements the Plan 9 capability devices
+ /dev/caphash and /dev/capuse
+
+ To compile this driver as a module, choose
+ M here: the module will be called p9caps.
diff --git a/drivers/staging/p9caps/Makefile b/drivers/staging/p9caps/Makefile
new file mode 100644
index 000000000000..67d38099a249
--- /dev/null
+++ b/drivers/staging/p9caps/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_PLAN9CAPS) += p9caps.o
diff --git a/drivers/staging/p9caps/p9caps.c b/drivers/staging/p9caps/p9caps.c
new file mode 100644
index 000000000000..e46b09821c18
--- /dev/null
+++ b/drivers/staging/p9caps/p9caps.c
@@ -0,0 +1,369 @@
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/errno.h>
+#include <linux/fcntl.h>
+#include <linux/cdev.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/string.h>
+#include <linux/scatterlist.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/user_namespace.h>
+#include <linux/mutex.h>
+#include <crypto/hash.h>
+#include <crypto/sha.h>
+
+/*
+ * Plan9 /dev/caphash and /dev/capuse device
+ *
+ * 2DO: - caphash should only allow one process (per userns)
+ * - support textual user names
+ * - invalidate old caps
+ */
+
+#define DEVICE_CAPUSE "/dev/capuse"
+#define DEVICE_CAPHASH "/dev/caphash"
+
+struct caphash_entry {
+ struct list_head list;
+ struct user_namespace *user_ns;
+ char data[SHA1_DIGEST_SIZE];
+};
+
+struct caphash_writer {
+ struct list_head list;
+ struct user_namespace *user_ns;
+};
+
+static dev_t caphash_devid = 0;
+static dev_t capuse_devid = 0;
+
+static LIST_HEAD(caphash_entries);
+static LIST_HEAD(caphash_writers);
+
+static DEFINE_MUTEX(lock);
+
+struct crypto_ahash *hmac_tfm = NULL;
+
+static int caphash_open(struct inode *inode, struct file *filp)
+{
+ struct caphash_writer *tmp = NULL;
+ struct user_namespace *user_ns = current_user_ns();
+ int retval = 0;
+ struct list_head *pos, *q;
+
+ /* make sure only one instance per namespace can be opened */
+ mutex_lock(&lock);
+
+ list_for_each_safe(pos, q, &(caphash_writers)) {
+ tmp = list_entry(pos, struct caphash_writer, list);
+ if (tmp->user_ns == user_ns) {
+ pr_err("already locked in this namespace\n");
+ retval = -EBUSY;
+ goto out;
+ }
+ }
+
+ if (!(tmp = kzalloc(sizeof(struct caphash_writer), GFP_KERNEL))) {
+ retval = -ENOMEM;
+ goto out;
+ }
+
+ tmp->user_ns = get_user_ns(user_ns);
+ list_add(&(tmp->list), &caphash_writers);
+
+out:
+ mutex_unlock(&lock);
+ return retval;
+}
+
+static int caphash_release(struct inode *inode, struct file *filp)
+{
+ int retval = 0;
+ struct user_namespace *user_ns = current_user_ns();
+ struct list_head *pos, *q;
+ struct caphash_entry *tmp;
+
+ mutex_lock(&lock);
+
+ list_for_each_safe(pos, q, &(caphash_writers)) {
+ tmp = list_entry(pos, struct caphash_entry, list);
+ if (tmp->user_ns == user_ns) {
+ list_del(pos);
+ kfree(tmp);
+ goto out;
+ }
+ }
+
+out:
+ mutex_unlock(&lock);
+ return retval;
+}
+
+static ssize_t caphash_write(struct file *filp, const char __user *buf,
+ size_t count, loff_t *f_pos)
+{
+ struct caphash_entry *ent;
+
+ if (count > SHA1_DIGEST_SIZE) {
+ pr_err("SHA1 digest size too large: %d\n", count);
+ return -E2BIG;
+ }
+
+ if (!(ent = kzalloc(sizeof(struct caphash_entry), GFP_KERNEL)))
+ return -ENOMEM;
+
+ if (copy_from_user(&(ent->data), buf, count)) {
+ kfree(ent);
+ return -EFAULT;
+ }
+
+ ent->user_ns = get_user_ns(current_user_ns());
+
+ mutex_lock(&lock);
+ list_add(&(ent->list), &caphash_entries);
+ mutex_unlock(&lock);
+
+ return count;
+}
+
+/* called w/ lock held. we can relieve this by allocating tfm locally */
+static ssize_t hash(const char *src, const char* dst, const char *key, u8 *result)
+{
+ struct scatterlist sg;
+ struct ahash_request *req;
+ int retval;
+ char *text = NULL;
+ size_t text_len;
+ int digest_len;
+ u8* digest = NULL;
+
+ text_len = strlen(src)+strlen(dst)+1; /* src@dst */
+ digest_len = crypto_ahash_reqsize(hmac_tfm);
+
+ digest = kzalloc(digest_len, GFP_KERNEL);
+ text = kzalloc(text_len+1, GFP_KERNEL);
+
+ if (!digest || !text) {
+ retval = -ENOMEM;
+ goto out;
+ }
+
+ if (!(req = ahash_request_alloc(hmac_tfm, GFP_KERNEL))) {
+ pr_err("failed to alloc ahash_request\n");
+ retval = -ENOMEM;
+ goto out;
+ }
+
+ snprintf(text, text_len+1, "%s@%s", src, dst);
+ sg_set_buf(&sg, text, text_len);
+
+ ahash_request_set_callback(req, 0, NULL, NULL);
+ ahash_request_set_crypt(req, &sg, digest, text_len);
+
+ if ((retval = crypto_ahash_setkey(hmac_tfm, key, strlen(key)))) {
+ pr_err("crypto_ahash_setkey() failed ret=%d\n", retval);
+ goto out;
+ }
+
+ if ((retval = crypto_ahash_digest(req))) {
+ pr_err("digest() failed ret=%d\n", retval);
+ goto out;
+ }
+
+ memcpy(result, digest, SHA1_DIGEST_SIZE);
+
+out:
+ kfree(text);
+ kfree(digest);
+
+ return 0;
+}
+
+static inline kuid_t convert_uid(const char* uname)
+{
+ return make_kuid(current_user_ns(), simple_strtol(uname, NULL, 0));
+}
+
+static ssize_t switch_uid(const char *src_uname, const char *dst_uname)
+{
+ struct cred *creds = prepare_creds();
+
+ kuid_t src_uid = convert_uid(src_uname);
+ kuid_t dst_uid = convert_uid(dst_uname);
+
+ if (!uid_eq(src_uid, current_uid())) {
+ pr_info("src uid mismatch\n");
+ return -EPERM;
+ }
+
+ if (!(creds = prepare_creds()))
+ return -ENOMEM;
+
+ creds->uid = dst_uid;
+ creds->euid = dst_uid;
+
+ pr_info("switching from kuid %d to %d\n", src_uid.val, dst_uid.val);
+ return commit_creds(creds);
+}
+
+static ssize_t try_switch(const char* src_uname, const char* dst_uname, const u8* hashval)
+{
+ struct list_head *pos;
+ list_for_each(pos, &(caphash_entries)) {
+ struct caphash_entry *tmp = list_entry(pos, struct caphash_entry, list);
+ if ((0 == memcmp(hashval, tmp->data, SHA1_DIGEST_SIZE)) &&
+ (tmp->user_ns == current_user_ns())) {
+
+ int retval;
+
+ if ((retval = switch_uid(src_uname, dst_uname))) {
+ pr_info("uid switch failed\n");
+ return retval;
+ }
+
+ tmp = list_entry(pos, struct caphash_entry, list);
+ list_del(pos);
+ put_user_ns(tmp->user_ns);
+ kfree(tmp);
+
+ return 0;
+ }
+ }
+
+ pr_info("cap not found\n");
+
+ return -ENOENT;
+}
+
+static ssize_t capuse_write(struct file *filp, const char __user *buf,
+ size_t count, loff_t *f_pos)
+{
+ ssize_t retval = count;
+ char *rand_str, *src_uname, *dst_uname;
+ u8 hashval[SHA1_DIGEST_SIZE] = { 0 };
+ char *cmdbuf;
+
+ if (!(cmdbuf = kzalloc(count, GFP_KERNEL)))
+ return -ENOMEM;
+
+ if (copy_from_user(cmdbuf, buf, count)) {
+ retval = -EFAULT;
+ goto out_free;
+ }
+
+ {
+ char *walk = cmdbuf;
+ src_uname = strsep(&walk, "@");
+ dst_uname = strsep(&walk, "@");
+ rand_str = walk;
+ if (!src_uname || !dst_uname || !rand_str) {
+ retval = -EINVAL;
+ goto out_free;
+ }
+ }
+
+ mutex_lock(&lock);
+
+ if ((retval = hash(src_uname, dst_uname, rand_str, hashval)))
+ goto out_unlock;
+
+ if ((retval = try_switch(src_uname, dst_uname, hashval)))
+ goto out_unlock;
+
+ retval = count;
+
+out_unlock:
+ mutex_unlock(&lock);
+
+out_free:
+ kfree(cmdbuf);
+ return retval;
+}
+
+static const struct file_operations caphash_fops = {
+ .owner = THIS_MODULE,
+ .write = caphash_write,
+ .open = caphash_open,
+ .release = caphash_release,
+};
+
+static const struct file_operations capuse_fops = {
+ .owner = THIS_MODULE,
+ .write = capuse_write,
+};
+
+static struct cdev caphash_dev;
+static struct cdev capuse_dev;
+
+static int clear(void)
+{
+ struct caphash_entry *tmp;
+ struct list_head *pos, *q;
+
+ list_for_each_safe(pos, q, &(caphash_entries)) {
+ tmp = list_entry(pos, struct caphash_entry, list);
+ list_del(pos);
+ kfree(tmp);
+ }
+
+ return 0;
+}
+
+static void _cleanup_module(void)
+{
+ clear();
+
+ cdev_del(&caphash_dev);
+ cdev_del(&capuse_dev);
+
+ unregister_chrdev_region(caphash_devid, 1);
+ unregister_chrdev_region(capuse_devid, 1);
+
+ if (hmac_tfm)
+ crypto_free_ahash(hmac_tfm);
+}
+
+static int _init_module(void)
+{
+ int retval;
+
+ hmac_tfm = crypto_alloc_ahash("hmac(sha1)", 0, CRYPTO_ALG_ASYNC);
+ if (IS_ERR(hmac_tfm)) {
+ retval = -PTR_ERR(hmac_tfm);
+ pr_err("failed to load transform for hmac(sha1): %d\n", retval);
+ goto fail;
+ }
+
+ if ((retval = alloc_chrdev_region(&caphash_devid, 0, 1, DEVICE_CAPHASH)))
+ goto fail;
+
+ if ((retval = alloc_chrdev_region(&capuse_devid, 0, 1, DEVICE_CAPUSE)))
+ goto fail;
+
+ cdev_init(&caphash_dev, &caphash_fops);
+ caphash_dev.owner = THIS_MODULE;
+ if ((retval = cdev_add(&caphash_dev, caphash_devid, 1)))
+ pr_err("failed adding " DEVICE_CAPHASH ": %d\n", retval);
+
+ cdev_init(&capuse_dev, &capuse_fops);
+ capuse_dev.owner = THIS_MODULE;
+ if ((retval = cdev_add(&capuse_dev, capuse_devid, 1)))
+ pr_err("failed adding " DEVICE_CAPUSE ": %d\n", retval);
+
+ return 0;
+
+fail:
+ _cleanup_module();
+ return retval;
+}
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <[email protected]>");
+MODULE_LICENSE("GPL");
+
+module_init(_init_module);
+module_exit(_cleanup_module);
--
2.11.0


2018-02-13 07:19:07

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH] p9caps: add Plan9 capability devices

On Sun, Feb 11, 2018 at 09:50:28PM +0000, Enrico Weigelt, metux IT consult wrote:
> From: "Enrico Weigelt, metux IT consult" <[email protected]>
>
> This driver implements the Plan9 capability devices, used for
> switching user id via capability tokens.
>
> https://9p.io/sys/doc/auth.html
> ---
> drivers/staging/Kconfig | 2 +
> drivers/staging/Makefile | 1 +
> drivers/staging/p9caps/Kconfig | 11 ++
> drivers/staging/p9caps/Makefile | 1 +
> drivers/staging/p9caps/p9caps.c | 369 ++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 384 insertions(+)
> create mode 100644 drivers/staging/p9caps/Kconfig
> create mode 100644 drivers/staging/p9caps/Makefile
> create mode 100644 drivers/staging/p9caps/p9caps.c
>
> diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> index 554683912cff..23f325339fe8 100644
> --- a/drivers/staging/Kconfig
> +++ b/drivers/staging/Kconfig
> @@ -118,4 +118,6 @@ source "drivers/staging/vboxvideo/Kconfig"
>
> source "drivers/staging/pi433/Kconfig"
>
> +source "drivers/staging/p9caps/Kconfig"
> +
> endif # STAGING
> diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> index 6e536020029a..eccdf4643453 100644
> --- a/drivers/staging/Makefile
> +++ b/drivers/staging/Makefile
> @@ -3,6 +3,7 @@
>
> obj-y += media/
> obj-y += typec/
> +obj-$(CONFIG_PLAN9CAPS) += p9caps/
> obj-$(CONFIG_IRDA) += irda/net/
> obj-$(CONFIG_IRDA) += irda/drivers/
> obj-$(CONFIG_PRISM2_USB) += wlan-ng/
> diff --git a/drivers/staging/p9caps/Kconfig b/drivers/staging/p9caps/Kconfig
> new file mode 100644
> index 000000000000..b909daaa79ce
> --- /dev/null
> +++ b/drivers/staging/p9caps/Kconfig
> @@ -0,0 +1,11 @@
> +config PLAN9CAPS
> + tristate "Plan 9 capability device"
> + default n
> + select CRYPTO_HMAC
> + select CRYPTO_SHA1
> + help
> + This module implements the Plan 9 capability devices
> + /dev/caphash and /dev/capuse
> +
> + To compile this driver as a module, choose
> + M here: the module will be called p9caps.
> diff --git a/drivers/staging/p9caps/Makefile b/drivers/staging/p9caps/Makefile
> new file mode 100644
> index 000000000000..67d38099a249
> --- /dev/null
> +++ b/drivers/staging/p9caps/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_PLAN9CAPS) += p9caps.o
> diff --git a/drivers/staging/p9caps/p9caps.c b/drivers/staging/p9caps/p9caps.c
> new file mode 100644
> index 000000000000..e46b09821c18
> --- /dev/null
> +++ b/drivers/staging/p9caps/p9caps.c
> @@ -0,0 +1,369 @@
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/errno.h>
> +#include <linux/fcntl.h>
> +#include <linux/cdev.h>
> +#include <linux/list.h>
> +#include <linux/mm.h>
> +#include <linux/string.h>
> +#include <linux/scatterlist.h>
> +#include <linux/cred.h>
> +#include <linux/err.h>
> +#include <linux/user_namespace.h>
> +#include <linux/mutex.h>
> +#include <crypto/hash.h>
> +#include <crypto/sha.h>
> +
> +/*
> + * Plan9 /dev/caphash and /dev/capuse device
> + *
> + * 2DO: - caphash should only allow one process (per userns)
> + * - support textual user names
> + * - invalidate old caps
> + */
> +
> +#define DEVICE_CAPUSE "/dev/capuse"
> +#define DEVICE_CAPHASH "/dev/caphash"
> +
> +struct caphash_entry {
> + struct list_head list;
> + struct user_namespace *user_ns;
> + char data[SHA1_DIGEST_SIZE];
> +};
> +
> +struct caphash_writer {
> + struct list_head list;
> + struct user_namespace *user_ns;
> +};
> +
> +static dev_t caphash_devid = 0;
> +static dev_t capuse_devid = 0;
> +
> +static LIST_HEAD(caphash_entries);
> +static LIST_HEAD(caphash_writers);
> +
> +static DEFINE_MUTEX(lock);
> +
> +struct crypto_ahash *hmac_tfm = NULL;
> +
> +static int caphash_open(struct inode *inode, struct file *filp)
> +{
> + struct caphash_writer *tmp = NULL;
> + struct user_namespace *user_ns = current_user_ns();
> + int retval = 0;
> + struct list_head *pos, *q;
> +
> + /* make sure only one instance per namespace can be opened */

... at a time

might be better to keep this state in the user_ns itself, would
avoid kzalloc below.

Would it be worth doing any privilege checking here?

(incidentally, for historical reference, https://lkml.org/lkml/2010/4/20/404 :)

> + mutex_lock(&lock);
> +
> + list_for_each_safe(pos, q, &(caphash_writers)) {
> + tmp = list_entry(pos, struct caphash_writer, list);
> + if (tmp->user_ns == user_ns) {
> + pr_err("already locked in this namespace\n");
> + retval = -EBUSY;
> + goto out;
> + }
> + }
> +
> + if (!(tmp = kzalloc(sizeof(struct caphash_writer), GFP_KERNEL))) {
> + retval = -ENOMEM;
> + goto out;
> + }
> +
> + tmp->user_ns = get_user_ns(user_ns);
> + list_add(&(tmp->list), &caphash_writers);
> +
> +out:
> + mutex_unlock(&lock);
> + return retval;
> +}

Subject: Re: [PATCH] p9caps: add Plan9 capability devices

On 13.02.2018 07:16, Serge E. Hallyn wrote:
>> + /* make sure only one instance per namespace can be opened */ > > ... at a time
yeah, right.

> might be better to keep this state in the user_ns itself, would
> avoid kzalloc below.

thought about, but hesitated to touch user_ns. might not be the best
idea when having p9caps as module (OTOH, doesn't need to be a module)

the whole thing might become a bit more complex when introducing
plan9-like unprivileged mount operations. haven't sorted out how to
do that yet.

> Would it be worth doing any privilege checking here?

Which ones should I check ?


--mtx

2018-02-14 14:58:15

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH] p9caps: add Plan9 capability devices

Quoting Enrico Weigelt, metux IT consult ([email protected]):
> On 13.02.2018 07:16, Serge E. Hallyn wrote:
> >>+ /* make sure only one instance per namespace can be opened */ > > ... at a time
> yeah, right.
>
> >might be better to keep this state in the user_ns itself, would
> >avoid kzalloc below.
>
> thought about, but hesitated to touch user_ns. might not be the best
> idea when having p9caps as module (OTOH, doesn't need to be a module)

If it's an out of tree module you'd have to do it this way, but if
it's in-tree, even as a module, adding a bit to the userns struct
would imo be ok.

> the whole thing might become a bit more complex when introducing
> plan9-like unprivileged mount operations. haven't sorted out how to
> do that yet.

I hope you'll have a discussion here about that first.

> >Would it be worth doing any privilege checking here?
>
> Which ones should I check ?

Well, granting privileges to another task is extra-special, so
in the 2010 submission we created a new POSIX capability for
it (CAP_GRANT_ID).

Now speaking practically, I love the caphash idea, but it does
have issues with a modern login system. There are privileged
things which login needs to do besides changing uid, including but
not limited to:
1. setting limits
2. setting loginuid,
3. mounting things (polyinstantiated /tmp, decrypted homedir, etc)
4. setting selinux context

These are probably all solvable, and don't need to be solved before
we solve this part, that would be silly, but they're worth thinking
about. The ones which must be done on the current task are the hardest
to solve.

(and of course gplv3 as Al pointed out is a blocker)

-serge

Subject: Re: [PATCH] p9caps: add Plan9 capability devices

On 14.02.2018 15:56, Serge E. Hallyn wrote:

> If it's an out of tree module you'd have to do it this way, but if > it's in-tree, even as a module, adding a bit to the userns struct>
would imo be ok.
Assuming one doesn't try to load the module when the kernel image
previously was built w/o it ;-) (well, could export some dummy
symbol for protection ;-)).

OTOH, that raises the question, where / how exactly the cap list
destruction / expiry should be done. My original plan was adding
a timer in the p9caps module that just scans for old entries.

Should the userns code just call back on userns destruction ?
(in that case it would be tricky to have it as a module)

>> the whole thing might become a bit more complex when introducing
>> plan9-like unprivileged mount operations. haven't sorted out how to
>> do that yet.
>
> I hope you'll have a discussion here about that first.

Yes, of course - that's why I'm here :p

My current idea is introducing some special flag for disabling suid
completely and switch into an private namespace, where now the
unprivileged user can mount at will and create new mnt namespaces,
just like on Plan9.

I'll try some qnd hacks w/ a new syscall, lets see where it leads to,
and then sort out how to do that in a more appropriate way.

> Now speaking practically, I love the caphash idea, but it does
> have issues with a modern login system. There are privileged
> things which login needs to do besides changing uid, including but
> not limited to:
> 1. setting limits
> 2. setting loginuid,
> 3. mounting things (polyinstantiated /tmp, decrypted homedir, etc)
> 4. setting selinux context

For now, I don't think that's necessary for doing things the Plan9 way.
Perhaps we later could extend the /dev/caphash interface w/ additional
parameters for that.

> (and of course gplv3 as Al pointed out is a blocker)

already fixed.


--mtx

--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2018-02-17 22:40:01

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] p9caps: add Plan9 capability devices

On Sun, Feb 11, 2018 at 10:50 PM, Enrico Weigelt, metux IT consult
<[email protected]> wrote:
> From: "Enrico Weigelt, metux IT consult" <[email protected]>
>
> This driver implements the Plan9 capability devices, used for
> switching user id via capability tokens.
>
> https://9p.io/sys/doc/auth.html

Please see some nit-picks below.
I'm still reading the plan9 docs to fully get the big picture.

> ---
> drivers/staging/Kconfig | 2 +
> drivers/staging/Makefile | 1 +
> drivers/staging/p9caps/Kconfig | 11 ++
> drivers/staging/p9caps/Makefile | 1 +
> drivers/staging/p9caps/p9caps.c | 369 ++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 384 insertions(+)
> create mode 100644 drivers/staging/p9caps/Kconfig
> create mode 100644 drivers/staging/p9caps/Makefile
> create mode 100644 drivers/staging/p9caps/p9caps.c
>
> diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> index 554683912cff..23f325339fe8 100644
> --- a/drivers/staging/Kconfig
> +++ b/drivers/staging/Kconfig
> @@ -118,4 +118,6 @@ source "drivers/staging/vboxvideo/Kconfig"
>
> source "drivers/staging/pi433/Kconfig"
>
> +source "drivers/staging/p9caps/Kconfig"
> +
> endif # STAGING
> diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> index 6e536020029a..eccdf4643453 100644
> --- a/drivers/staging/Makefile
> +++ b/drivers/staging/Makefile
> @@ -3,6 +3,7 @@
>
> obj-y += media/
> obj-y += typec/
> +obj-$(CONFIG_PLAN9CAPS) += p9caps/
> obj-$(CONFIG_IRDA) += irda/net/
> obj-$(CONFIG_IRDA) += irda/drivers/
> obj-$(CONFIG_PRISM2_USB) += wlan-ng/
> diff --git a/drivers/staging/p9caps/Kconfig b/drivers/staging/p9caps/Kconfig
> new file mode 100644
> index 000000000000..b909daaa79ce
> --- /dev/null
> +++ b/drivers/staging/p9caps/Kconfig
> @@ -0,0 +1,11 @@
> +config PLAN9CAPS
> + tristate "Plan 9 capability device"
> + default n
> + select CRYPTO_HMAC
> + select CRYPTO_SHA1
> + help
> + This module implements the Plan 9 capability devices
> + /dev/caphash and /dev/capuse
> +
> + To compile this driver as a module, choose
> + M here: the module will be called p9caps.
> diff --git a/drivers/staging/p9caps/Makefile b/drivers/staging/p9caps/Makefile
> new file mode 100644
> index 000000000000..67d38099a249
> --- /dev/null
> +++ b/drivers/staging/p9caps/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_PLAN9CAPS) += p9caps.o
> diff --git a/drivers/staging/p9caps/p9caps.c b/drivers/staging/p9caps/p9caps.c
> new file mode 100644
> index 000000000000..e46b09821c18
> --- /dev/null
> +++ b/drivers/staging/p9caps/p9caps.c
> @@ -0,0 +1,369 @@
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/errno.h>
> +#include <linux/fcntl.h>
> +#include <linux/cdev.h>
> +#include <linux/list.h>
> +#include <linux/mm.h>
> +#include <linux/string.h>
> +#include <linux/scatterlist.h>
> +#include <linux/cred.h>
> +#include <linux/err.h>
> +#include <linux/user_namespace.h>
> +#include <linux/mutex.h>
> +#include <crypto/hash.h>
> +#include <crypto/sha.h>
> +
> +/*
> + * Plan9 /dev/caphash and /dev/capuse device
> + *
> + * 2DO: - caphash should only allow one process (per userns)
> + * - support textual user names
> + * - invalidate old caps
> + */
> +
> +#define DEVICE_CAPUSE "/dev/capuse"
> +#define DEVICE_CAPHASH "/dev/caphash"
> +
> +struct caphash_entry {
> + struct list_head list;
> + struct user_namespace *user_ns;
> + char data[SHA1_DIGEST_SIZE];
> +};
> +
> +struct caphash_writer {
> + struct list_head list;
> + struct user_namespace *user_ns;
> +};
> +
> +static dev_t caphash_devid = 0;
> +static dev_t capuse_devid = 0;

Static vars are already 0, no need to initialize them.

> +static LIST_HEAD(caphash_entries);
> +static LIST_HEAD(caphash_writers);
> +
> +static DEFINE_MUTEX(lock);
> +
> +struct crypto_ahash *hmac_tfm = NULL;
> +
> +static int caphash_open(struct inode *inode, struct file *filp)
> +{
> + struct caphash_writer *tmp = NULL;
> + struct user_namespace *user_ns = current_user_ns();
> + int retval = 0;
> + struct list_head *pos, *q;
> +
> + /* make sure only one instance per namespace can be opened */
> + mutex_lock(&lock);
> +
> + list_for_each_safe(pos, q, &(caphash_writers)) {
> + tmp = list_entry(pos, struct caphash_writer, list);
> + if (tmp->user_ns == user_ns) {
> + pr_err("already locked in this namespace\n");

So, evil guy opens the device but does not close it, therefore the
whole service is blocked in a namespace?

In general, I think we should open that device in
kernel_init_freeable() and hand over the fd to init/systemd.
As the plan9 docs state "The write-only file /dev/caphash can be
opened only by the host owner, and only once. Factotum opens this file
immediately after booting."

> + retval = -EBUSY;
> + goto out;
> + }
> + }
> +
> + if (!(tmp = kzalloc(sizeof(struct caphash_writer), GFP_KERNEL))) {
> + retval = -ENOMEM;
> + goto out;
> + }
> +
> + tmp->user_ns = get_user_ns(user_ns);
> + list_add(&(tmp->list), &caphash_writers);
> +
> +out:
> + mutex_unlock(&lock);
> + return retval;
> +}
> +
> +static int caphash_release(struct inode *inode, struct file *filp)
> +{
> + int retval = 0;
> + struct user_namespace *user_ns = current_user_ns();

Why not obtaining the user namespace from the open file?
That way one can close a caphash file hande she never opened.
Think of open, followed by nsenter, ...

> + struct list_head *pos, *q;
> + struct caphash_entry *tmp;
> +
> + mutex_lock(&lock);
> +
> + list_for_each_safe(pos, q, &(caphash_writers)) {
> + tmp = list_entry(pos, struct caphash_entry, list);

list_for_each_entry.

> + if (tmp->user_ns == user_ns) {
> + list_del(pos);
> + kfree(tmp);
> + goto out;
> + }
> + }
> +
> +out:
> + mutex_unlock(&lock);
> + return retval;
> +}
> +
> +static ssize_t caphash_write(struct file *filp, const char __user *buf,
> + size_t count, loff_t *f_pos)
> +{
> + struct caphash_entry *ent;
> +
> + if (count > SHA1_DIGEST_SIZE) {
> + pr_err("SHA1 digest size too large: %d\n", count);
> + return -E2BIG;
> + }
> +
> + if (!(ent = kzalloc(sizeof(struct caphash_entry), GFP_KERNEL)))
> + return -ENOMEM;
> +
> + if (copy_from_user(&(ent->data), buf, count)) {
> + kfree(ent);
> + return -EFAULT;
> + }
> +
> + ent->user_ns = get_user_ns(current_user_ns());

Again, why not taking the user namespace from the file handle?

> + mutex_lock(&lock);
> + list_add(&(ent->list), &caphash_entries);
> + mutex_unlock(&lock);
> +
> + return count;
> +}
> +
> +/* called w/ lock held. we can relieve this by allocating tfm locally */
> +static ssize_t hash(const char *src, const char* dst, const char *key, u8 *result)
> +{
> + struct scatterlist sg;
> + struct ahash_request *req;
> + int retval;
> + char *text = NULL;
> + size_t text_len;
> + int digest_len;
> + u8* digest = NULL;
> +
> + text_len = strlen(src)+strlen(dst)+1; /* src@dst */
> + digest_len = crypto_ahash_reqsize(hmac_tfm);
> +
> + digest = kzalloc(digest_len, GFP_KERNEL);
> + text = kzalloc(text_len+1, GFP_KERNEL);
> +
> + if (!digest || !text) {
> + retval = -ENOMEM;
> + goto out;
> + }
> +
> + if (!(req = ahash_request_alloc(hmac_tfm, GFP_KERNEL))) {
> + pr_err("failed to alloc ahash_request\n");
> + retval = -ENOMEM;
> + goto out;
> + }
> +
> + snprintf(text, text_len+1, "%s@%s", src, dst);
> + sg_set_buf(&sg, text, text_len);
> +
> + ahash_request_set_callback(req, 0, NULL, NULL);
> + ahash_request_set_crypt(req, &sg, digest, text_len);
> +
> + if ((retval = crypto_ahash_setkey(hmac_tfm, key, strlen(key)))) {
> + pr_err("crypto_ahash_setkey() failed ret=%d\n", retval);
> + goto out;
> + }
> +
> + if ((retval = crypto_ahash_digest(req))) {
> + pr_err("digest() failed ret=%d\n", retval);
> + goto out;
> + }
> +
> + memcpy(result, digest, SHA1_DIGEST_SIZE);
> +
> +out:
> + kfree(text);
> + kfree(digest);
> +
> + return 0;
> +}
> +
> +static inline kuid_t convert_uid(const char* uname)
> +{
> + return make_kuid(current_user_ns(), simple_strtol(uname, NULL, 0));
> +}
> +
> +static ssize_t switch_uid(const char *src_uname, const char *dst_uname)
> +{
> + struct cred *creds = prepare_creds();
> +
> + kuid_t src_uid = convert_uid(src_uname);
> + kuid_t dst_uid = convert_uid(dst_uname);
> +
> + if (!uid_eq(src_uid, current_uid())) {
> + pr_info("src uid mismatch\n");
> + return -EPERM;
> + }
> +
> + if (!(creds = prepare_creds()))
> + return -ENOMEM;
> +
> + creds->uid = dst_uid;
> + creds->euid = dst_uid;
> +
> + pr_info("switching from kuid %d to %d\n", src_uid.val, dst_uid.val);
> + return commit_creds(creds);
> +}
> +
> +static ssize_t try_switch(const char* src_uname, const char* dst_uname, const u8* hashval)
> +{
> + struct list_head *pos;
> + list_for_each(pos, &(caphash_entries)) {
> + struct caphash_entry *tmp = list_entry(pos, struct caphash_entry, list);
> + if ((0 == memcmp(hashval, tmp->data, SHA1_DIGEST_SIZE)) &&
> + (tmp->user_ns == current_user_ns())) {
> +
> + int retval;
> +
> + if ((retval = switch_uid(src_uname, dst_uname))) {
> + pr_info("uid switch failed\n");
> + return retval;
> + }
> +
> + tmp = list_entry(pos, struct caphash_entry, list);
> + list_del(pos);
> + put_user_ns(tmp->user_ns);
> + kfree(tmp);
> +
> + return 0;
> + }
> + }
> +
> + pr_info("cap not found\n");
> +
> + return -ENOENT;
> +}
> +
> +static ssize_t capuse_write(struct file *filp, const char __user *buf,
> + size_t count, loff_t *f_pos)
> +{
> + ssize_t retval = count;
> + char *rand_str, *src_uname, *dst_uname;
> + u8 hashval[SHA1_DIGEST_SIZE] = { 0 };
> + char *cmdbuf;
> +
> + if (!(cmdbuf = kzalloc(count, GFP_KERNEL)))

count is unbound, please apply a limit check.

> + return -ENOMEM;
> +
> + if (copy_from_user(cmdbuf, buf, count)) {
> + retval = -EFAULT;
> + goto out_free;
> + }
> +
> + {
> + char *walk = cmdbuf;

cmdbuf is toxic, make sure it is at least nul-terminated.

> + src_uname = strsep(&walk, "@");
> + dst_uname = strsep(&walk, "@");
> + rand_str = walk;
> + if (!src_uname || !dst_uname || !rand_str) {
> + retval = -EINVAL;
> + goto out_free;
> + }
> + }
> +
> + mutex_lock(&lock);
> +
> + if ((retval = hash(src_uname, dst_uname, rand_str, hashval)))
> + goto out_unlock;
> +
> + if ((retval = try_switch(src_uname, dst_uname, hashval)))
> + goto out_unlock;
> +
> + retval = count;
> +
> +out_unlock:
> + mutex_unlock(&lock);
> +
> +out_free:
> + kfree(cmdbuf);
> + return retval;
> +}
> +
> +static const struct file_operations caphash_fops = {
> + .owner = THIS_MODULE,
> + .write = caphash_write,
> + .open = caphash_open,
> + .release = caphash_release,
> +};
> +
> +static const struct file_operations capuse_fops = {
> + .owner = THIS_MODULE,
> + .write = capuse_write,
> +};
> +
> +static struct cdev caphash_dev;
> +static struct cdev capuse_dev;
> +
> +static int clear(void)
> +{
> + struct caphash_entry *tmp;
> + struct list_head *pos, *q;
> +
> + list_for_each_safe(pos, q, &(caphash_entries)) {
> + tmp = list_entry(pos, struct caphash_entry, list);

list_for_each_entry_safe?

> + list_del(pos);
> + kfree(tmp);
> + }
> +
> + return 0;
> +}
> +
> +static void _cleanup_module(void)
> +{
> + clear();
> +
> + cdev_del(&caphash_dev);
> + cdev_del(&capuse_dev);
> +
> + unregister_chrdev_region(caphash_devid, 1);
> + unregister_chrdev_region(capuse_devid, 1);
> +
> + if (hmac_tfm)

IS_ERR()? Otherwise you free a error value, if crypto_alloc_ahash() fails.

> + crypto_free_ahash(hmac_tfm);
> +}
> +
> +static int _init_module(void)
> +{
> + int retval;
> +
> + hmac_tfm = crypto_alloc_ahash("hmac(sha1)", 0, CRYPTO_ALG_ASYNC);
> + if (IS_ERR(hmac_tfm)) {
> + retval = -PTR_ERR(hmac_tfm);
> + pr_err("failed to load transform for hmac(sha1): %d\n", retval);
> + goto fail;
> + }
> +
> + if ((retval = alloc_chrdev_region(&caphash_devid, 0, 1, DEVICE_CAPHASH)))
> + goto fail;
> +
> + if ((retval = alloc_chrdev_region(&capuse_devid, 0, 1, DEVICE_CAPUSE)))
> + goto fail;
> +
> + cdev_init(&caphash_dev, &caphash_fops);
> + caphash_dev.owner = THIS_MODULE;
> + if ((retval = cdev_add(&caphash_dev, caphash_devid, 1)))
> + pr_err("failed adding " DEVICE_CAPHASH ": %d\n", retval);
> +
> + cdev_init(&capuse_dev, &capuse_fops);
> + capuse_dev.owner = THIS_MODULE;
> + if ((retval = cdev_add(&capuse_dev, capuse_devid, 1)))
> + pr_err("failed adding " DEVICE_CAPUSE ": %d\n", retval);
> +
> + return 0;
> +
> +fail:
> + _cleanup_module();
> + return retval;
> +}
> +
> +MODULE_AUTHOR("Enrico Weigelt, metux IT consult <[email protected]>");
> +MODULE_LICENSE("GPL");
> +
> +module_init(_init_module);
> +module_exit(_cleanup_module);
> --
> 2.11.0
>

--
Thanks,
//richard

Subject: Re: [PATCH] p9caps: add Plan9 capability devices

On 17.02.2018 23:11, Richard Weinberger wrote:

Hi,

>> +static LIST_HEAD(caphash_writers); >> +>> +static DEFINE_MUTEX(lock);>> +>> +struct crypto_ahash *hmac_tfm
= NULL;>> +>> +static int caphash_open(struct inode *inode, struct file
*filp)>> +{>> + struct caphash_writer *tmp = NULL;>> +
struct user_namespace *user_ns = current_user_ns();>> + int retval
= 0;>> + struct list_head *pos, *q;>> +>> + /* make sure
only one instance per namespace can be opened */>> +
mutex_lock(&lock);>> +>> + list_for_each_safe(pos, q,
&(caphash_writers)) {>> + tmp = list_entry(pos, struct
caphash_writer, list);>> + if (tmp->user_ns == user_ns)
{>> + pr_err("already locked in this namespace\n");>
> So, evil guy opens the device but does not close it, therefore the
> whole service is blocked in a namespace?

Yes, exactly as specified. There may be only one host factotum running,
which can create caps. It's an important security feature.

> In general, I think we should open that device in
> kernel_init_freeable() and hand over the fd to init/systemd.

That would require an customized init and factotum, and wouldn't be
Plan9 compatible.

>> +static int caphash_release(struct inode *inode, struct file *filp)
>> +{
>> + int retval = 0;
>> + struct user_namespace *user_ns = current_user_ns();
>
> Why not obtaining the user namespace from the open file?
> That way one can close a caphash file hande she never opened.
> Think of open, followed by nsenter, ...

hmm, good point.

>> + list_for_each_safe(pos, q, &(caphash_writers)) {
>> + tmp = list_entry(pos, struct caphash_entry, list);
>
> list_for_each_entry.

what's the exact difference ?

>> +static ssize_t capuse_write(struct file *filp, const char __user *buf,
>> + size_t count, loff_t *f_pos)
>> +{
>> + ssize_t retval = count;
>> + char *rand_str, *src_uname, *dst_uname;
>> + u8 hashval[SHA1_DIGEST_SIZE] = { 0 };
>> + char *cmdbuf;
>> +
>> + if (!(cmdbuf = kzalloc(count, GFP_KERNEL)))
>
> count is unbound, please apply a limit check.

ok.

>> + {
>> + char *walk = cmdbuf;
>
> cmdbuf is toxic, make sure it is at least nul-terminated.

ok.

>> + if (hmac_tfm)
>
> IS_ERR()? Otherwise you free a error value, if crypto_alloc_ahash() fails.

ok


--mtx

--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2018-04-25 12:25:25

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] p9caps: add Plan9 capability devices

Am Mittwoch, 25. April 2018, 12:38:02 CEST schrieb Enrico Weigelt:
> >> + list_for_each_safe(pos, q, &(caphash_writers)) {
> >> + tmp = list_entry(pos, struct caphash_entry, list);
> >
> > list_for_each_entry.
>
> what's the exact difference ?

Whoops, I meant list_for_each_entry_safe().
You don't list_entry() then.

Thanks,
//richard

--
sigma star gmbh - Eduard-Bodem-Gasse 6 - 6020 Innsbruck - Austria
ATU66964118 - FN 374287y