Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 11 Oct 2000 10:41:49 -0700 (PDT)
From:      Archie Cobbs <archie@whistle.com>
To:        Ruslan Ermilov <ru@FreeBSD.org>
Cc:        bmilekic@FreeBSD.org, freebsd-net@FreeBSD.org
Subject:   Re: ip_input.c patch
Message-ID:  <200010111741.e9BHfnA45588@bubba.whistle.com>
In-Reply-To: <20001011105823.C56373@sunbay.com> "from Ruslan Ermilov at Oct 11, 2000 10:58:23 am"

next in thread | previous in thread | raw e-mail | index | archive | help
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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200010111741.e9BHfnA45588>