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 05699b8..df2a079 100644 (file)
--- 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 aa22757..8dc32b2 100644 (file)
--- 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 0c83210..1e307f3 100644 (file)
--- 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 c669e5f..2a79481 100644 (file)
--- 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;
        }