github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-06-09 23:23:53 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

ashmem: avoid deadlock between read and mmap calls

Browse Source 
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.

Bug: 9261835
Change-Id: If1322e981d29c889a56cdc9dfcbc6df2729a45e9
Signed-off-by: Todd Poynor <toddpoynor@google.com>
This commit is contained in:
Todd Poynor 2013-06-04 17:29:38 -07:00
parent 82fc90bbc2
commit c1d43119ec
1 changed files with 33 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
drivers/staging/android/ashmem.c
View File

@ -221,21 +221,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;
}
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)
goto out;
if (ret >= 0) {
/** Update backing file pos, since f_ops->read() doesn't */
asma->file->f_pos = *pos;
}
return ret;
/** Update backing file pos, since f_ops->read() doesn't */
asma->file->f_pos = *pos;
out:
out_unlock:
mutex_unlock(&ashmem_mutex);
return ret;
}
@ -402,50 +410,48 @@ static int set_prot_mask(struct ashmem_area *asma, unsigned long prot)
static int set_name(struct ashmem_area *asma, void __user *name)
{
char lname[ASHMEM_NAME_LEN];
int len;
int ret = 0;
len = strncpy_from_user(lname, name, ASHMEM_NAME_LEN);
if (len < 0)
return len;
if (len == ASHMEM_NAME_LEN)
lname[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;
}
else
strcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, lname);
if (unlikely(copy_from_user(asma->name + ASHMEM_NAME_PREFIX_LEN,
name, ASHMEM_NAME_LEN)))
ret = -EFAULT;
asma->name[ASHMEM_FULL_NAME_LEN-1] = '\0';
out:
mutex_unlock(&ashmem_mutex);
return ret;
}
static int get_name(struct ashmem_area *asma, void __user *name)
{
int ret = 0;
char lname[ASHMEM_NAME_LEN];
size_t len;
mutex_lock(&ashmem_mutex);
if (asma->name[ASHMEM_NAME_PREFIX_LEN] != '\0') {
size_t len;
/*
* Copying only `len', instead of ASHMEM_NAME_LEN, bytes
* prevents us from revealing one user's stack to another.
*/
len = strlen(asma->name + ASHMEM_NAME_PREFIX_LEN) + 1;
if (unlikely(copy_to_user(name,
asma->name + ASHMEM_NAME_PREFIX_LEN, len)))
ret = -EFAULT;
memcpy(lname, asma->name + ASHMEM_NAME_PREFIX_LEN, len);
} else {
if (unlikely(copy_to_user(name, ASHMEM_NAME_DEF,
sizeof(ASHMEM_NAME_DEF))))
ret = -EFAULT;
len = strlen(ASHMEM_NAME_DEF) + 1;
memcpy(lname, ASHMEM_NAME_DEF, len);
}
mutex_unlock(&ashmem_mutex);
if (unlikely(copy_to_user(name, lname, len)))
ret = -EFAULT;
return ret;
}