From: claudio <claudio@openbsd.org>
Date: Wed, 11 Sep 2024 12:22:34 +0000 (+0000)
Subject: After calling m_freem() on nmi_mrep (or nmi_mreq) set the pointer to NULL.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=df0421f788c05a447b4b21994ae20d538867f53b;p=openbsd

After calling m_freem() on nmi_mrep (or nmi_mreq) set the pointer to NULL.

Only do this if struct nfsm_info doesn't have local scope.
In some cases the caller would perfrom another m_freem and double free
the mbuf and Bad Things(TM) would happen.

Reported by Claes M Nyberg on bugs@; with & ok miod@
---

diff --git a/sys/nfs/nfs_serv.c b/sys/nfs/nfs_serv.c
index 05699b8dd1a..df2a079d687 100644
--- a/sys/nfs/nfs_serv.c
+++ b/sys/nfs/nfs_serv.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_serv.c,v 1.128 2024/09/10 18:44:04 miod Exp $	*/
+/*	$OpenBSD: nfs_serv.c,v 1.129 2024/09/11 12:22:34 claudio Exp $	*/
 /*     $NetBSD: nfs_serv.c,v 1.34 1997/05/12 23:37:12 fvdl Exp $       */
 
 /*
@@ -94,10 +94,8 @@ nfsm_reply(struct nfsm_info *infop, struct nfsrv_descript *nfsd,
 		statuslen = 0;
 	(void)nfs_rephead(statuslen, nfsd, slp, error,
 		&infop->nmi_mreq, &infop->nmi_mb);
-	if (infop->nmi_mrep != NULL) {
-		m_freem(infop->nmi_mrep);
-		infop->nmi_mrep = NULL;
-	}
+	m_freem(infop->nmi_mrep);
+	infop->nmi_mrep = NULL;
 	*mrq = infop->nmi_mreq;
 	if (error && (!infop->nmi_v3 || error == EBADRPC))
 		return error;
diff --git a/sys/nfs/nfs_socket.c b/sys/nfs/nfs_socket.c
index aa22757800e..8dc32b230e5 100644
--- a/sys/nfs/nfs_socket.c
+++ b/sys/nfs/nfs_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_socket.c,v 1.152 2024/09/04 07:54:53 mglocker Exp $	*/
+/*	$OpenBSD: nfs_socket.c,v 1.153 2024/09/11 12:22:34 claudio Exp $	*/
 /*	$NetBSD: nfs_socket.c,v 1.27 1996/04/15 20:20:00 thorpej Exp $	*/
 
 /*
@@ -1020,6 +1020,7 @@ tryagain:
 			if ((nmp->nm_flag & NFSMNT_NFSV3) &&
 			    error == NFSERR_TRYLATER) {
 				m_freem(info.nmi_mrep);
+				info.nmi_mrep = NULL;
 				error = 0;
 				tsleep_nsec(&nowake, PSOCK, "nfsretry",
 				    SEC_TO_NSEC(trylater_delay));
diff --git a/sys/nfs/nfs_vnops.c b/sys/nfs/nfs_vnops.c
index 0c83210624d..1e307f33169 100644
--- a/sys/nfs/nfs_vnops.c
+++ b/sys/nfs/nfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_vnops.c,v 1.201 2024/07/06 09:53:25 jsg Exp $	*/
+/*	$OpenBSD: nfs_vnops.c,v 1.202 2024/09/11 12:22:34 claudio Exp $	*/
 /*	$NetBSD: nfs_vnops.c,v 1.62.4.1 1996/07/08 20:26:52 jtc Exp $	*/
 
 /*
@@ -540,6 +540,7 @@ nfsm_loadattr(struct nfsm_info *infop, struct vnode **vpp, struct vattr *vap)
 	error = nfs_loadattrcache(&ttvp, &infop->nmi_md, &infop->nmi_dpos, vap);
 	if (error != 0) {
 		m_freem(infop->nmi_mrep);
+		infop->nmi_mrep = NULL;
 		*infop->nmi_errorp = error;
 		return error;
 	}
@@ -781,6 +782,7 @@ nfsm_getfh(struct nfsm_info *infop, int *sizep, int v3)
 		size = fxdr_unsigned(int, *tl);
 		if (size <= 0 || size > NFSX_V3FHMAX) {
 			m_freem(infop->nmi_mrep);
+			infop->nmi_mrep = NULL;
 			*infop->nmi_errorp = EBADRPC;
 			return NULL;
 		}
@@ -1400,6 +1402,7 @@ nfsm_mtofh(struct nfsm_info *infop, struct vnode *dvp, struct vnode **vpp,
 		error = nfs_nget(dvp->v_mount, ttfhp, ttfhsize, &ttnp);
 		if (error != 0) {
 			m_freem(infop->nmi_mrep);
+			infop->nmi_mrep = NULL;
 			*infop->nmi_errorp = error;
 			return error;
 		}
diff --git a/sys/nfs/nfsm_subs.h b/sys/nfs/nfsm_subs.h
index c669e5ff3b2..2a794811b8a 100644
--- a/sys/nfs/nfsm_subs.h
+++ b/sys/nfs/nfsm_subs.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfsm_subs.h,v 1.48 2024/04/30 17:04:23 miod Exp $	*/
+/*	$OpenBSD: nfsm_subs.h,v 1.49 2024/09/11 12:22:34 claudio Exp $	*/
 /*	$NetBSD: nfsm_subs.h,v 1.10 1996/03/20 21:59:56 fvdl Exp $	*/
 
 /*
@@ -72,6 +72,7 @@ nfsm_dissect(struct nfsm_info *infop, int s)
 	error = nfsm_disct(&infop->nmi_md, &infop->nmi_dpos, s, avail, &ret);
 	if (error != 0) {
 		m_freem(infop->nmi_mrep);
+		infop->nmi_mrep = NULL;
 		*infop->nmi_errorp = error;
 		return NULL;
 	} else {
@@ -95,6 +96,7 @@ nfsm_adv(struct nfsm_info *infop, int s)
 	error = nfs_adv(&infop->nmi_md, &infop->nmi_dpos, s, avail);
 	if (error != 0) {
 		m_freem(infop->nmi_mrep);
+		infop->nmi_mrep = NULL;
 		*infop->nmi_errorp = error;
 		return error;
 	}
@@ -121,6 +123,7 @@ nfsm_postop_attr(struct nfsm_info *infop, struct vnode **vpp, int *attrflagp)
 		    &infop->nmi_dpos, NULL);
 		if (error != 0) {
 			m_freem(infop->nmi_mrep);
+			infop->nmi_mrep = NULL;
 			*infop->nmi_errorp = error;
 			return error;
 		}
@@ -140,6 +143,7 @@ nfsm_strsiz(struct nfsm_info *infop, int *lenp, int maxlen)
 	len = fxdr_unsigned(int32_t, *tl);
 	if (len < 0 || len > maxlen) {
 		m_freem(infop->nmi_mrep);
+		infop->nmi_mrep = NULL;
 		*infop->nmi_errorp = EBADRPC;
 		return 1;
 	}
@@ -158,6 +162,7 @@ nfsm_mtouio(struct nfsm_info *infop, struct uio *uiop, int len)
 	error = nfsm_mbuftouio(&infop->nmi_md, uiop, len, &infop->nmi_dpos);
 	if (error != 0) {
 		m_freem(infop->nmi_mrep);
+		infop->nmi_mrep = NULL;
 		*infop->nmi_errorp = error;
 		return error;
 	}
@@ -169,6 +174,7 @@ nfsm_strtom(struct nfsm_info *infop, char *str, size_t len, size_t maxlen)
 {
 	if (len > maxlen) {
 		m_freem(infop->nmi_mreq);
+		infop->nmi_mreq = NULL;
 		*infop->nmi_errorp = ENAMETOOLONG;
 		return 1;
 	}