Re: infinite recursion in m_split()

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

On Fri, Apr 03, 2009 at 10:15:08AM +0200, Manuel Bouyer wrote:
> On Fri, Apr 03, 2009 at 01:22:30AM +0000, YAMAMOTO Takashi wrote:
> > > Documentatio doesn't mention that calling m_split() with a 0 len0 is 
> > > invalid.
> > > My fix is to check for (len0 == 0) in m_split() and return m0 in this 
> > > case.
> > > Is it OK ?
> > 
> > the fix seems wrong because the caller of m_split will keep a reference to
> > the original m0 as well and it will be double-freed eventually.
> > i think it's more straightforward to fix nfsrv_getstream.
> 
> Right, it appeared to me this morning (sleep gives good things sometimes :)
> The obvious place to fix it is indeed nfsrv_getstream() and add a KASSERT
> in m_split() (and update the documentation).
> 
> To bring the implementation in sync with the documentation we would need to
> return a copy of m0 in this case, but this may be overkill. It's better
> to let the caller deal with it.

Here's my proposed patch. I didn't see a panic, but I do get strange errors
with a build.sh -j16 on the linux box: build tools complaining about a
nonexistent file, when the file should have been created just before
(e.g. nbinstall complaining "can't rename nbpax.foo to nbpax: no such file or
directory"). Maybe there's something going wrong with NFS retries.
Maybe we should still copy the mbuf to be able to add it at the tail of
the slp->ns_frag chain ?

I also do get a lot of:
Apr  3 10:35:08 horn /netbsd: nfsd send error 32

I don't know if it's releated.

-- 
Manuel Bouyer, LIP6, Universite Paris VI.           
Manuel.Bouyer%lip6.fr@localhost
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: share/man/man9/mbuf.9
===================================================================
RCS file: /cvsroot/src/share/man/man9/mbuf.9,v
retrieving revision 1.48
diff -u -r1.48 mbuf.9
--- share/man/man9/mbuf.9       31 May 2008 16:24:59 -0000      1.48
+++ share/man/man9/mbuf.9       3 Apr 2009 08:50:21 -0000
@@ -494,6 +494,8 @@
 .Dv NULL
 and attempts to
 restore the chain to its original state.
+It should not be called with a zero
+.Fa len0 .
 .It Fn m_adj "struct mbuf *mp" "int req_len"
 Shaves off
 .Fa req_len
Index: sys/nfs/nfs_socket.c
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_socket.c,v
retrieving revision 1.173.4.3
diff -u -r1.173.4.3 nfs_socket.c
--- sys/nfs/nfs_socket.c        6 Feb 2009 01:48:58 -0000       1.173.4.3
+++ sys/nfs/nfs_socket.c        3 Apr 2009 08:50:21 -0000
@@ -2420,13 +2420,18 @@
                        slp->ns_raw = slp->ns_rawend = (struct mbuf *)0;
                        slp->ns_cc = slp->ns_reclen = 0;
                } else if (slp->ns_cc > slp->ns_reclen) {
-                       recm = slp->ns_raw;
-                       m = m_split(recm, slp->ns_reclen, waitflag);
-                       if (m == NULL) {
-                               error = EWOULDBLOCK;
-                               break;
+                       if (slp->ns_reclen > 0) {
+                               recm = slp->ns_raw;
+                               m = m_split(recm, slp->ns_reclen, waitflag);
+                               if (m == NULL) {
+                                       error = EWOULDBLOCK;
+                                       break;
+                               }
+                               m_claimm(recm, &nfs_mowner);
+                       } else {
+                               recm = NULL;
+                               m = slp->ns_raw;
                        }
-                       m_claimm(recm, &nfs_mowner);
                        slp->ns_raw = m;
                        if (m->m_next == NULL)
                                slp->ns_rawend = m;
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 08:50:21 -0000
@@ -1019,11 +1019,11 @@
 struct mbuf *
 m_split(struct mbuf *m0, int len0, int wait)
 {
-
+       KASSERT(len0 > 0);
        return m_split0(m0, len0, wait, 1);
 }
 
 static struct mbuf *
 m_split0(struct mbuf *m0, int len0, int wait, int copyhdr)
 {
        struct mbuf *m, *n;