Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 82fc02db authored by Colin Cross's avatar Colin Cross Committed by Mitchel Humpherys
Browse files

ion: hold reference to handle after ion_uhandle_get 



commit 1262ab1846cf76f7549c66ef709120dbfbe6d49f (ion: replace
userspace handle cookies with idr) broke the locking in ion.
The ION_IOC_FREE and ION_IOC_MAP ioctls were relying on
ion_handle_validate to detect the case where a call raced
with another ION_IOC_FREE which may have freed the struct
ion_handle.

Rename ion_uhandle_get to ion_handle_get_by_id, and have it
take the client lock and return with an extra reference to
the handle.  Make each caller put its reference once it
is done with the handle.

Also modify users of ion_handle_validate to continue to hold
the client lock after calling ion_handle_validate until
they are done with the handle, and warn if ion_handle_validate
is called without the client lock held.

Change-Id: I56da5624fca3bed4ee24806b6ec39de903543341
Signed-off-by: default avatarColin Cross <ccross@android.com>
Git-commit: 33a57aa073dac709bcdcba23bc4e2e7fcc6330f6
Git-repo: https://android.googlesource.com/kernel/common


[mitchelh@codeaurora.org: minor merge conflicts due to some missing
 casts since we haven't brought in the change that makes
 ion_user_handle_t an int.]
Signed-off-by: default avatarMitchel Humpherys <mitchelh@codeaurora.org>
parent cdf68a92

drivers/gpu/ion/ion.c

+41 −16
Original line number Original line Diff line number Diff line
@@ -362,7 +362,14 @@ static void ion_handle_get(struct ion_handle *handle) 




static int ion_handle_put(struct ion_handle *handle)

static int ion_handle_put(struct ion_handle *handle)

{

{

	return kref_put(&handle->ref, ion_handle_destroy);

	struct ion_client *client = handle->client;

	int ret;



	mutex_lock(&client->lock);

	ret = kref_put(&handle->ref, ion_handle_destroy);

	mutex_unlock(&client->lock);



	return ret;

}

}





static struct ion_handle *ion_handle_lookup(struct ion_client *client,

static struct ion_handle *ion_handle_lookup(struct ion_client *client,
@@ -379,14 +386,24 @@ static struct ion_handle *ion_handle_lookup(struct ion_client *client, 
	return ERR_PTR(-EINVAL);

	return ERR_PTR(-EINVAL);

}

}





static struct ion_handle *ion_uhandle_get(struct ion_client *client, int id)

static struct ion_handle *ion_handle_get_by_id(struct ion_client *client,

						int id)

{

{

	return idr_find(&client->idr, id);

	struct ion_handle *handle;



	mutex_lock(&client->lock);

	handle = idr_find(&client->idr, id);

	if (handle)

		ion_handle_get(handle);

	mutex_unlock(&client->lock);



	return handle ? handle : ERR_PTR(-EINVAL);

}

}





static bool ion_handle_validate(struct ion_client *client, struct ion_handle *handle)

static bool ion_handle_validate(struct ion_client *client, struct ion_handle *handle)

{

{

	return (ion_uhandle_get(client, handle->id) == handle);

	WARN_ON(!mutex_is_locked(&client->lock));

	return (idr_find(&client->idr, handle->id) == handle);

}

}





static int ion_handle_add(struct ion_client *client, struct ion_handle *handle)

static int ion_handle_add(struct ion_client *client, struct ion_handle *handle)
@@ -523,11 +540,11 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, 




	mutex_lock(&client->lock);

	mutex_lock(&client->lock);

	ret = ion_handle_add(client, handle);

	ret = ion_handle_add(client, handle);

	mutex_unlock(&client->lock);

	if (ret) {

	if (ret) {

		ion_handle_put(handle);

		ion_handle_put(handle);

		handle = ERR_PTR(ret);

		handle = ERR_PTR(ret);

	}

	}

	mutex_unlock(&client->lock);





	return handle;

	return handle;

}

}
@@ -546,8 +563,8 @@ void ion_free(struct ion_client *client, struct ion_handle *handle) 
		mutex_unlock(&client->lock);

		mutex_unlock(&client->lock);

		return;

		return;

	}

	}

	ion_handle_put(handle);

	mutex_unlock(&client->lock);

	mutex_unlock(&client->lock);

	ion_handle_put(handle);

}

}

EXPORT_SYMBOL(ion_free);

EXPORT_SYMBOL(ion_free);
@@ -1180,14 +1197,15 @@ struct dma_buf *ion_share_dma_buf(struct ion_client *client, 




	mutex_lock(&client->lock);

	mutex_lock(&client->lock);

	valid_handle = ion_handle_validate(client, handle);

	valid_handle = ion_handle_validate(client, handle);

	mutex_unlock(&client->lock);

	if (!valid_handle) {

	if (!valid_handle) {

		WARN(1, "%s: invalid handle passed to share.\n", __func__);

		WARN(1, "%s: invalid handle passed to share.\n", __func__);

		mutex_unlock(&client->lock);

		return ERR_PTR(-EINVAL);

		return ERR_PTR(-EINVAL);

	}

	}



	buffer = handle->buffer;

	buffer = handle->buffer;

	ion_buffer_get(buffer);

	ion_buffer_get(buffer);

	mutex_unlock(&client->lock);



	dmabuf = dma_buf_export(buffer, &dma_buf_ops, buffer->size, O_RDWR);

	dmabuf = dma_buf_export(buffer, &dma_buf_ops, buffer->size, O_RDWR);

	if (IS_ERR(dmabuf)) {

	if (IS_ERR(dmabuf)) {

		ion_buffer_put(buffer);

		ion_buffer_put(buffer);
@@ -1239,18 +1257,24 @@ struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd) 
	handle = ion_handle_lookup(client, buffer);

	handle = ion_handle_lookup(client, buffer);

	if (!IS_ERR(handle)) {

	if (!IS_ERR(handle)) {

		ion_handle_get(handle);

		ion_handle_get(handle);

		mutex_unlock(&client->lock);

		goto end;

		goto end;

	}

	}

	mutex_unlock(&client->lock);



	handle = ion_handle_create(client, buffer);

	handle = ion_handle_create(client, buffer);

	if (IS_ERR(handle))

	if (IS_ERR(handle))

		goto end;

		goto end;



	mutex_lock(&client->lock);

	ret = ion_handle_add(client, handle);

	ret = ion_handle_add(client, handle);

	mutex_unlock(&client->lock);

	if (ret) {

	if (ret) {

		ion_handle_put(handle);

		ion_handle_put(handle);

		handle = ERR_PTR(ret);

		handle = ERR_PTR(ret);

	}

	}



end:

end:

	mutex_unlock(&client->lock);

	dma_buf_put(dmabuf);

	dma_buf_put(dmabuf);

	return handle;

	return handle;

}

}
@@ -1314,12 +1338,11 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) 
		if (copy_from_user(&data, (void __user *)arg,

		if (copy_from_user(&data, (void __user *)arg,

				   sizeof(struct ion_handle_data)))

				   sizeof(struct ion_handle_data)))

			return -EFAULT;

			return -EFAULT;

		mutex_lock(&client->lock);

		handle = ion_handle_get_by_id(client, (int)data.handle);

		handle = ion_uhandle_get(client, (int)data.handle);

		if (IS_ERR(handle))

		mutex_unlock(&client->lock);

			return PTR_ERR(handle);

		if (!handle)

			return -EINVAL;

		ion_free(client, handle);

		ion_free(client, handle);

		ion_handle_put(handle);

		break;

		break;

	}

	}

	case ION_IOC_SHARE:

	case ION_IOC_SHARE:
@@ -1330,9 +1353,11 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) 
		if (copy_from_user(&data, (void __user *)arg, sizeof(data)))

		if (copy_from_user(&data, (void __user *)arg, sizeof(data)))

			return -EFAULT;

			return -EFAULT;





		handle = ion_uhandle_get(client, (int)data.handle);

		handle = ion_handle_get_by_id(client, (int)data.handle);

		if (IS_ERR(handle))

			return PTR_ERR(handle);

		data.fd = ion_share_dma_buf_fd(client, handle);

		data.fd = ion_share_dma_buf_fd(client, handle);



		ion_handle_put(handle);

		if (copy_to_user((void __user *)arg, &data, sizeof(data)))

		if (copy_to_user((void __user *)arg, &data, sizeof(data)))

			return -EFAULT;

			return -EFAULT;

		if (data.fd < 0)

		if (data.fd < 0)