From owner-freebsd-stable@freebsd.org Mon Feb 6 16:19:43 2017 Return-Path: Delivered-To: freebsd-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DC44ECD3E90 for ; Mon, 6 Feb 2017 16:19:43 +0000 (UTC) (envelope-from Mark.Martinec+freebsd@ijs.si) Received: from mail.ijs.si (mail.ijs.si [IPv6:2001:1470:ff80::25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 617C8EAC; Mon, 6 Feb 2017 16:19:43 +0000 (UTC) (envelope-from Mark.Martinec+freebsd@ijs.si) Received: from amavis-ori.ijs.si (localhost [IPv6:::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.ijs.si (Postfix) with ESMTPS id 3vHCNT05VXz1YC; Mon, 6 Feb 2017 17:19:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ijs.si; h= user-agent:message-id:references:in-reply-to:organization :subject:subject:from:from:date:date:content-transfer-encoding :content-type:content-type:mime-version:received:received :received:received; s=jakla4; t=1486397972; x=1488989973; bh=c91 BdZYuNYaYDrDB0REdlqdsnIJTXMFdJgUpv6qYIcM=; b=P37cNiG2zTBCsnkJbBG tr0nlbeo1gfWEoEC7ST0R35WDAvJt9yfRUgbeh2a3r5JshS/xENtqFQfj3ba9jG3 LYHpOpmYVo82Q7EQNrAxFmnmZyoswKpyeSROZd4HHp4i+gD2t7TYF65xfbmkLYWC pAGfTn266zUdAKeJkpS8l3xg= X-Virus-Scanned: amavisd-new at ijs.si Received: from mail.ijs.si ([IPv6:::1]) by amavis-ori.ijs.si (mail.ijs.si [IPv6:::1]) (amavisd-new, port 10026) with LMTP id H-jda3537PAj; Mon, 6 Feb 2017 17:19:32 +0100 (CET) Received: from mildred.ijs.si (mailbox.ijs.si [IPv6:2001:1470:ff80::143:1]) by mail.ijs.si (Postfix) with ESMTP id 3vHCNN24rMz1YB; Mon, 6 Feb 2017 17:19:30 +0100 (CET) Received: from nabiralnik.ijs.si (nabiralnik.ijs.si [IPv6:2001:1470:ff80::80:16]) by mildred.ijs.si (Postfix) with ESMTP id 3vHCNL3sm2z1YG; Mon, 6 Feb 2017 17:19:30 +0100 (CET) Received: from neli.ijs.si (2001:1470:ff80:88:21c:c0ff:feb1:8c91) by webmail.ijs.si with HTTP (HTTP/1.1 POST); Mon, 06 Feb 2017 17:19:30 +0100 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 06 Feb 2017 17:19:30 +0100 From: Mark Martinec To: Eric van Gyzen Cc: freebsd-stable@freebsd.org Subject: Re: net.inet.udp.log_in_vain strange syslog reports Organization: Jozef Stefan Institute In-Reply-To: References: <76681a24b7935674585b5ac585f4575c@ijs.si> Message-ID: <667fa3e1dd8e7cebbf4566467a7987bf@ijs.si> X-Sender: Mark.Martinec+freebsd@ijs.si User-Agent: Roundcube Webmail/1.2.3 X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Feb 2017 16:19:44 -0000 > On 2017-02-02 12:55, Mark Martinec wrote: >> 11.0-RELEASE-p7, net.inet.udp.log_in_vain=1 >> >> The following syslog entries seem to indicate some buffer overruns >> in the reporting code (not all log lines are broken, just some). >> >> (the actual failed connection attempts were indeed there, >> it's just that the reported IP address is highly suspicious) >> >> Mark 2017-02-03 20:05, Eric van Gyzen wrote: > There is no buffer overrun, so no cause for alarm. The problem is > concurrent usage of a single string buffer by multiple threads. The > buffer is inside inet_ntoa(), defined in sys/libkern/inet_ntoa.c. In > this case, it is called from udp_input(). > > Would you like to test the following patch? > > Eric > > > diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c > index 173c44c..ca2dda1 100644 > --- a/sys/netinet/udp_usrreq.c > +++ b/sys/netinet/udp_usrreq.c > @@ -674,13 +674,13 @@ udp_input(struct mbuf **mp, int *offp, int proto) > INPLOOKUP_RLOCKPCB, ifp, m); > if (inp == NULL) { > if (udp_log_in_vain) { > - char buf[4*sizeof "123"]; > + char src[4*sizeof "123"]; > + char dst[4*sizeof "123"]; > > - strcpy(buf, inet_ntoa(ip->ip_dst)); > log(LOG_INFO, > "Connection attempt to UDP %s:%d from > %s:%d\n", > - buf, ntohs(uh->uh_dport), > inet_ntoa(ip->ip_src), > - ntohs(uh->uh_sport)); > + inet_ntoa_r(ip->ip_dst, dst), > ntohs(uh->uh_dport), > + inet_ntoa_r(ip->ip_src, src), > ntohs(uh->uh_sport)); > } > UDPSTAT_INC(udps_noport); > if (m->m_flags & (M_BCAST | M_MCAST)) { Thanks, the explanation makes sense and the patch looks good (mind the TABs). Running it now, expecting no surprises there. One minor nit: instead of a hack: char src[4*sizeof "123"]; char dst[4*sizeof "123"]; it would be cleaner and in sync with the equivalent code in sys/netinet6/udp6_usrreq.c to use the INET_ADDRSTRLEN constant (from sys/netinet/in.h, value 16): char src[INET_ADDRSTRLEN]; char dst[INET_ADDRSTRLEN]; Hope the fix finds its way into 11.1 (or better yet, as a patch level in 10.0). Should I open a bug report? Mark