From owner-freebsd-net Wed Oct 11 10:42:14 2000 Delivered-To: freebsd-net@freebsd.org Received: from whistle.com (s205m131.whistle.com [207.76.205.131]) by hub.freebsd.org (Postfix) with ESMTP id 3E5BA37B503; Wed, 11 Oct 2000 10:42:11 -0700 (PDT) Received: (from smap@localhost) by whistle.com (8.10.0/8.10.0) id e9BHgAM25207; Wed, 11 Oct 2000 10:42:10 -0700 (PDT) Received: from bubba.whistle.com( 207.76.205.7) by whistle.com via smap (V2.0) id xma025202; Wed, 11 Oct 2000 10:41:49 -0700 Received: (from archie@localhost) by bubba.whistle.com (8.11.0/8.11.0) id e9BHfnA45588; Wed, 11 Oct 2000 10:41:49 -0700 (PDT) (envelope-from archie) From: Archie Cobbs Message-Id: <200010111741.e9BHfnA45588@bubba.whistle.com> Subject: Re: ip_input.c patch In-Reply-To: <20001011105823.C56373@sunbay.com> "from Ruslan Ermilov at Oct 11, 2000 10:58:23 am" To: Ruslan Ermilov Date: Wed, 11 Oct 2000 10:41:49 -0700 (PDT) Cc: bmilekic@FreeBSD.org, freebsd-net@FreeBSD.org X-Mailer: ELM [version 2.4ME+ PL82 (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org Ruslan Ermilov writes: > > +#if BYTE_ORDER != BIG_ENDIAN > > /* > > - * Convert fields to host representation. > > + * Convert fields to host representation. But first make > > + * sure we don't write into a multiply-referenced mbuf. > > */ > > + if ((m->m_flags & M_EXT) != 0 && MEXT_IS_REF(m) > > + && (m = m_pullup(m, sizeof(*ip))) == NULL) { > > + ipstat.ips_badhlen++; > > + return; > > + } > > NTOHS(ip->ip_len); > > + NTOHS(ip->ip_off); > > +#endif /* !BIG_ENDIAN */ > > if (ip->ip_len < hlen) { > > ipstat.ips_badlen++; > > goto bad; > > } > > - NTOHS(ip->ip_off); > > > > /* > > * Check that the amount of data in the buffers > > This hunk does not look fine to me. Firstly, there is no need for > BYTE_ORDER check; endian.h'es already handle this. Secondly, if you > m_pullup(), `m' may become realloced thus invalidating the `ip' pointer. > And lastly, it does not do anything useful, since at this point `m' is > already pulled up to hlen >= sizeof(struct ip). Or maybe I am just > overlooking something... You are right about not updating `ip'. Here is the justification for the other stuff: - #if BYTE_ORDER is used because the entire `if ((m->m_flags & M_EXT) ...' statement can be omitted on big endian machines, because we don't need to modify the mbuf - Secondly, `it does not do anything useful' is incorrect.. the point it to insure the mbuf is writable. The hlen >= sizeof(struct ip) pullup only happens if the first mbuf was < sizeof(struct ip), and in particular in the case of a shared cluster this will not be the case. Now, having said all that, I agree that the best solution is to forget my patches and instead change ip_input() to not modify the mbuf at all. Thanks, -Archie ___________________________________________________________________________ Archie Cobbs * Whistle Communications, Inc. * http://www.whistle.com To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message