Date: Wed, 11 Oct 2000 10:58:23 +0300 From: Ruslan Ermilov <ru@FreeBSD.org> To: Archie Cobbs <archie@whistle.com> Cc: bmilekic@FreeBSD.org, freebsd-net@FreeBSD.org Subject: Re: ip_input.c patch Message-ID: <20001011105823.C56373@sunbay.com> In-Reply-To: <200010102202.e9AM2L538821@bubba.whistle.com>; from archie@whistle.com on Tue, Oct 10, 2000 at 03:02:21PM -0700 References: <200010102202.e9AM2L538821@bubba.whistle.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Oct 10, 2000 at 03:02:21PM -0700, Archie Cobbs wrote: > Bosko (and anyone else..), > > Does this patch look appropriate to you? > > Thanks, > -Archie > > ___________________________________________________________________________ > Archie Cobbs * Whistle Communications, Inc. * http://www.whistle.com > > Index: ip_input.c > =================================================================== > RCS file: /home/ncvs/src/sys/netinet/ip_input.c,v > retrieving revision 1.141 > diff -u -r1.141 ip_input.c > --- ip_input.c 2000/09/14 21:06:48 1.141 > +++ ip_input.c 2000/10/10 21:58:46 > @@ -338,15 +338,23 @@ > goto bad; > } > > +#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... > @@ -599,7 +607,7 @@ > * Reassembly should be able to treat a mbuf cluster, for later > * operation of contiguous protocol headers on the cluster. (KAME) > */ > - if (m->m_flags & M_EXT) { /* XXX */ > + if ((m->m_flags & M_EXT) != 0 && MEXT_IS_REF(m)) { > if ((m = m_pullup(m, hlen)) == 0) { > ipstat.ips_toosmall++; > #ifdef IPFIREWALL_FORWARD > @@ -688,6 +696,14 @@ > #ifdef IPDIVERT > /* Restore original checksum before diverting packet */ > if (divert_info != 0) { > + /* Don't overwrite multiply-referenced mbuf */ > + if ((m->m_flags & M_EXT) != 0 && MEXT_IS_REF(m) > + && (m = m_pullup(m, sizeof(*ip))) == NULL) { > +#ifdef IPFIREWALL_FORWARD > + ip_fw_fwd_addr = NULL; > +#endif > + return; > + } > ip->ip_len += hlen; > HTONS(ip->ip_len); > HTONS(ip->ip_off); The same here. > @@ -717,6 +733,15 @@ > /* Clone packet if we're doing a 'tee' */ > if ((divert_info & IP_FW_PORT_TEE_FLAG) != 0) > clone = m_dup(m, M_DONTWAIT); > + > + /* Don't overwrite multiply-referenced mbuf */ > + if ((m->m_flags & M_EXT) != 0 && MEXT_IS_REF(m) > + && (m = m_pullup(m, sizeof(*ip))) == NULL) { > +#ifdef IPFIREWALL_FORWARD > + ip_fw_fwd_addr = NULL; > +#endif > + return; > + } > > /* Restore packet header fields to original values */ > ip->ip_len += hlen; > > The same here. -- Ruslan Ermilov Oracle Developer/DBA, ru@sunbay.com Sunbay Software AG, ru@FreeBSD.org FreeBSD committer, +380.652.512.251 Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20001011105823.C56373>