Commit 89194773 authored by Gopi Krishna Menon's avatar Gopi Krishna Menon Committed by Rob Clark
Browse files

drm/msm: Add NULL check in vm_op_enqueue() 



vm_op_enqueue() allocates an msm_vm_op struct with kmalloc,
but the return value is not checked for NULL value which
can be returned by kmalloc under low-memory conditions.
This can result in NULL pointer dereference when the pointer
is dereferenced.

Add NULL check after the allocation and propagate -ENOMEM back
to the caller in case of a failure.

Signed-off-by: default avatarGopi Krishna Menon <krishnagopi487@gmail.com>
Patchwork: https://patchwork.freedesktop.org/patch/678416/


Signed-off-by: default avatarRob Clark <robin.clark@oss.qualcomm.com>
parent e2f085ab

drivers/gpu/drm/msm/msm_gem_vma.c

+22 −6
Original line number Diff line number Diff line
@@ -462,15 +462,20 @@ struct op_arg { 
	bool kept;

};



static void

static int

vm_op_enqueue(struct op_arg *arg, struct msm_vm_op _op)

{

	struct msm_vm_op *op = kmalloc(sizeof(*op), GFP_KERNEL);

	if (!op)

		return -ENOMEM;



	*op = _op;

	list_add_tail(&op->node, &arg->job->vm_ops);



	if (op->obj)

		drm_gem_object_get(op->obj);



	return 0;

}



static struct drm_gpuva *
@@ -489,6 +494,7 @@ msm_gem_vm_sm_step_map(struct drm_gpuva_op *op, void *_arg) 
	struct drm_gpuva *vma;

	struct sg_table *sgt;

	unsigned prot;

	int ret;



	if (arg->kept)

		return 0;
@@ -500,8 +506,6 @@ msm_gem_vm_sm_step_map(struct drm_gpuva_op *op, void *_arg) 
	vm_dbg("%p:%p:%p: %016llx %016llx", vma->vm, vma, vma->gem.obj,

	       vma->va.addr, vma->va.range);



	vma->flags = ((struct op_arg *)arg)->flags;



	if (obj) {

		sgt = to_msm_bo(obj)->sgt;

		prot = msm_gem_prot(obj);
@@ -510,7 +514,7 @@ msm_gem_vm_sm_step_map(struct drm_gpuva_op *op, void *_arg) 
		prot = IOMMU_READ | IOMMU_WRITE;

	}



	vm_op_enqueue(arg, (struct msm_vm_op){

	ret = vm_op_enqueue(arg, (struct msm_vm_op){

		.op = MSM_VM_OP_MAP,

		.map = {

			.sgt = sgt,
@@ -523,6 +527,10 @@ msm_gem_vm_sm_step_map(struct drm_gpuva_op *op, void *_arg) 
		.obj = vma->gem.obj,

	});



	if (ret)

		return ret;



	vma->flags = ((struct op_arg *)arg)->flags;

	to_msm_vma(vma)->mapped = true;



	return 0;
@@ -538,6 +546,7 @@ msm_gem_vm_sm_step_remap(struct drm_gpuva_op *op, void *arg) 
	struct drm_gpuvm_bo *vm_bo = orig_vma->vm_bo;

	bool mapped = to_msm_vma(orig_vma)->mapped;

	unsigned flags;

	int ret;



	vm_dbg("orig_vma: %p:%p:%p: %016llx %016llx", vm, orig_vma,

	       orig_vma->gem.obj, orig_vma->va.addr, orig_vma->va.range);
@@ -547,7 +556,7 @@ msm_gem_vm_sm_step_remap(struct drm_gpuva_op *op, void *arg) 


		drm_gpuva_op_remap_to_unmap_range(&op->remap, &unmap_start, &unmap_range);



		vm_op_enqueue(arg, (struct msm_vm_op){

		ret = vm_op_enqueue(arg, (struct msm_vm_op){

			.op = MSM_VM_OP_UNMAP,

			.unmap = {

				.iova = unmap_start,
@@ -557,6 +566,9 @@ msm_gem_vm_sm_step_remap(struct drm_gpuva_op *op, void *arg) 
			.obj = orig_vma->gem.obj,

		});



		if (ret)

			return ret;



		/*

		 * Part of this GEM obj is still mapped, but we're going to kill the

		 * existing VMA and replace it with one or two new ones (ie. two if
@@ -618,6 +630,7 @@ msm_gem_vm_sm_step_unmap(struct drm_gpuva_op *op, void *_arg) 
	struct msm_vm_bind_job *job = arg->job;

	struct drm_gpuva *vma = op->unmap.va;

	struct msm_gem_vma *msm_vma = to_msm_vma(vma);

	int ret;



	vm_dbg("%p:%p:%p: %016llx %016llx", vma->vm, vma, vma->gem.obj,

	       vma->va.addr, vma->va.range);
@@ -650,7 +663,7 @@ msm_gem_vm_sm_step_unmap(struct drm_gpuva_op *op, void *_arg) 
	if (!msm_vma->mapped)

		goto out_close;



	vm_op_enqueue(arg, (struct msm_vm_op){

	ret = vm_op_enqueue(arg, (struct msm_vm_op){

		.op = MSM_VM_OP_UNMAP,

		.unmap = {

			.iova = vma->va.addr,
@@ -660,6 +673,9 @@ msm_gem_vm_sm_step_unmap(struct drm_gpuva_op *op, void *_arg) 
		.obj = vma->gem.obj,

	});



	if (ret)

		return ret;



	msm_vma->mapped = false;



out_close: