Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 06 Nov 2004 11:49:17 +0100
From:      Andre Oppermann <andre@freebsd.org>
To:        James <haesu@towardex.com>
Cc:        freebsd-net@freebsd.org
Subject:   Re: ip_fastforward() sanity check..
Message-ID:  <418CAC2D.41A7B5DD@freebsd.org>
References:  <20041106102539.GA30766@scylla.towardex.com>

next in thread | previous in thread | raw e-mail | index | archive | help
James wrote:
> 
> I seem to have a little concern in one specific early-sanity check in the
> ip_fastforward() function of the latest 5.3 code base:
> 
>         /*
>          * Is first mbuf large enough for ip header and is header present?
>          */
>         if (m->m_len < sizeof (struct ip) &&
>            (m = m_pullup(m, sizeof (struct ip))) == 0) {
>                 ipstat.ips_toosmall++;
>                 goto drop;
>         }
> 
> Okay, if m_pullup() returns 0 due to failure, it already called m_freem(m) by
> itself. But we have "goto drop;" after that, which is redundant, no?
> 
> I don't think this is a bit of issue in IPv4 implementation, but as obviously,
> in IPv6 implementation, if calling 'goto drop' or redundant m_freem(m) in case
> where m_pullup returns NULL/0, it may crash the kernel rock hard at
> m_tag_delete_chain in uipc_mbuf.c (even if you are checking 'if (m) m_freem(m)'
> as remains are left over)
> 
> If any one has any comments, please let me know. If this is not a concern
> please disregard my rant and excuse me for waste of time :)

This is indeed a bug.  Fixed in ip_fastfwd.c rev 1.24 a couple of minutes
ago.  Thanks for reporting.

-- 
Andre



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?418CAC2D.41A7B5DD>