[PATCH 2/2] binder: Use receive_fd() to receive file from another process

[Al Viro]

On Fri, Apr 16, 2021 at 05:13:10PM +0200, Christian Brauner wrote:

> My point here was more that the _file_ has already been opened _before_
> that call to io_uring_add_task_file(). But any potential non-trivial
> side-effects of opening that file that you correctly pointed out in an
> earlier mail has already happened by that time.

The file comes from io_uring_get_file(), the entire thing is within the
io_ring_ctx constructor and the only side effect there is ->ring_sock
creation.  And that stays until the io_ring_ctx is freed.  I'm _not_
saying I like io_uring style in general, BTW - in particular,
->ring_sock->file handling is a kludge (as is too much of interation
with AF_UNIX machinery there).  But from side effects POV we are fine
there.

> Granted there are more
> obvious examples, e.g. the binder stuff.
> 
> 		int fd = get_unused_fd_flags(O_CLOEXEC);
> 
> 		if (fd < 0) {
> 			binder_debug(BINDER_DEBUG_TRANSACTION,
> 				     "failed fd fixup txn %d fd %d\n",
> 				     t->debug_id, fd);
> 			ret = -ENOMEM;
> 			break;
> 		}
> 		binder_debug(BINDER_DEBUG_TRANSACTION,
> 			     "fd fixup txn %d fd %d\n",
> 			     t->debug_id, fd);
> 		trace_binder_transaction_fd_recv(t, fd, fixup->offset);
> 		fd_install(fd, fixup->file);
> 		fixup->file = NULL;
> 		if (binder_alloc_copy_to_buffer(&proc->alloc, t->buffer,
> 						fixup->offset, &fd,
> 						sizeof(u32))) {
> 			ret = -EINVAL;
> 			break;
> 		}

... and it's actually broken, since this
        /* All copies must be 32-bit aligned and 32-bit size */
	if (!check_buffer(alloc, buffer, buffer_offset, bytes))
		return -EINVAL;
in binder_alloc_copy_to_buffer() should've been done *before*
fd_install().  If anything, it's an example of the situation when
fd_receive() would be wrong...