Re: [PATCH 2/2] ptrace: add ability to attach a file descriptor to another task

[Oleg Nesterov <oleg@redhat.com>]

On 12/16, Andrew Vagin wrote:
>
> Signed-off-by: Andrew Vagin <avagin@openvz.org>

Nice changelog ;)

I agree with Tejun, this doesn't look like the good idea to me...



As for the patch itself,

> @@ -826,6 +827,32 @@ int ptrace_request(struct task_struct *child, long request,
>  		break;
>  	}
>  #endif
> +	case PTRACE_DUPFD:
> +	{
> +		struct file *file = fget_raw(data);
> +		ret = -EBADF;
> +
> +		if (!file)
> +			break;
> +
> +		task_lock(child);
> +		if (!child->files)
> +			goto out_getfd;
> +
> +		ret = alloc_task_fd(child, 0, 0);

alloc_fdtable() does kmalloc(GFP_KERNEL)/vmalloc(), not good under
spin_lock().

> +		if (ret < 0) {
> +			fput(file);

We can't do this under task_lock() too.

A tracer's sub-thread (or any CLONE_FILES task) can close this file,
it is possible that we have the last reference.


Probably, instead of the new *_task_* helpers, you can add
alloc_file_fd/fd_files_install which take the result of
get_files_struct() as the additional argument. This way you can
avoid task_lock().

Oleg.