Re: [PATCH 3/3] io_uring: refactor io_sq_offload_create()

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, Jul 23, 2021 at 07:00:40PM +0000, Al Viro wrote:
> On Fri, Jul 23, 2021 at 11:56:29AM -0600, Jens Axboe wrote:
> 
> > Will send out two patches for this. Note that I don't see this being a
> > real issue, as we explicitly gave the ring fd to another task, and being
> > that this is purely for read/write, it would result in -EFAULT anyway.
> 
> You do realize that ->release() might come from seriously unexpected places,
> right?  E.g. recvmsg() by something that doesn't expect SCM_RIGHTS attached
> to it will end up with all struct file references stashed into the sucker
> dropped, and if by that time that's the last reference - welcome to ->release()
> run as soon as recepient hits task_work_run().
> 
> What's more, if you stash that into garbage for unix_gc() to pick, *any*
> process closing an AF_UNIX socket might end up running your ->release().
> 
> So you really do *not* want to spawn any threads there, let alone
> possibly exfiltrating memory contents of happy recepient of your present...

To elaborate: ->release() instance may not assume anything about current->mm,
or assume anything about current, for that matter.  It is entirely possible
to arrange its execution in context of a process that is not yours and had not
consent to doing that.  In particular, it's a hard bug to have _any_ visible
effects depending upon the memory mappings, memory contents or the contents of
descriptor table of the process in question.

There's really no way around that.