On 2/14/21, Al Viro [off-list ref] wrote:

On Fri, Jan 29, 2021 at 01:18:55PM +0000, Christoph Hellwig wrote:

quoted

On Fri, Jan 29, 2021 at 04:11:05PM +0300, Denis Kirjanov wrote:

quoted

Do you mean just:

We'll still need to lock the parent inode.

Not just "lock", we wouldd need to have the lock _held_ across the
entire sequence.  Without that there's no warranty that it will refer
to the same object we'd created.

In any case, unlink in any potentially public area is pretty much
never the right approach.  Once mknod has happened, that's it - too
late to bail out.

IIRC, most of the PITA in that area is due to unix_autobind()
iteractions.  Basically, we try to bind() an unbound socket and
another thread does sendmsg() on the same while we are in the
middle of ->mknod().  Who should wait for whom?

->mknod() really should be a point of no return - any games with
"so we unlink it" are unreliable in the best case, and that's
only if we do _not_ unlock the parent through the entire sequence.

Seeing that we have separate bindlock and iolock now...  How about
this (completely untested) delta?

We had a change like that:
Author: WANG Cong [off-list ref]
Date:   Mon Jan 23 11:17:35 2017 -0800

    af_unix: move unix_mknod() out of bindlock

    Dmitry reported a deadlock scenario:

    unix_bind() path:
    u->bindlock ==> sb_writer

    do_splice() path:
    sb_writer ==> pipe->mutex ==> u->bindlock

    In the unix_bind() code path, unix_mknod() does not have to
    be done with u->bindlock held, since it is a pure fs operation,
    so we can just move unix_mknod() out.