NetBSD Problem Report #48746

From hikaru.abe@gmail.com  Mon Apr 14 12:19:42 2014
Return-Path: <hikaru.abe@gmail.com>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 9BE90A580F
	for <gnats-bugs@gnats.netbsd.org>; Mon, 14 Apr 2014 12:19:42 +0000 (UTC)
Message-Id: <20140414121936.3AA0914D20F3@itbnb.abear.jp>
Date: Mon, 14 Apr 2014 21:19:36 +0900 (JST)
From: hikaru.abe@gmail.com
Reply-To: hikaru.abe@gmail.com
To: gnats-bugs@gnats.NetBSD.org
Subject: mbuf corruption possibility in nfs_boot_sendrecv
X-Send-Pr-Version: 3.95

>Number:         48746
>Category:       kern
>Synopsis:       mbuf corruption possibility in nfs_boot_sendrecv
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    hikaru
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Apr 14 12:20:00 +0000 2014
>Closed-Date:    Sun Jul 05 16:23:19 +0000 2015
>Last-Modified:  Sun Jul 05 16:23:19 +0000 2015
>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;

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->hikaru
Responsible-Changed-By: hikaru@NetBSD.org
Responsible-Changed-When: Fri, 27 Mar 2015 06:09:33 +0000
Responsible-Changed-Why:
mine


State-Changed-From-To: open->pending-pullups
State-Changed-By: hikaru@NetBSD.org
State-Changed-When: Mon, 06 Apr 2015 04:52:31 +0000
State-Changed-Why:
Pulled up -7 and waiting pullup-6


State-Changed-From-To: pending-pullups->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 05 Jul 2015 16:23:19 +0000
State-Changed-Why:
There is nothing matching in the netbsd-6 pullup queue, so I assume it's
been handled. (In general it helps to include the pullup ticket numbers
when marking PRs pending-pullups)


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.