Date: Tue, 13 Dec 2016 10:14:53 -0800 From: Conrad Meyer <cem@freebsd.org> To: Hiroki Sato <hrs@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r309933 - head/usr.sbin/syslogd Message-ID: <CAG6CVpV=nshki9k8DM-4Cni1gTJ8U-Eh1t8D9KfUqo0MKnC=wA@mail.gmail.com> In-Reply-To: <201612121933.uBCJXen2093959@repo.freebsd.org> References: <201612121933.uBCJXen2093959@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Dec 12, 2016 at 11:33 AM, Hiroki Sato <hrs@freebsd.org> wrote: > Author: hrs > Date: Mon Dec 12 19:33:40 2016 > New Revision: 309933 > URL: https://svnweb.freebsd.org/changeset/base/309933 > > Log: > - Refactor listening socket list. All of the listening sockets are > now maintained in a single linked-list in a transport-independent manner. > - Use queue.h for linked-list structure. > - Use linked-list for AllowedPeers. > - Use getaddrinfo(8) even for Unix Domain sockets. > - Use macros to type-casting from/to struct sockaddr{,_in,_in6}. > - Define fu_* macro for union f_un to shorten the member names. > - Remove an extra #include <sys/type.h>. > - Add "static" to non-exported symbols. > - !INET support is still incomplete but will be fixed later. > > There is no functional change except for some minor debug messages. Hello Hiroki, This refactor introduced a bug in the IPv6 address comparison/rejection logic. > Modified: head/usr.sbin/syslogd/syslogd.c > ============================================================================== > --- head/usr.sbin/syslogd/syslogd.c Mon Dec 12 19:26:55 2016 (r309932) > +++ head/usr.sbin/syslogd/syslogd.c Mon Dec 12 19:33:40 2016 (r309933) > ... > reject = 0; > - for (j = 0; j < 16; j += 4) { > - if ((*(u_int32_t *)&sin6->sin6_addr.s6_addr[j] & *(u_int32_t *)&m6p->sin6_addr.s6_addr[j]) > - != *(u_int32_t *)&a6p->sin6_addr.s6_addr[j]) { > - ++reject; > - break; > - } > + if (IN6_ARE_MASKED_ADDR_EQUAL(&sin6->sin6_addr, > + &a6p->sin6_addr, &m6p->sin6_addr) != 0) { > + ++reject; > + break; > } > if (reject) { > dprintf("rejected in rule %d due to IP mismatch.\n", i); The new check isn't a loop, so the 'break' breaks out of the outer loop, which is unintentional. I think we should just remove 'break'. This was found by Coverity CID 1366941. Best, Conrad
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpV=nshki9k8DM-4Cni1gTJ8U-Eh1t8D9KfUqo0MKnC=wA>