[PATCH 7/7] drm/mode: Add user blob-creation ioctl

[Maarten Lankhorst]

Op 20-04-15 om 20:22 schreef Daniel Stone:
> Add an ioctl which allows users to create blob properties from supplied
> data. Currently this only supports modes, creating a drm_display_mode from
> the userspace drm_mode_modeinfo.
>
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
> <snip>
> +int drm_mode_createblob_ioctl(struct drm_device *dev,
> +			      void *data, struct drm_file *file_priv)
> +{
> +	struct drm_mode_create_blob *out_resp = data;
> +	struct drm_property_blob *blob;
> +	void *blob_data;
> +	int ret = 0;
> +	void __user *blob_ptr;
> +
> +	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +		return -EINVAL;
> +
> +	switch (out_resp->type) {
> +	case DRM_MODE_BLOB_TYPE_MODE: {
> +		if (out_resp->length != sizeof(struct drm_mode_modeinfo)) {
> +			ret = -E2BIG;to length
You want -EINVAL here, -E2BIG means "argument list too long".
> +			goto out_unlocked;
> +		}
> +		break;
> +	}
> +	default:
> +		ret = -EINVAL;
> +		goto out_unlocked;
> +	}
> +
> +	blob_data = kzalloc(out_resp->length, GFP_USER);
> +	if (!blob_data) {
> +		ret = -ENOMEM;
> +		goto out_unlocked;
> +	}
> +
> +	blob_ptr = (void __user *)(unsigned long)out_resp->data;
> +	if (copy_from_user(blob_data, blob_ptr, out_resp->length)) {
> +		ret = -EFAULT;
> +		goto out_data;
> +	}
> +
> +	blob = drm_property_create_blob(dev, out_resp->length, blob_data);
> +	if (!blob) {
> +		ret = -EINVAL;
drm_property_create_blob can fail from -ENOMEM or -EINVAL here, so correctly return the error from drm_property_create_blob?

You're also doing a double allocation, one for blob_ptr and the other for blob. It might be better to do a single allocation of the blob and copy
the data to blob->data directly.

For the whole series if this patch is fixed up:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>