Date: Fri, 14 Apr 2017 23:58:55 -0700 From: Peter Wemm <peter@wemm.org> To: "Ngie Cooper (yaneurabeya)" <yaneurabeya@gmail.com> Cc: svn-src-all@freebsd.org, src-committers <src-committers@freebsd.org>, Hiroki Sato <hrs@freebsd.org>, Hiren Panchasara <hiren@freebsd.org>, svn-src-head@freebsd.org, Maxim Sobolev <sobomax@sippysoft.com> Subject: Re: svn commit: r316874 - head/sys/kern Message-ID: <11221477.gRA6BXfcIE@overcee.wemm.org> In-Reply-To: <CDFAE7DD-8F3A-43D9-AAD5-DF9FABEC4669@gmail.com> References: <201704141723.v3EHNS3B043902@repo.freebsd.org> <2109294.KVn8xeR1jZ@overcee.wemm.org> <CDFAE7DD-8F3A-43D9-AAD5-DF9FABEC4669@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Friday, April 14, 2017 08:13:52 PM Ngie Cooper wrote:
> > On Apr 14, 2017, at 20:12, Peter Wemm <peter@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@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@.
> >>
> >> 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@wemm.org; peter@FreeBSD.org; peter@yahoo-inc.com; KI6FJV
UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246
[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
iQEzBAABCAAdFiEEBgrA0Vr/vfNVuPoUNdaXCeyAngQFAljxxK8ACgkQNdaXCeyA
ngTWzAgAwTxqh8URPRWoP4xssbIcrHxQp24Vo3b2ybHpVmG+4VNudT2X2gb5tRso
CpCy4rMNZd8OY+rIcC8lzIexFgmvnwGEfZQgBjXiDK1cup5ylg6aR/CPziJ38jk/
uR56sh3hPiIONPHht5TlRrXpZAMN3NrX08GqTP1gXC5uoB0nCViffBkew+54bK+h
OduYGUkqkiIV2aHzVKDX/sv8dnqS0SOJKZIf4YRyDp5qyg3Wu2yAj7427+vI2pPc
d1R4A5BHajEPLcxssFRWD/SQug0hUG7cZwyX7d8qoD3LqZEFecF/Oe3sdJAFFiQZ
ZGxmddHKfLwJhhtvOwIbYn+4IFvc9g==
=XJBZ
-----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?11221477.gRA6BXfcIE>