Re: CVS commit: src/sys

[David Brownlee <abs%absd.org@localhost>]

On 15 December 2017 at 14:24, Robert Elz <kre%munnari.oz.au@localhost> wrote:
>
>     Date:        Fri, 15 Dec 2017 05:01:17 +0000
>     From:        "Kengo NAKAHARA" <knakahara%netbsd.org@localhost>
>     Message-ID:  <20171215050117.241BAFB40%cvs.NetBSD.org@localhost>
>
>   | Fix pullup'ed mbuf leaks.
>   | The match function just requires enough mbuf length.
>
> This is still not correct.   There are no mbuf leaks, I think you did
> not understand maxv's point (if I remember it correctly).
>
> The problem isn't lost mbufs, it is that if code does a m_pullup()
> (or m_ensure_contig() which is essentially the same thing) the
> mbuf chain can be altered.
>
> What that means is that after a call of in_l2tp_match(m, ...) in the
> calling function, the value of m has been lost - using the old one is
> incorrect and anything might happen (up to and including a panic).
>
> This is why so many mbuf functions pass a struct mbuf ** instead of a
> struct mbuf * so when the parameter is changed, it gets passed back to
> the caller.   This function probably needs something like that (the other
> way is returning the updated m value, but I don't think that fits here.)
>
> Unfortunately, because of the way it gets called, this might not be an
> easy change to make.
>
> The problem occurs way back in encap4_input() which does
>
>         match = encap4_lookup(m,...);
>
> and then later
>
>         encap_fillarg(m, match);
> or
>         m_freem(m);
> or
>         rip_input(m, off, proto);
>
> all of which use the 'm' that was passed to encap4_lookup().
>
> encap4_lookup does ..
>
>         prio = (*ep->func)(m, off, proto, ep->arg);
>
> where *ep->func is in_l2tp_match which does the m_pullup() or now
> m_ensure_contig() instead which potentially changes what 'm' should
> be (the old one may have been freed and replaced by a different one.)

Just wondering, is there scope for a (rather heavy) debug option in
which m_ensure_contig() always returns a new mbuf for valid input?

David