[PATCH 5/8] p9auth cleanup

From: Serge Hallyn
Date: Tue Feb 16 2010 - 17:45:39 EST


From: Serge E. Hallyn <serue@xxxxxxxxxx>

Move the code doing the actual uid change into its own helper
function. Next it will become a bit more complicated when I
add primary and auxiliary groups handling.

Split the handling of /dev/capuse and /dev/caphash writes into
their own functions.

Changelog:
Jan 3: fix memory leak in error case

Signed-off-by: Serge E. Hallyn <serue@xxxxxxxxxx>
Cc: Greg KH <greg@xxxxxxxxx>
cc: rsc@xxxxxxxxx
Cc: Ashwin Ganti <ashwin.ganti@xxxxxxxxx>
Cc: ericvh@xxxxxxxxx
Cc: devel@xxxxxxxxxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
Cc: Ron Minnich <rminnich@xxxxxxxxx>
---
drivers/staging/p9auth/p9auth.c | 308 +++++++++++++++++++++------------------
1 files changed, 168 insertions(+), 140 deletions(-)

diff --git a/drivers/staging/p9auth/p9auth.c b/drivers/staging/p9auth/p9auth.c
index fb27459..50447d4 100644
--- a/drivers/staging/p9auth/p9auth.c
+++ b/drivers/staging/p9auth/p9auth.c
@@ -165,26 +165,180 @@ static int cap_release(struct inode *inode, struct file *filp)
return 0;
}

+struct id_set {
+ char *source_user, *target_user;
+ char *randstr;
+ uid_t old_uid, new_uid;
+ char *full; /* The full entry which must be freed */
+};
+
+/*
+ * read an entry. For now it is
+ * source_user@target_user@rand
+ * Next it will become
+ * source_user@target_user@target_group@numgroups@grp1..@grpn@rand
+ */
+static int parse_user_capability(char *s, struct id_set *set)
+{
+ char *tmpu;
+
+ /*
+ * break the supplied string into tokens with @ as the
+ * delimiter If the string is "user1@user2@randomstring" we
+ * need to split it and hash 'user1@user2' using 'randomstring'
+ * as the key.
+ */
+ tmpu = set->full = kstrdup(s, GFP_KERNEL);
+ if (!tmpu)
+ return -ENOMEM;
+
+ set->source_user = strsep(&tmpu, "@");
+ set->target_user = strsep(&tmpu, "@");
+ set->randstr = tmpu;
+ if (!set->source_user || !set->target_user || !set->randstr) {
+ kfree(set->full);
+ return -EINVAL;
+ }
+
+ set->new_uid = simple_strtoul(set->target_user, NULL, 0);
+ set->old_uid = simple_strtoul(set->source_user, NULL, 0);
+
+ return 0;
+}
+
+static int grant_id(struct id_set *set)
+{
+ struct cred *new;
+ int ret;
+
+ /*
+ * Check whether the process writing to capuse
+ * is actually owned by the source owner
+ */
+ if (set->old_uid != current_uid()) {
+ printk(KERN_ALERT
+ "Process is not owned by the source user of the capability.\n");
+ return -EFAULT;
+ }
+
+ /*
+ * Change uid, euid, and fsuid. The suid remains for
+ * flexibility - though I'm torn as to the tradeoff of
+ * usefulness vs. danger in that.
+ */
+ new = prepare_creds();
+ if (!new)
+ return -ENOMEM;
+
+ ret = cred_setresuid(new, set->new_uid, set->new_uid, set->new_uid,
+ CRED_SETID_FORCE);
+ if (ret == 0)
+ commit_creds(new);
+ else
+ abort_creds(new);
+
+ return ret;
+}
+
+static int add_caphash_entry(struct cap_dev *dev, char *user_buf, size_t count)
+{
+ struct cap_node *node_ptr;
+
+ if (count > CAP_NODE_SIZE)
+ return -EINVAL;
+ if (!capable(CAP_GRANT_ID))
+ return -EPERM;
+ node_ptr = kmalloc(sizeof(struct cap_node), GFP_KERNEL);
+ if (!node_ptr)
+ return -ENOMEM;
+
+ printk(KERN_INFO "Capability being written to /dev/caphash : \n");
+ hexdump(user_buf, count);
+ memcpy(node_ptr->data, user_buf, count);
+ list_add(&(node_ptr->list), &(dev->head->list));
+
+ return 0;
+}
+
+static int use_caphash_entry(struct cap_dev *dev, char *user_buf)
+{
+ struct cap_node *node;
+ struct id_set set;
+ int ret, len, found = 0;
+ char *tohash, *hashed;
+ struct list_head *pos;
+
+ if (!cap_devices[0].head)
+ return -EINVAL;
+ if (list_empty(&(cap_devices[0].head->list)))
+ return -EINVAL;
+
+ ret = parse_user_capability(user_buf, &set);
+ if (ret)
+ return ret;
+
+ /* hash the string user1@user2 with randstr as the key */
+ len = strlen(set.source_user) + strlen(set.target_user) + 1;
+ /* src, @, len, \0 */
+ tohash = kzalloc(len+1, GFP_KERNEL);
+ if (!tohash) {
+ kfree(set.full);
+ return -ENOMEM;
+ }
+ strcat(tohash, set.source_user);
+ strcat(tohash, "@");
+ strcat(tohash, set.target_user);
+ printk(KERN_ALERT "the source user is %s \n", set.source_user);
+ printk(KERN_ALERT "the target user is %s \n", set.target_user);
+ hashed = cap_hash(tohash, len, set.randstr, strlen(set.randstr));
+ kfree(set.full);
+ kfree(tohash);
+ if (NULL == hashed)
+ return -EFAULT;
+
+ /* Change the process's uid if the hash is present in the
+ * list of hashes
+ */
+ list_for_each(pos, &(cap_devices->head->list)) {
+ /*
+ * Change the user id of the process if the hashes
+ * match
+ */
+ node = list_entry(pos, struct cap_node, list);
+ if (0 == memcmp(hashed, node->data, CAP_NODE_SIZE)) {
+ ret = grant_id(&set);
+ if (ret < 0)
+ goto out;
+
+ /* Capability may only be used once */
+ list_del(pos);
+ kfree(node);
+ found = 1;
+ break;
+ }
+ }
+ if (!found) {
+ printk(KERN_ALERT
+ "Invalid capabiliy written to /dev/capuse\n");
+ ret = -EFAULT;
+ }
+out:
+ kfree(hashed);
+ return ret;
+}
+
static ssize_t cap_write(struct file *filp, const char __user *buf,
size_t count, loff_t *f_pos)
{
- struct cap_node *node_ptr, *tmp;
- struct list_head *pos;
struct cap_dev *dev = filp->private_data;
ssize_t retval = -ENOMEM;
- struct cred *new;
- int len, target_int, source_int, flag = 0;
- char *user_buf, *user_buf_running, *source_user, *target_user,
- *rand_str, *hash_str, *result;
+ char *user_buf;

if (down_interruptible(&dev->sem))
return -ERESTARTSYS;

- user_buf_running = NULL;
- hash_str = NULL;
- node_ptr = kmalloc(sizeof(struct cap_node), GFP_KERNEL);
user_buf = kzalloc(count+1, GFP_KERNEL);
- if (!node_ptr || !user_buf)
+ if (!user_buf)
goto out;

if (copy_from_user(user_buf, buf, count)) {
@@ -196,134 +350,11 @@ static ssize_t cap_write(struct file *filp, const char __user *buf,
* If the minor number is 0 ( /dev/caphash ) then simply add the
* hashed capability supplied by the user to the list of hashes
*/
- if (0 == iminor(filp->f_dentry->d_inode)) {
- if (count > CAP_NODE_SIZE) {
- retval = -EINVAL;
- goto out;
- }
- if (!capable(CAP_GRANT_ID)) {
- retval = -EPERM;
- goto out;
- }
- printk(KERN_INFO "Capability being written to /dev/caphash : \n");
- hexdump(user_buf, count);
- memcpy(node_ptr->data, user_buf, count);
- list_add(&(node_ptr->list), &(dev->head->list));
- node_ptr = NULL;
- } else {
- char *tmpu;
- if (!cap_devices[0].head ||
- list_empty(&(cap_devices[0].head->list))) {
- retval = -EINVAL;
- goto out;
- }
- /*
- * break the supplied string into tokens with @ as the
- * delimiter If the string is "user1@user2@randomstring" we
- * need to split it and hash 'user1@user2' using 'randomstring'
- * as the key.
- */
- tmpu = user_buf_running = kstrdup(user_buf, GFP_KERNEL);
- source_user = strsep(&tmpu, "@");
- target_user = strsep(&tmpu, "@");
- rand_str = tmpu;
- if (!source_user || !target_user || !rand_str) {
- retval = -EINVAL;
- goto out;
- }
+ if (0 == iminor(filp->f_dentry->d_inode))
+ retval = add_caphash_entry(dev, user_buf, count);
+ else
+ retval = use_caphash_entry(dev, user_buf);

- /* hash the string user1@user2 with rand_str as the key */
- len = strlen(source_user) + strlen(target_user) + 1;
- /* src, @, len, \0 */
- hash_str = kzalloc(len+1, GFP_KERNEL);
- strcat(hash_str, source_user);
- strcat(hash_str, "@");
- strcat(hash_str, target_user);
-
- printk(KERN_ALERT "the source user is %s \n", source_user);
- printk(KERN_ALERT "the target user is %s \n", target_user);
-
- result = cap_hash(hash_str, len, rand_str, strlen(rand_str));
- if (NULL == result) {
- retval = -EFAULT;
- goto out;
- }
- memcpy(node_ptr->data, result, CAP_NODE_SIZE); /* why? */
- /* Change the process's uid if the hash is present in the
- * list of hashes
- */
- list_for_each(pos, &(cap_devices->head->list)) {
- /*
- * Change the user id of the process if the hashes
- * match
- */
- if (0 ==
- memcmp(result,
- list_entry(pos, struct cap_node,
- list)->data,
- CAP_NODE_SIZE)) {
- target_int = (unsigned int)
- simple_strtol(target_user, NULL, 0);
- source_int = (unsigned int)
- simple_strtol(source_user, NULL, 0);
- flag = 1;
-
- /*
- * Check whether the process writing to capuse
- * is actually owned by the source owner
- */
- if (source_int != current_uid()) {
- printk(KERN_ALERT
- "Process is not owned by the source user of the capability.\n");
- retval = -EFAULT;
- goto out;
- }
- /*
- * Change all uids. It might be useful to
- * keep suid unchanged, however that will
- * mean that changing from uid=0 to uid=!0
- * pP is not emptied (only pE is), and when
- * changing from uid=!0 to uid=0, sets are
- * not filled. They will be correct after
- * the next exec, but this is IMO not
- * sufficient. So change all uids.
- */
- new = prepare_creds();
- if (!new) {
- retval = -ENOMEM;
- goto out;
- }
- retval = cred_setresuid(new, target_int,
- target_int, target_int,
- CRED_SETID_FORCE);
- if (retval == 0)
- commit_creds(new);
- else {
- abort_creds(new);
- goto out;
- }
-
- /*
- * Remove the capability from the list and
- * break
- */
- tmp = list_entry(pos, struct cap_node, list);
- list_del(pos);
- kfree(tmp);
- break;
- }
- }
- if (0 == flag) {
- /*
- * The capability is not present in the list of the
- * hashes stored, hence return failure
- */
- printk(KERN_ALERT
- "Invalid capabiliy written to /dev/capuse \n");
- retval = -EFAULT;
- goto out;
- }
- }
*f_pos += count;
retval = count;
/* update the size */
@@ -331,10 +362,7 @@ static ssize_t cap_write(struct file *filp, const char __user *buf,
dev->size = *f_pos;

out:
- kfree(node_ptr);
kfree(user_buf);
- kfree(user_buf_running);
- kfree(hash_str);
up(&dev->sem);
return retval;
}
--
1.6.1

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