From 54f617de7f85d04b74c766147ea06f6407beb84d Mon Sep 17 00:00:00 2001 From: Jiamin Ma Date: Fri, 20 Oct 2017 16:25:07 +0800 Subject: [PATCH] unifykey: fix arbitrary memory access in unifykeys ioctl PD#152036: arbitrary memory access in unifykeys ioctl Change-Id: I2e76906bef8f417909d97c8a04289ce38baa6087 Signed-off-by: Jiamin Ma --- drivers/amlogic/unifykey/unifykey.c | 79 +++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 22 deletions(-) diff --git a/drivers/amlogic/unifykey/unifykey.c b/drivers/amlogic/unifykey/unifykey.c index 3ca70acccddb..60ffa32547be 100644 --- a/drivers/amlogic/unifykey/unifykey.c +++ b/drivers/amlogic/unifykey/unifykey.c @@ -736,19 +736,24 @@ static long unifykey_unlocked_ioctl(struct file *file, switch (cmd) { case KEYUNIFY_ATTACH: { - struct key_item_t appitem; - char initvalue[KEY_UNIFY_NAME_LEN]; + struct key_item_t *appitem; int ret; - ret = copy_from_user(&appitem, argp, sizeof(appitem)); + appitem = kmalloc(sizeof(struct key_item_t), + GFP_KERNEL); + if (!appitem) + return -ENOMEM; + ret = copy_from_user(appitem, argp, + sizeof(struct key_item_t)); if (ret != 0) { pr_err("%s:%d,copy_from_user fail\n", __func__, __LINE__); + kfree(appitem); return ret; } - //appitem = (struct key_item_t *)arg; - memcpy(initvalue, appitem.name, KEY_UNIFY_NAME_LEN); - ret = key_unify_init(initvalue, KEY_UNIFY_NAME_LEN); + ret = key_unify_init(appitem->name, + KEY_UNIFY_NAME_LEN); + kfree(appitem); if (ret < 0) { pr_err("%s:%d,key unify init fail\n", __func__, __LINE__); @@ -762,20 +767,31 @@ static long unifykey_unlocked_ioctl(struct file *file, unsigned int index, reallen; unsigned int keypermit, keystate; struct key_item_t *kkey; - struct key_item_info_t key_item_info; + struct key_item_info_t *key_item_info; char *keyname; int ret; - ret = copy_from_user(&key_item_info, - argp, sizeof(key_item_info)); + + key_item_info = kmalloc(sizeof(struct key_item_info_t), + GFP_KERNEL); + if (!key_item_info) + return -ENOMEM; + ret = copy_from_user(key_item_info, argp, + sizeof(struct key_item_info_t)); if (ret != 0) { pr_err("%s:%d,copy_from_user fail\n", __func__, __LINE__); + kfree(key_item_info); return ret; } - //key_item_info = (struct key_item_info_t *)arg; - index = key_item_info.id; - keyname = key_item_info.name; + index = key_item_info->id; + keyname = key_item_info->name; + if (strlen(keyname) > KEY_UNIFY_NAME_LEN - 1) { + pr_err("%s() keyname invalid\n", __func__); + kfree(key_item_info); + return -EINVAL; + } + if (strlen(keyname)) kkey = unifykey_find_item_by_name(keyname); else @@ -783,7 +799,8 @@ static long unifykey_unlocked_ioctl(struct file *file, if (kkey == NULL) { pr_err("%s() %d, find name fail\n", __func__, __LINE__); - return -ENOTTY; + kfree(key_item_info); + return -EINVAL; } pr_err("%s() %d, %d, %s\n", __func__, __LINE__, kkey->id, kkey->name); @@ -793,36 +810,40 @@ static long unifykey_unlocked_ioctl(struct file *file, if (ret < 0) { pr_err("%s() %d, get size fail\n", __func__, __LINE__); + kfree(key_item_info); return -EFAULT; } - key_item_info.permit = keypermit; - key_item_info.flag = keystate; - key_item_info.id = kkey->id; - strncpy(key_item_info.name, - kkey->name, strlen(kkey->name)); + key_item_info->permit = keypermit; + key_item_info->flag = keystate; + key_item_info->id = kkey->id; + strncpy(key_item_info->name, + kkey->name, KEY_UNIFY_NAME_LEN); ret = key_unify_size(kkey->name, &reallen); if (ret < 0) { pr_err("%s() %d, get size fail\n", __func__, __LINE__); + kfree(key_item_info); return -EFAULT; } /* set key info */ - key_item_info.size = reallen; + key_item_info->size = reallen; - ret = copy_to_user(argp, - &key_item_info, sizeof(key_item_info)); + ret = copy_to_user(argp, key_item_info, + sizeof(struct key_item_info_t)); if (ret != 0) { pr_err("%s:%d,copy_to_user fail\n", __func__, __LINE__); + kfree(key_item_info); return ret; } + kfree(key_item_info); return 0; } break; default: pr_err("%s() %d\n", __func__, __LINE__); - return -ENOTTY; + return -EINVAL; } return 0; } @@ -1082,6 +1103,11 @@ static ssize_t name_store(struct class *cla, if (count >= KEY_UNIFY_NAME_LEN) count = KEY_UNIFY_NAME_LEN - 1; + if (count <= 0) { + pr_err(" count=%ld is invalid in %s\n", count, __func__); + return -EINVAL; + } + key_cnt = unifykey_count_key(); name = kzalloc(count, GFP_KERNEL); if (!name) { @@ -1195,6 +1221,10 @@ static ssize_t write_store(struct class *cla, unsigned char *keydata = NULL; size_t key_len = 0; + if (count <= 0) { + pr_err(" count=%ld is invalid in %s\n", count, __func__); + return -EINVAL; + } if (curkey != NULL) { keydata = kzalloc(count, GFP_KERNEL); @@ -1275,6 +1305,11 @@ static ssize_t lock_store(struct class *cla, { unsigned int state, len; + if (count <= 0) { + pr_err(" count=%ld is invalid in %s\n", count, __func__); + return -EINVAL; + } + /* check '\n' and del */ if (buf[count - 1] == '\n') len = count - 1;