On Wed, Sep 22, 2010 at 06:28:38PM +0200, Mike Belopuhov wrote:
> m_pad was introduced with an "ipsec package" import in openbsd
> in 1997. m_inject was introduced in 1999 but this code wasn't
> changed.
>
> m_pad is equivalent to m_inject with an offset equal to the
> actual data length.
>
> Doesn't break anything here, but additional tests won't harm.
No objection in principle (consolidation of code in mbufs is
one of my currently-stalled-by-dayjob projects); comment inline.
>
> OK?
>
> Index: netinet/ip_esp.c
> ===================================================================
> RCS file: /home/cvs/src/sys/netinet/ip_esp.c,v
> retrieving revision 1.112
> diff -u -p -r1.112 ip_esp.c
> --- netinet/ip_esp.c 22 Sep 2010 13:40:05 -0000 1.112
> +++ netinet/ip_esp.c 22 Sep 2010 15:47:38 -0000
> @@ -932,9 +932,10 @@ esp_output(struct mbuf *m, struct tdb *t
> * Add padding -- better to do it ourselves than use the crypto engine,
> * although if/when we support compression, we'd have to do that.
> */
> - pad = (u_char *) m_pad(m, padding + alen);
> + mo = m_inject(m, m->m_pkthdr.len, padding + alen, M_DONTWAIT);
> + pad = mtod(mo, u_char *);
Sure, but this code creates a NULL pointer dereference;
mtod() isn't a function that does anything smart, as
is evidenced by the explanation in the mbuf man page:
#define mtod(m,t) ((t)((m)->m_data))
> if (pad == NULL) {
> - DPRINTF(("esp_output(): m_pad() failed for SA %s/%08x\n",
> + DPRINTF(("esp_output(): m_inject failed for SA %s/%08x\n",
> ipsp_address(tdb->tdb_dst), ntohl(tdb->tdb_spi)));
> return ENOBUFS;
> }
> @@ -1178,77 +1179,4 @@ checkreplaywindow32(u_int32_t seq, u_int
>
> *bitmap |= (((u_int32_t) 1) << diff);
> return 0;
> -}
> -
> -/*
> - * m_pad(m, n) pads <m> with <n> bytes at the end. The packet header
> - * length is updated, and a pointer to the first byte of the padding
> - * (which is guaranteed to be all in one mbuf) is returned.
> - */
> -
> -caddr_t
> -m_pad(struct mbuf *m, int n)
> -{
> - struct mbuf *m0, *m1;
> - int len, pad;
> - caddr_t retval;
> -
> - if (n <= 0) { /* No stupid arguments. */
> - DPRINTF(("m_pad(): pad length invalid (%d)\n", n));
> - m_freem(m);
> - return NULL;
> - }
> -
> - len = m->m_pkthdr.len;
> - pad = n;
> - m0 = m;
> -
> - while (m0->m_len < len) {
> - len -= m0->m_len;
> - m0 = m0->m_next;
> - }
> -
> - if (m0->m_len != len) {
> - DPRINTF(("m_pad(): length mismatch (should be %d instead of "
> - "%d)\n", m->m_pkthdr.len,
> - m->m_pkthdr.len + m0->m_len - len));
> -
> - m_freem(m);
> - return NULL;
> - }
> -
> - /* Check for zero-length trailing mbufs, and find the last one. */
> - for (m1 = m0; m1->m_next; m1 = m1->m_next) {
> - if (m1->m_next->m_len != 0) {
> - DPRINTF(("m_pad(): length mismatch (should be %d "
> - "instead of %d)\n", m->m_pkthdr.len,
> - m->m_pkthdr.len + m1->m_next->m_len));
> -
> - m_freem(m);
> - return NULL;
> - }
> -
> - m0 = m1->m_next;
> - }
> -
> - if ((m0->m_flags & M_EXT) ||
> - m0->m_data + m0->m_len + pad >= &(m0->m_dat[MLEN])) {
> - /* Add an mbuf to the chain. */
> - MGET(m1, M_DONTWAIT, MT_DATA);
> - if (m1 == 0) {
> - m_freem(m0);
> - DPRINTF(("m_pad(): cannot append\n"));
> - return NULL;
> - }
> -
> - m0->m_next = m1;
> - m0 = m1;
> - m0->m_len = 0;
> - }
> -
> - retval = m0->m_data + m0->m_len;
> - m0->m_len += pad;
> - m->m_pkthdr.len += pad;
> -
> - return retval;
> }
> Index: netinet/ip_ipsp.h
> ===================================================================
> RCS file: /home/cvs/src/sys/netinet/ip_ipsp.h,v
> retrieving revision 1.144
> diff -u -p -r1.144 ip_ipsp.h
> --- netinet/ip_ipsp.h 9 Jul 2010 16:58:06 -0000 1.144
> +++ netinet/ip_ipsp.h 22 Sep 2010 15:49:32 -0000
> @@ -624,9 +624,6 @@ extern int tcp_signature_tdb_input(struc
> extern int tcp_signature_tdb_output(struct mbuf *, struct tdb *,
> struct mbuf **, int, int);
>
> -/* Padding */
> -extern caddr_t m_pad(struct mbuf *, int);
> -
> /* Replay window */
> extern int checkreplaywindow32(u_int32_t, u_int32_t, u_int32_t *, u_int32_t,
> u_int32_t *, int);
