ANDROID: video: adf: Avoid directly referencing user pointers
authorJonathan Hamilton <jonathan.hamilton@imgtec.com>
Wed, 21 Sep 2016 19:40:51 +0000 (12:40 -0700)
committerGreg Hackmann <ghackmann@google.com>
Wed, 9 Nov 2016 00:30:37 +0000 (00:30 +0000)
Enabling KASAN on a kernel using ADF causes a number of places where
user-supplied pointers to ioctls  pointers are directly dereferenced
without copy_from_user or access_ok.

Bug: 31806036

Signed-off-by: Jonathan Hamilton <jonathan.hamilton@imgtec.com>
Change-Id: I6e86237aaa6cec0f6e1c385336aefcc5332080ae

drivers/video/adf/adf_fops.c

index 8726617f73ab5cccc63418f17cb51e048ecc33c0..705411bfaebba04868d3ea864f4725ac63298f73 100644 (file)
@@ -132,7 +132,7 @@ static int adf_eng_get_data(struct adf_overlay_engine *eng,
                                        eng->ops->n_supported_formats));
 
        mutex_lock(&dev->client_lock);
-       ret = adf_obj_copy_custom_data_to_user(&eng->base, arg->custom_data,
+       ret = adf_obj_copy_custom_data_to_user(&eng->base, data.custom_data,
                        &data.custom_data_size);
        mutex_unlock(&dev->client_lock);
 
@@ -144,7 +144,7 @@ static int adf_eng_get_data(struct adf_overlay_engine *eng,
                goto done;
        }
 
-       if (supported_formats && copy_to_user(arg->supported_formats,
+       if (supported_formats && copy_to_user(data.supported_formats,
                        supported_formats,
                        n_supported_formats * sizeof(supported_formats[0])))
                ret = -EFAULT;
@@ -220,56 +220,45 @@ static int adf_device_post_config(struct adf_device *dev,
        int complete_fence_fd;
        struct adf_buffer *bufs = NULL;
        struct adf_interface **intfs = NULL;
-       size_t n_intfs, n_bufs, i;
+       struct adf_post_config data;
+       size_t i;
        void *custom_data = NULL;
-       size_t custom_data_size;
        int ret = 0;
 
+       if (copy_from_user(&data, arg, sizeof(data)))
+               return -EFAULT;
+
        complete_fence_fd = get_unused_fd_flags(O_CLOEXEC);
        if (complete_fence_fd < 0)
                return complete_fence_fd;
 
-       if (get_user(n_intfs, &arg->n_interfaces)) {
-               ret = -EFAULT;
-               goto err_get_user;
-       }
-
-       if (n_intfs > ADF_MAX_INTERFACES) {
+       if (data.n_interfaces > ADF_MAX_INTERFACES) {
                ret = -EINVAL;
                goto err_get_user;
        }
 
-       if (get_user(n_bufs, &arg->n_bufs)) {
-               ret = -EFAULT;
-               goto err_get_user;
-       }
-
-       if (n_bufs > ADF_MAX_BUFFERS) {
+       if (data.n_bufs > ADF_MAX_BUFFERS) {
                ret = -EINVAL;
                goto err_get_user;
        }
 
-       if (get_user(custom_data_size, &arg->custom_data_size)) {
-               ret = -EFAULT;
-               goto err_get_user;
-       }
-
-       if (custom_data_size > ADF_MAX_CUSTOM_DATA_SIZE) {
+       if (data.custom_data_size > ADF_MAX_CUSTOM_DATA_SIZE) {
                ret = -EINVAL;
                goto err_get_user;
        }
 
-       if (n_intfs) {
-               intfs = kmalloc(sizeof(intfs[0]) * n_intfs, GFP_KERNEL);
+       if (data.n_interfaces) {
+               intfs = kmalloc(sizeof(intfs[0]) * data.n_interfaces,
+                       GFP_KERNEL);
                if (!intfs) {
                        ret = -ENOMEM;
                        goto err_get_user;
                }
        }
 
-       for (i = 0; i < n_intfs; i++) {
+       for (i = 0; i < data.n_interfaces; i++) {
                u32 intf_id;
-               if (get_user(intf_id, &arg->interfaces[i])) {
+               if (get_user(intf_id, &data.interfaces[i])) {
                        ret = -EFAULT;
                        goto err_get_user;
                }
@@ -281,31 +270,31 @@ static int adf_device_post_config(struct adf_device *dev,
                }
        }
 
-       if (n_bufs) {
-               bufs = kzalloc(sizeof(bufs[0]) * n_bufs, GFP_KERNEL);
+       if (data.n_bufs) {
+               bufs = kzalloc(sizeof(bufs[0]) * data.n_bufs, GFP_KERNEL);
                if (!bufs) {
                        ret = -ENOMEM;
                        goto err_get_user;
                }
        }
 
-       for (i = 0; i < n_bufs; i++) {
-               ret = adf_buffer_import(dev, &arg->bufs[i], &bufs[i]);
+       for (i = 0; i < data.n_bufs; i++) {
+               ret = adf_buffer_import(dev, &data.bufs[i], &bufs[i]);
                if (ret < 0) {
                        memset(&bufs[i], 0, sizeof(bufs[i]));
                        goto err_import;
                }
        }
 
-       if (custom_data_size) {
-               custom_data = kzalloc(custom_data_size, GFP_KERNEL);
+       if (data.custom_data_size) {
+               custom_data = kzalloc(data.custom_data_size, GFP_KERNEL);
                if (!custom_data) {
                        ret = -ENOMEM;
                        goto err_import;
                }
 
-               if (copy_from_user(custom_data, arg->custom_data,
-                               custom_data_size)) {
+               if (copy_from_user(custom_data, data.custom_data,
+                               data.custom_data_size)) {
                        ret = -EFAULT;
                        goto err_import;
                }
@@ -316,8 +305,8 @@ static int adf_device_post_config(struct adf_device *dev,
                goto err_import;
        }
 
-       complete_fence = adf_device_post_nocopy(dev, intfs, n_intfs, bufs,
-                       n_bufs, custom_data, custom_data_size);
+       complete_fence = adf_device_post_nocopy(dev, intfs, data.n_interfaces,
+                       bufs, data.n_bufs, custom_data, data.custom_data_size);
        if (IS_ERR(complete_fence)) {
                ret = PTR_ERR(complete_fence);
                goto err_import;
@@ -327,7 +316,7 @@ static int adf_device_post_config(struct adf_device *dev,
        return 0;
 
 err_import:
-       for (i = 0; i < n_bufs; i++)
+       for (i = 0; i < data.n_bufs; i++)
                adf_buffer_cleanup(&bufs[i]);
 
 err_get_user:
@@ -481,19 +470,19 @@ static int adf_device_get_data(struct adf_device *dev,
                        data.n_allowed_attachments);
 
        mutex_lock(&dev->client_lock);
-       ret = adf_obj_copy_custom_data_to_user(&dev->base, arg->custom_data,
+       ret = adf_obj_copy_custom_data_to_user(&dev->base, data.custom_data,
                        &data.custom_data_size);
        mutex_unlock(&dev->client_lock);
 
        if (ret < 0)
                goto done;
 
-       ret = adf_copy_attachment_list_to_user(arg->attachments,
+       ret = adf_copy_attachment_list_to_user(data.attachments,
                        data.n_attachments, attach, n_attach);
        if (ret < 0)
                goto done;
 
-       ret = adf_copy_attachment_list_to_user(arg->allowed_attachments,
+       ret = adf_copy_attachment_list_to_user(data.allowed_attachments,
                        data.n_allowed_attachments, allowed_attach,
                        n_allowed_attach);
        if (ret < 0)
@@ -592,7 +581,7 @@ static int adf_intf_get_data(struct adf_interface *intf,
        data.n_available_modes = intf->n_modes;
        read_unlock_irqrestore(&intf->hotplug_modelist_lock, flags);
 
-       if (copy_to_user(arg->available_modes, modelist, modelist_size)) {
+       if (copy_to_user(data.available_modes, modelist, modelist_size)) {
                ret = -EFAULT;
                goto done;
        }
@@ -601,7 +590,7 @@ static int adf_intf_get_data(struct adf_interface *intf,
        memcpy(&data.current_mode, &intf->current_mode,
                        sizeof(intf->current_mode));
 
-       ret = adf_obj_copy_custom_data_to_user(&intf->base, arg->custom_data,
+       ret = adf_obj_copy_custom_data_to_user(&intf->base, data.custom_data,
                        &data.custom_data_size);
 done:
        mutex_unlock(&dev->client_lock);