Commit 077f6db9 authored by Todd Poynor's avatar Todd Poynor Committed by Greg Kroah-Hartman
Browse files

staging: ashmem: Avoid deadlock between read and mmap calls 



Avoid holding ashmem_mutex across code that can page fault.  Page faults
grab the mmap_sem for the process, which are also held by mmap calls
prior to calling ashmem_mmap, which locks ashmem_mutex.  The reversed
order of locking between the two can deadlock.

The calls that can page fault are read() and the ASHMEM_SET_NAME and
ASHMEM_GET_NAME ioctls.  Move the code that accesses userspace pages
outside the ashmem_mutex.

Cc: Colin Cross <ccross@android.com>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: default avatarTodd Poynor <toddpoynor@google.com>
[jstultz: minor commit message tweaks]
Signed-off-by: default avatarJohn Stultz <john.stultz@linaro.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 5cf045f5

drivers/staging/android/ashmem.c

+26 −19
Original line number Diff line number Diff line
@@ -295,21 +295,29 @@ static ssize_t ashmem_read(struct file *file, char __user *buf, 


	/* If size is not set, or set to 0, always return EOF. */

	if (asma->size == 0)

		goto out;

		goto out_unlock;



	if (!asma->file) {

		ret = -EBADF;

		goto out;

		goto out_unlock;

	}



	ret = asma->file->f_op->read(asma->file, buf, len, pos);

	if (ret < 0)

		goto out;

	mutex_unlock(&ashmem_mutex);



	/*

	 * asma and asma->file are used outside the lock here.  We assume

	 * once asma->file is set it will never be changed, and will not

	 * be destroyed until all references to the file are dropped and

	 * ashmem_release is called.

	 */

	ret = asma->file->f_op->read(asma->file, buf, len, pos);

	if (ret >= 0) {

		/** Update backing file pos, since f_ops->read() doesn't */

		asma->file->f_pos = *pos;

	}

	return ret;



out:

out_unlock:

	mutex_unlock(&ashmem_mutex);

	return ret;

}
@@ -498,6 +506,7 @@ out: 


static int set_name(struct ashmem_area *asma, void __user *name)

{

	int len;

	int ret = 0;

	char local_name[ASHMEM_NAME_LEN];
@@ -510,21 +519,19 @@ static int set_name(struct ashmem_area *asma, void __user *name) 
	 * variable that does not need protection and later copy the local

	 * variable to the structure member with lock held.

	 */

	if (copy_from_user(local_name, name, ASHMEM_NAME_LEN))

		return -EFAULT;



	len = strncpy_from_user(local_name, name, ASHMEM_NAME_LEN);

	if (len < 0)

		return len;

	if (len == ASHMEM_NAME_LEN)

		local_name[ASHMEM_NAME_LEN - 1] = '\0';

	mutex_lock(&ashmem_mutex);

	/* cannot change an existing mapping's name */

	if (unlikely(asma->file)) {

	if (unlikely(asma->file))

		ret = -EINVAL;

		goto out;

	}

	memcpy(asma->name + ASHMEM_NAME_PREFIX_LEN,

		local_name, ASHMEM_NAME_LEN);

	asma->name[ASHMEM_FULL_NAME_LEN-1] = '\0';

out:

	mutex_unlock(&ashmem_mutex);

	else

		strcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name);



	mutex_unlock(&ashmem_mutex);

	return ret;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE