Re: fix for IPSEC-IPV4 breakage

[Andrew Atrens <atrens@xxxxxxxxxxxxxxxxxx>]

Matthew Dillon wrote:
> 
>     Our new TCP and UDP protocol stacks require the entire header
>     to reside in the first mbuf and assert if it isn't.

Okay, that would explain it then. :)
> 
>     You have the correct solution, but could you use m_pullup() instead
>     of manually manipulating the mbuf?  I think m_pullup() will do the
>     job here.

Hmm. cbc_decrypt() takes mbuf *m as an argument. Does m_pullup() guarantee
that the root pointer isn't modified, otherwise I'll need to call it higher
up in the chain. Maybe in esp4_input.

Actually I tried using m_pullup in both spots with different results. First
I tried m_pullup as a straight replacement for my change in cbc_decrypt.
This resulted in panics in mfreem called from esp4_input.
The code I added looked like -
. ..
        if ( d0 )
                scut = m_pullup( scut, scutoff + d0->m_len );
. ..

Next I tried calling m_pullup in esp4_input, immediately following the call
to cbc_decrypt. This didn't trap, but didn't seem to work either, I added
a printf and could see the m_pullup being called, and the results from the
pullup looked to have worked as the m_len field looked to have expanded.
But those packets were just getting dropped. Not sure why yet.

        /*
         * decrypt the packet.
         */
        save_len = m->m_len;  /* XXX optimisation ? */
        if (!algo->decrypt)
                panic("internal error: no decrypt function");
        if ((*algo->decrypt)(m, off, sav, algo, ivlen)) {
                /* m is already freed */
                m = NULL;
                ipseclog((LOG_ERR, "decrypt fail in IPv4 ESP input: %s\n",
                    ipsec_logsastr(sav)));
                ipsecstat.in_inval++;
                goto bad;
        }
        ipsecstat.in_esphist[sav->alg_enc]++;

        /* pullup if the decryptor has modified the mbuf chain */
        if ( m->m_len < save_len && m->m_next ) {
                m = m_pullup(m, m->m_len + m->m_next->m_len);
                printf("doing a pullup %p %d %d\n", m, m->m_len, save_len);
        }

This code is pretty new to me (I had no clue what m_pullup was for until
you mentioned it, so I wouldn't be surprised if I've messed up the logic
somewhere :P

Andrew.