Re: [PATCH v7 4/9] coredump: add coredump socket

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Jann Horn <jannh@google.com>
Date: Thu, 15 May 2025 22:54:14 +0200
> > +               /*
> > +                * It is possible that the userspace process which is
> > +                * supposed to handle the coredump and is listening on
> > +                * the AF_UNIX socket coredumps. Userspace should just
> > +                * mark itself non dumpable.
> > +                */
> > +
> > +               retval = sock_create_kern(&init_net, AF_UNIX, SOCK_STREAM, 0, &socket);
> > +               if (retval < 0)
> > +                       goto close_fail;
> > +
> > +               file = sock_alloc_file(socket, 0, NULL);
> > +               if (IS_ERR(file)) {
> > +                       sock_release(socket);
> 
> I think you missed an API gotcha here. See the sock_alloc_file() documentation:
> 
>  * On failure @sock is released, and an ERR pointer is returned.
> 
> So I think basically sock_alloc_file() always consumes the socket
> reference provided by the caller, and the sock_release() in this
> branch is a double-free?

Good catch, yes, sock_release() is not needed here.


> 
> > +                       goto close_fail;
> > +               }
> [...]
> > diff --git a/include/linux/net.h b/include/linux/net.h
> > index 0ff950eecc6b..139c85d0f2ea 100644
> > --- a/include/linux/net.h
> > +++ b/include/linux/net.h
> > @@ -81,6 +81,7 @@ enum sock_type {
> >  #ifndef SOCK_NONBLOCK
> >  #define SOCK_NONBLOCK  O_NONBLOCK
> >  #endif
> > +#define SOCK_COREDUMP  O_NOCTTY
> 
> Hrrrm. I looked through all the paths from which the ->connect() call
> can come, and I think this is currently safe; but I wonder if it would
> make sense to either give this highly privileged bit a separate value
> that can never come from userspace, or explicitly strip it away in
> __sys_connect_file() just to be safe.

I had the same thought, but I think it's fine to leave the code as
is for now.  We can revisit it later once someone reports a strange
regression, which will be most unlikely.