kern/48746: mbuf corruption possibility in nfs_boot_sendrecv

[hikaru.abe%gmail.com@localhost]

>Number:         48746
>Category:       kern
>Synopsis:       mbuf corruption possibility in nfs_boot_sendrecv
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Apr 14 12:20:00 +0000 2014
>Originator:     Hikaru Abe
>Release:        NetBSD NetBSD 6.99.40
>Organization:
>Environment:
        $NetBSD: nfs_boot.c,v 1.81 2013/10/25 20:46:29 martin Exp $
>Description:
        m_pullup() is called in rcvproc callback functions, so
        nfs_boot_sendrecv() should keep track of the head of mbuf chain.
>How-To-Repeat:
        see above
>Fix:

Index: nfs/nfsdiskless.h
===================================================================
RCS file: /mirror/netbsd/src/sys/nfs/nfsdiskless.h,v
retrieving revision 1.30
diff -u -r1.30 nfsdiskless.h
--- nfs/nfsdiskless.h   4 Oct 2010 23:48:23 -0000       1.30
+++ nfs/nfsdiskless.h   13 Apr 2014 09:04:20 -0000
@@ -79,7 +79,7 @@
 int nfs_boot_sobind_ipport (struct socket *, uint16_t, struct lwp *);
 int nfs_boot_sendrecv (struct socket *, struct mbuf *,
                           int (*)(struct mbuf*, void*, int), struct mbuf*,
-                          int (*)(struct mbuf*, void*), struct mbuf**,
+                          int (*)(struct mbuf**, void*), struct mbuf**,
                           struct mbuf**, void*, struct lwp *);
 
 int nfs_bootdhcp  (struct nfs_diskless *, struct lwp *, int *);
Index: nfs/nfs_bootdhcp.c
===================================================================
RCS file: /mirror/netbsd/src/sys/nfs/nfs_bootdhcp.c,v
retrieving revision 1.52
diff -u -r1.52 nfs_bootdhcp.c
--- nfs/nfs_bootdhcp.c  4 Oct 2010 23:48:22 -0000       1.52
+++ nfs/nfs_bootdhcp.c  13 Apr 2014 10:02:30 -0000
@@ -302,7 +302,7 @@
 };
 
 static int bootpset (struct mbuf*, void*, int);
-static int bootpcheck (struct mbuf*, void*);
+static int bootpcheck (struct mbuf**, void*);
 
 static int
 bootpset(struct mbuf *m, void *context, int waited)
@@ -318,10 +318,11 @@
 }
 
 static int
-bootpcheck(struct mbuf *m, void *context)
+bootpcheck(struct mbuf **mp, void *context)
 {
        struct bootp *bootp;
        struct bootpcontext *bpc = context;
+       struct mbuf *m = *mp;
        u_int tag, len;
        u_char *p, *limit;
 
@@ -343,7 +344,7 @@
         * don't make first checks more expensive than necessary
         */
        if (m->m_len < offsetof(struct bootp, bp_sname)) {
-               m = m_pullup(m, offsetof(struct bootp, bp_sname));
+               m = *mp = m_pullup(m, offsetof(struct bootp, bp_sname));
                if (m == NULL) {
                        DPRINTF(("bootpcheck: m_pullup failed\n"));
                        return (-1);
Index: nfs/nfs_boot.c
===================================================================
RCS file: /mirror/netbsd/src/sys/nfs/nfs_boot.c,v
retrieving revision 1.81
diff -u -r1.81 nfs_boot.c
--- nfs/nfs_boot.c      25 Oct 2013 20:46:29 -0000      1.81
+++ nfs/nfs_boot.c      13 Apr 2014 16:12:15 -0000
@@ -432,7 +432,7 @@
 nfs_boot_sendrecv(struct socket *so, struct mbuf *nam,
                int (*sndproc)(struct mbuf *, void *, int),
                struct mbuf *snd,
-               int (*rcvproc)(struct mbuf *, void *),
+               int (*rcvproc)(struct mbuf **, void *),
                struct mbuf **rcv, struct mbuf **from_p,
                void *context, struct lwp *lwp)
 {
@@ -510,7 +510,7 @@
                        panic("nfs_boot_sendrecv: return size");
 #endif
 
-               if ((*rcvproc)(m, context))
+               if ((*rcvproc)(&m, context))
                        continue;
 
                if (rcv)
Index: nfs/krpc_subr.c
===================================================================
RCS file: /mirror/netbsd/src/sys/nfs/krpc_subr.c,v
retrieving revision 1.37
diff -u -r1.37 krpc_subr.c
--- nfs/krpc_subr.c     15 Mar 2009 17:20:09 -0000      1.37
+++ nfs/krpc_subr.c     13 Apr 2014 09:59:00 -0000
@@ -124,7 +124,7 @@
 
 #define MIN_REPLY_HDR 16       /* xid, dir, astat, errno */
 
-static int krpccheck(struct mbuf*, void*);
+static int krpccheck(struct mbuf**, void*);
 
 /*
  * Call portmap to lookup a port number for a particular rpc program
@@ -183,15 +183,16 @@
        return 0;
 }
 
-static int krpccheck(struct mbuf *m, void *context)
+static int krpccheck(struct mbuf **mp, void *context)
 {
        struct rpc_reply *reply;
+       struct mbuf *m = *mp;
 
        /* Does the reply contain at least a header? */
        if (m->m_pkthdr.len < MIN_REPLY_HDR)
                return(-1);
        if (m->m_len < sizeof(struct rpc_reply)) {
-               m = m_pullup(m, sizeof(struct rpc_reply));
+               m = *mp = m_pullup(m, sizeof(struct rpc_reply));
                if (m == NULL)
                        return(-1);
        }
Index: kern/subr_tftproot.c
===================================================================
RCS file: /mirror/netbsd/src/sys/kern/subr_tftproot.c,v
retrieving revision 1.12
diff -u -r1.12 subr_tftproot.c
--- kern/subr_tftproot.c        1 Dec 2012 11:41:50 -0000       1.12
+++ kern/subr_tftproot.c        14 Apr 2014 11:37:01 -0000
@@ -117,7 +117,7 @@
 int tftproot_dhcpboot(device_t);
 
 static int tftproot_getfile(struct tftproot_handle *, struct lwp *);
-static int tftproot_recv(struct mbuf *, void *);
+static int tftproot_recv(struct mbuf **, void *);
 
 int
 tftproot_dhcpboot(device_t bootdv)
@@ -349,10 +349,11 @@
 }
 
 static int
-tftproot_recv(struct mbuf *m, void *ctx)
+tftproot_recv(struct mbuf **mp, void *ctx)
 {
        struct tftproot_handle *trh = ctx;
        struct tftphdr *tftp;
+       struct mbuf *m = *mp;
        size_t newlen;
        size_t hdrlen = sizeof(*tftp) - sizeof(tftp->th_data);
 
@@ -379,7 +380,7 @@
         * Examine the TFTP header
         */
        if (m->m_len > sizeof(*tftp)) {
-               if ((m = m_pullup(m, sizeof(*tftp))) == NULL) {
+               if ((m = *mp = m_pullup(m, sizeof(*tftp))) == NULL) {
                        DPRINTF(("%s():%d m_pullup failed\n",
                            __func__, __LINE__));
                        return -1;