On Wed, Feb 20, 2013 at 07:57:08PM +0530, Shankar Brahadeeswaran wrote:
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -413,50 +413,66 @@ out:
>
> static int set_name(struct ashmem_area *asma, void __user *name)
> {
> - int ret = 0;
> -
> - mutex_lock(&ashmem_mutex);
> + char local_name[ASHMEM_NAME_LEN];
>
> /* cannot change an existing mapping's name */
> - if (unlikely(asma->file)) {
> - ret = -EINVAL;
> - goto out;
> - }
> + if (unlikely(asma->file))
> + return -EINVAL;
Gar. No. Sorry I wasn't paying attention properly last time. This
isn't right but I didn't explain things properly from the beginning.
When you drop a lock, obviously the first thing I'm going to look at
is if it introduces race conditions. The problem is that checking
asma->file has to be done under lock and also we can't drop the lock
before we set the name. Otherwise someone could set asma->file
while we were waiting for the copy to complete.
It should be something like this:
char local_name[ASHMEM_NAME_LEN];
int ret = 0;
if (copy_from_user(local_name, name, ASHMEM_NAME_LEN))
return -EFAULT;
local_name[ASHMEM_FULL_NAME_LEN - 1] = '\0';
mutex_lock(&ashmem_mutex);
if (asma->file) {
ret = -EINVAL;
goto out;
}
memcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name,
ASHMEM_NAME_LEN);
out:
mutex_unlock(&ashmem_mutex);
return ret;
(I removed some calls to likely/unlikely() because putting those
around copy_from_user() is probably not going to speed anything up.)
Sorry, again for the miscommunication.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
