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


_______________________________________________
freebsd-stable@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Reply via email to