From owner-svn-src-all@FreeBSD.ORG Thu May 8 20:23:13 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id B479B147; Thu, 8 May 2014 20:23:13 +0000 (UTC) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.69.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "cell.glebius.int.ru", Issuer "cell.glebius.int.ru" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 21C88F51; Thu, 8 May 2014 20:23:12 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.8/8.14.8) with ESMTP id s48KNA7j050615 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 9 May 2014 00:23:10 +0400 (MSK) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.8/8.14.8/Submit) id s48KNAt4050614; Fri, 9 May 2014 00:23:10 +0400 (MSK) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Fri, 9 May 2014 00:23:10 +0400 From: Gleb Smirnoff To: Michael Tuexen Subject: Re: svn commit: r265691 - head/sys/netinet Message-ID: <20140508202310.GC50446@FreeBSD.org> References: <201405081727.s48HRkiT056077@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201405081727.s48HRkiT056077@svn.freebsd.org> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 May 2014 20:23:13 -0000 On Thu, May 08, 2014 at 05:27:46PM +0000, Michael Tuexen wrote: M> Author: tuexen M> Date: Thu May 8 17:27:46 2014 M> New Revision: 265691 M> URL: http://svnweb.freebsd.org/changeset/base/265691 M> M> Log: M> For some UDP packets (for example with 200 byte payload) and IP options, M> the IP header and the UDP header are not in the same mbuf. M> Add code to in_delayed_cksum() to deal with this case. M> M> MFC after: 3 days M> M> Modified: M> head/sys/netinet/ip_output.c M> M> Modified: head/sys/netinet/ip_output.c M> ============================================================================== M> --- head/sys/netinet/ip_output.c Thu May 8 17:20:45 2014 (r265690) M> +++ head/sys/netinet/ip_output.c Thu May 8 17:27:46 2014 (r265691) M> @@ -887,15 +887,23 @@ in_delayed_cksum(struct mbuf *m) M> csum = 0xffff; M> offset += m->m_pkthdr.csum_data; /* checksum offset */ M> M> + /* find the mbuf in the chain where the checksum starts*/ M> + while ((m != NULL) && (offset >= m->m_len)) { M> + offset -= m->m_len; M> + m = m->m_next; M> + } M> + if (m == NULL) { M> + /* This should not happen. */ M> + printf("in_delayed_cksum(): checksum outside mbuf chain.\n"); M> + return; M> + } M> if (offset + sizeof(u_short) > m->m_len) { M> - printf("delayed m_pullup, m->len: %d off: %d p: %d\n", M> - m->m_len, offset, ip->ip_p); M> /* M> * XXX M> - * this shouldn't happen, but if it does, the M> - * correct behavior may be to insert the checksum M> - * in the appropriate next mbuf in the chain. M> + * This should not happen, but if it does, it might make more M> + * sense to fix the caller than to add code to split it here. M> */ M> + printf("in_delayed_cksum(): checksum split between mbufs.\n"); M> return; M> } M> *(u_short *)(m->m_data + offset) = csum; If "this should not happen" really means that we do not expect this issue at all, assuming we are coding correctly, then all these comments and printfs should be converted to KASSERTs. If "this should not happen" means 'such packets shouldn't arrrive from network", then we need to remove printfs, because having a remotely triggerable printf(9) is a DDoS vulnerability. -- Totus tuus, Glebius.