security: fix security issues about mem corruption and info leakage
authorJiamin Ma <jiamin.ma@amlogic.com>
Wed, 6 Sep 2017 03:50:07 +0000 (11:50 +0800)
committerJianxin Pan <jianxin.pan@amlogic.com>
Mon, 18 Sep 2017 03:06:01 +0000 (20:06 -0700)
PD#150578: kernel4.9 secure review issues
1. Kernel Debugging Options Enabled

2. Exploit Mitigations (KASLR, Stack Protector)
   Not Enabled In Kernel Config

3. Buffer Overflow in Amvideo Driver file_name
   SysFS Attribute

4. Memory Corruption in VFM Driver SysFS Attribute

5. Kernel Syslog Accessible To  Unprivileged Users

6. Kernel Memory Corruption in EFuse Driver

7. Kernel Memory Corruption in Unifykey IOCTL

Change-Id: I277c449f4cb14141bf37f90fd66764b8dccaaee0
Signed-off-by: Jiamin Ma <jiamin.ma@amlogic.com>
arch/arm64/configs/meson64_defconfig
drivers/amlogic/efuse/efuse.c
drivers/amlogic/efuse/efuse64.c
drivers/amlogic/media/common/vfm/vfm.c
drivers/amlogic/media/video_sink/video.c
drivers/amlogic/unifykey/unifykey.c

index 54a2c55..6abd781 100644 (file)
@@ -18,10 +18,12 @@ CONFIG_CGROUP_CPUACCT=y
 CONFIG_CGROUP_SCHED=y
 CONFIG_RT_GROUP_SCHED=y
 CONFIG_BLK_DEV_INITRD=y
+CONFIG_KALLSYMS_ALL=y
 CONFIG_EMBEDDED=y
 # CONFIG_COMPAT_BRK is not set
 CONFIG_PROFILING=y
 CONFIG_JUMP_LABEL=y
+CONFIG_CC_STACKPROTECTOR_STRONG=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 CONFIG_PCI=y
@@ -520,8 +522,7 @@ CONFIG_DEBUG_ATOMIC_SLEEP=y
 CONFIG_FTRACE_SYSCALLS=y
 CONFIG_STACK_TRACER=y
 CONFIG_FUNCTION_PROFILER=y
-CONFIG_KGDB=y
-CONFIG_KGDB_TESTS=y
+CONFIG_SECURITY_DMESG_RESTRICT=y
 CONFIG_SECURITY_PERF_EVENTS_RESTRICT=y
 CONFIG_SECURITY=y
 CONFIG_SECURITYFS=y
index 35109c6..55b4e4c 100644 (file)
@@ -101,13 +101,30 @@ loff_t efuse_llseek(struct file *filp, loff_t off, int whence)
 static long efuse_unlocked_ioctl(struct file *file, unsigned int cmd,
        unsigned long arg)
 {
-       struct efuseinfo_item_t *info;
+       void __user             *argp = (void __user *)arg;
+       struct efuseinfo_item_t info;
+       int                     ret;
+
 
        switch (cmd) {
        case EFUSE_INFO_GET:
-               info = (struct efuseinfo_item_t *)arg;
-               if (efuse_getinfo_byTitle(info->title, info) < 0)
+               ret = copy_from_user(&info, argp, sizeof(info));
+               if (ret != 0) {
+                       pr_err("%s:%d,copy_from_user fail\n",
+                               __func__, __LINE__);
+                       return ret;
+               }
+
+               if (efuse_getinfo_byTitle(info.title, &info) < 0)
                        return  -EFAULT;
+
+               ret = copy_to_user(argp, &info, sizeof(info));
+               if (ret != 0) {
+                       pr_err("%s:%d,copy_to_user fail\n",
+                               __func__, __LINE__);
+                       return ret;
+               }
+
                break;
 
        default:
index 1b8cd90..7526452 100644 (file)
@@ -169,15 +169,30 @@ loff_t efuse_llseek(struct file *filp, loff_t off, int whence)
 static long efuse_unlocked_ioctl(struct file *file, unsigned int cmd,
        unsigned long arg)
 {
-       struct efusekey_info *info;
+       void __user          *argp = (void __user *)arg;
+       struct efusekey_info info;
+       int                  ret;
+
 
        switch (cmd) {
        case EFUSE_INFO_GET:
-               info = (struct efusekey_info *)arg;
-               if (efuse_getinfo(info->keyname, info) < 0) {
-                       pr_err("%s if not found\n", info->keyname);
+               ret = copy_from_user(&info, argp, sizeof(info));
+               if (ret != 0) {
+                       pr_err("%s:%d,copy_from_user fail\n",
+                               __func__, __LINE__);
+                       return ret;
+               }
+               if (efuse_getinfo(info.keyname, &info) < 0) {
+                       pr_err("%s if not found\n", info.keyname);
                        return -EFAULT;
                }
+
+               ret = copy_to_user(argp, &info, sizeof(info));
+               if (ret != 0) {
+                       pr_err("%s:%d,copy_to_user fail\n",
+                               __func__, __LINE__);
+                       return ret;
+               }
                break;
 
        default:
index ab0001a..aca9bca 100644 (file)
@@ -170,7 +170,11 @@ int vfm_map_add(char *id, char *name_chain)
                return -ENOMEM;
        }
        memset(p, 0, sizeof(struct vfm_map_s));
-       memcpy(p->id, id, strlen(id));
+       if (strlen(id) >= sizeof(p->id)) {
+               memcpy(p->id, id, sizeof(p->id));
+               p->id[sizeof(p->id)-1] = '\0';
+       } else
+               memcpy(p->id, id, strlen(id));
        p->valid = 1;
        ptr = name_chain;
 
@@ -180,7 +184,13 @@ int vfm_map_add(char *id, char *name_chain)
                        break;
                if (*token == '\0')
                        continue;
-               memcpy(p->name[p->vfm_map_size], token, strlen(token));
+               if (strlen(token) >= sizeof(p->name[p->vfm_map_size])) {
+                       memcpy(p->name[p->vfm_map_size], token,
+                               sizeof(p->name[p->vfm_map_size]));
+                       p->name[p->vfm_map_size][
+                               sizeof(p->name[p->vfm_map_size])-1] = '\0';
+               } else
+                       memcpy(p->name[p->vfm_map_size], token, strlen(token));
                p->vfm_map_size++;
        } while (token && cnt--);
 
index 6f0e8ac..7256a48 100644 (file)
@@ -7884,7 +7884,13 @@ static ssize_t video_filename_store(struct class *cla,
 {
        size_t r;
 
-       r = sscanf(buf, "%s", file_name);
+       /* check input buf to mitigate buffer overflow issue */
+       if (strlen(buf) >= sizeof(file_name)) {
+               memcpy(file_name, buf, sizeof(file_name));
+               file_name[sizeof(file_name)-1] = '\0';
+               r = 1;
+       } else
+               r = sscanf(buf, "%s", file_name);
        if (r != 1)
                return -EINVAL;
 
index 5631349..3ca70ac 100644 (file)
@@ -731,15 +731,23 @@ static long unifykey_unlocked_ioctl(struct file *file,
        unsigned int cmd,
        unsigned long arg)
 {
+       void __user *argp = (void __user *)arg;
+
        switch (cmd) {
        case KEYUNIFY_ATTACH:
                {
-                       struct key_item_t *appitem;
+                       struct key_item_t appitem;
                        char initvalue[KEY_UNIFY_NAME_LEN];
                        int ret;
 
-                       appitem = (struct key_item_t *)arg;
-                       memcpy(initvalue, appitem->name, KEY_UNIFY_NAME_LEN);
+                       ret = copy_from_user(&appitem, argp, sizeof(appitem));
+                       if (ret != 0) {
+                               pr_err("%s:%d,copy_from_user fail\n",
+                                       __func__, __LINE__);
+                               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);
                        if (ret < 0) {
                                pr_err("%s:%d,key unify init fail\n",
@@ -754,13 +762,20 @@ 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;
 
-                       key_item_info = (struct key_item_info_t *)arg;
-                       index = key_item_info->id;
-                       keyname = key_item_info->name;
+                       ret = copy_from_user(&key_item_info,
+                               argp, sizeof(key_item_info));
+                       if (ret != 0) {
+                               pr_err("%s:%d,copy_from_user fail\n",
+                                       __func__, __LINE__);
+                               return ret;
+                       }
+                       //key_item_info = (struct key_item_info_t *)arg;
+                       index = key_item_info.id;
+                       keyname = key_item_info.name;
                        if (strlen(keyname))
                                kkey = unifykey_find_item_by_name(keyname);
                        else
@@ -780,10 +795,10 @@ static long unifykey_unlocked_ioctl(struct file *file,
                                        __func__, __LINE__);
                                return -EFAULT;
                        }
-                       key_item_info->permit = keypermit;
-                       key_item_info->flag = keystate;
-                       key_item_info->id = kkey->id;
-                       strncpy(key_item_info->name,
+                       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));
                        ret = key_unify_size(kkey->name, &reallen);
                        if (ret < 0) {
@@ -792,7 +807,15 @@ static long unifykey_unlocked_ioctl(struct file *file,
                                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));
+                       if (ret != 0) {
+                               pr_err("%s:%d,copy_to_user fail\n",
+                                       __func__, __LINE__);
+                               return ret;
+                       }
 
                        return 0;
                }