Re: infinite recursion in m_split()

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Fri, Apr 03, 2009 at 06:18:35PM +0200, Manuel Bouyer wrote:
> [...]
> already happened. The attached patch gives more details on how we get in
> this situation:
> m 0xc3bae000 m0 0xc3bae000 remain 1444 len_save -4 len0 0 len 0 m_len 1444 
> MHLEN 200
> panic: m_split0

Sorry forgot the patch, here it is

> 
> This mbuf obviously has a cluster attached (gdb on the core dump confirmed),
> yet it doesn't have M_EXT set (the (m->m_flags & M_EXT) goto didn't
> catch it, and gdb on the core dump also confirms). So the mbuf got partially
> updated before m_split() I also guess m0->m_pkthdr.len is not supposed
> to be negative, yet we have len_save as -4 (I've also seen -252 or -260).
> So something is putting wrong values in the mbuf before calling m_split()
> on it, but I don't know where it happens.
> The printf() I added to check for len0 == 0 at the top of m_split()
> fires only once, so I guess when slp->ns_reclen == 0 damage has already
> been done to the associated mbuf; it could be related to this
> specific condition.
> 
> Any idea very welcome.
> 
> -- 
> Manuel Bouyer, LIP6, Universite Paris VI.           
> Manuel.Bouyer%lip6.fr@localhost
>      NetBSD: 26 ans d'experience feront toujours la difference
> --
-- 
Manuel Bouyer, LIP6, Universite Paris VI.           
Manuel.Bouyer%lip6.fr@localhost
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: sys/kern/uipc_mbuf.c
===================================================================
RCS file: /cvsroot/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.128
diff -u -r1.128 uipc_mbuf.c
--- sys/kern/uipc_mbuf.c        2 Jul 2008 14:47:34 -0000       1.128
+++ sys/kern/uipc_mbuf.c        3 Apr 2009 16:16:47 -0000
@@ -108,7 +108,7 @@
 };
 
 static struct mbuf *m_copym0(struct mbuf *, int, int, int, int);
-static struct mbuf *m_split0(struct mbuf *, int, int, int);
+struct mbuf *m_split0(struct mbuf *, int, int, int);
 static int m_copyback0(struct mbuf **, int, int, const void *, int, int);
 
 /* flags for m_copyback0 */
@@ -1019,11 +1019,12 @@
 struct mbuf *
 m_split(struct mbuf *m0, int len0, int wait)
 {
-
+       if (len0 == 0) 
+               printf("m_split(%p, 0, %d)\n", m0, wait);
        return m_split0(m0, len0, wait, 1);
 }
 
-static struct mbuf *
+struct mbuf *
 m_split0(struct mbuf *m0, int len0, int wait, int copyhdr)
 {
        struct mbuf *m, *n;
@@ -1046,10 +1047,16 @@
                if (m->m_flags & M_EXT)
                        goto extpacket;
                if (remain > MHLEN) {
+                       if (m == m0) {
+                               printf ("m %p m0 %p remain %d len_save %d len0 
%d len %d m_len %d MHLEN %d\n", m, m0, remain, len_save, len0, len, m->m_len, 
MHLEN);
+                               panic("m_split0");
+                       }
                        /* m can't be the lead packet */
+                       KASSERT(m != m0);
+                       KASSERT((m->m_flags & M_PKTHDR) == 0);
                        MH_ALIGN(n, 0);
                        n->m_next = m_split(m, len, wait);
-                       if (n->m_next == 0) {
+                       if (n->m_next == NULL) {
                                (void) m_free(n);
                                m0->m_pkthdr.len = len_save;
                                return (NULL);
@@ -1059,11 +1066,11 @@
                        MH_ALIGN(n, remain);
        } else if (remain == 0) {
                n = m->m_next;
-               m->m_next = 0;
+               m->m_next = NULL;
                return (n);
        } else {
                MGET(n, wait, m->m_type);
-               if (n == 0)
+               if (n == NULL)
                        return (NULL);
                MCLAIM(n, m->m_owner);
                M_ALIGN(n, remain);
@@ -1078,7 +1085,7 @@
        n->m_len = remain;
        m->m_len = len;
        n->m_next = m->m_next;
-       m->m_next = 0;
+       m->m_next = NULL;
        return (n);
 }
 /*