Hi Cyrill,
I think the patch is fine but I can't understand the usage of mmap_sem
and alloc_lock,
> + stack_vma = find_vma(mm, (unsigned long)prctl_map->start_stack);
OK, find_vma() needs mmap_sem. But otherwise, why this should be called
under down_read(&mm->mmap_sem) ? What this lock tries to protect?
> + if (prctl_map.auxv_size) {
> + up_read(&mm->mmap_sem);
> + memset(user_auxv, 0, sizeof(user_auxv));
> + error = copy_from_user(user_auxv,
> + (const void __user *)prctl_map.auxv,
> + prctl_map.auxv_size);
> + down_read(&mm->mmap_sem);
And if we actually need this lock, why it is safe to drop it temporary?
And why we can't move this copy_from_user() up before down_read) in any
case?
> + if (prctl_map.auxv_size) {
> + /* Last entry must be AT_NULL as specification requires */
> + user_auxv[AT_VECTOR_SIZE - 2] = AT_NULL;
> + user_auxv[AT_VECTOR_SIZE - 1] = AT_NULL;
> +
> + task_lock(current);
> + memcpy(mm->saved_auxv, user_auxv, sizeof(user_auxv));
> + task_unlock(current);
Again, could you explain this task_lock() ?
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
