svn commit: r316874 - head/sys/kern

Maxim Sobolev sobomax at freebsd.org
Sat Apr 15 11:00:29 UTC 2017


Peter, Ngie, none of this stuff is really directly related to the
shutdown(2) change, so I'll probably let Hiroki to clean it up.

-Max

On Fri, Apr 14, 2017 at 11:58 PM, Peter Wemm <peter at wemm.org> wrote:

> On Friday, April 14, 2017 08:13:52 PM Ngie Cooper wrote:
> > > On Apr 14, 2017, at 20:12, Peter Wemm <peter at wemm.org> wrote:
> > >
> > > On Friday, April 14, 2017 07:36:55 PM Peter Wemm wrote:
> > >> On Friday, April 14, 2017 02:14:16 PM Ngie Cooper wrote:
> > >>>> On Apr 14, 2017, at 14:10, Maxim Sobolev <sobomax at sippysoft.com>
> wrote:
> > >>>>
> > >>>> Peter, Ngie,
> > >>>>
> > >>>> Looks like out of that refactoring came a logical bug that is
> present
> > >>>> in
> > >>>> the head, which causes syslod to first to shutdown the socket for
> > >>>> reading
> > >>>> and then try to select/recv on it (which is somewhat stupid). And
> that
> > >>>> issue has been masked by shutdown() on datagram socket becoming
> > >>>> effectively a NOP in 11 & head 20 months ago. It only affects head
> > >>>> though, 11-stable still has the old code which does not include that
> > >>>> half-closed socket into the select list. Attached patch is expected
> to
> > >>>> fix head, Peter, it would be nice if you can give it a try
> (restoring
> > >>>> latest changes into uipc_sockets.c) and let me know if it helps.
> > >>>>
> > >>>> Thanks!
> > >>>
> > >>> CCing hrs@ for input as he did the refactoring.
> > >>> Thanks!
> > >>> -Ngie
> > >>>
> > >>> PS LGTM with the change. Will wait for feedback from wemm at .
> > >>
> > >> This is definitely not working.  I get ENOSPC  and listen queue
> overflows
> > >> on /var/run/logpriv now.
> > >>
> > >> Grabbing an old 10.3 /usr/sbin/syslogd and placing it on the top of
> the
> > >> 12.x one worked fine, aside from the include statements.
> > >
> > > This can't be right:
> > >                 if (SecureMode || res->ai_family == AF_LOCAL) {
> > >
> > >                        /* Forbid communication in secure mode. */
> > >                        if (shutdown(s, SHUT_RD) < 0 &&
> > >
> > >                            errno != ENOTCONN) {
> > >
> > >                                logerror("shutdown");
> > >                                if (!Debug)
> > >
> > >                                        die(0);
> > >
> > >                        }
> > >                        dprintf("listening on socket\n");
> > >                        sl_recv = NULL;
> > >
> > >                   }
> > >
> > > This appears to disable unix domain sockets like /var/run/log and
> > > /var/run/logpriv.
> >
> > ACK. This looks like a fun bug.
>
> > -Ngie
>
> I suspect it's meant to be "if (SecureMode && res->ai_family != AF_LOCAL)
> {"
> as a simple logic inversion error of another line earlier.  However
> there's an
> awful lot of strange things in this code.
>
> 1) listen(s, 5) - on datagram sockets.
> 2) dprintf("shutdown") in code regardless of whether the shutdown is going
> to
> happen.
> 3) dprintf("listening on socket") in code that only happens when we're NOT
> going to listen.
> 4) dprintf("sending on socket") in the code path when we're going to
> listen.
> 5) shutdown on all unix domain sockets, regardless of securemode..
>
> This code block makes my head spin.
>
> --
> Peter Wemm - peter at wemm.org; peter at FreeBSD.org; peter at yahoo-inc.com;
> KI6FJV
> UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246
>


More information about the svn-src-head mailing list