When dealing with mbuf pointers passed down as function parameters,
authorbluhm <bluhm@openbsd.org>
Mon, 19 Jun 2017 17:58:49 +0000 (17:58 +0000)
committerbluhm <bluhm@openbsd.org>
Mon, 19 Jun 2017 17:58:49 +0000 (17:58 +0000)
bugs could easily result in use-after-free or double free.  Introduce
m_freemp() which automatically resets the pointer before freeing
it.  So we have less dangling pointers in the kernel.
OK krw@ mpi@ claudio@

15 files changed:
share/man/man9/mbuf.9
sys/net/bsd-comp.c
sys/net/if_gif.c
sys/net/pf_norm.c
sys/net/ppp-deflate.c
sys/netinet/igmp.c
sys/netinet/ip_carp.c
sys/netinet/ip_ether.c
sys/netinet/ip_icmp.c
sys/netinet/ip_input.c
sys/netinet/ip_ipip.c
sys/netinet/ipsec_input.c
sys/netinet6/ip6_input.c
sys/nfs/nfs_socket.c
sys/sys/mbuf.h

index dba1344..49b3922 100644 (file)
@@ -1,4 +1,4 @@
-.\"     $OpenBSD: mbuf.9,v 1.107 2017/05/19 05:59:27 jmc Exp $
+.\"     $OpenBSD: mbuf.9,v 1.108 2017/06/19 17:58:49 bluhm Exp $
 .\"
 .\" Copyright (c) 2001 Jean-Jacques Bernard-Gundol <jjbg@openbsd.org>
 .\" All rights reserved.
@@ -25,7 +25,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: May 19 2017 $
+.Dd $Mdocdate: June 19 2017 $
 .Dt MGET 9
 .Os
 .Sh NAME
@@ -48,6 +48,7 @@
 .Nm m_copyback ,
 .Nm m_defrag ,
 .Nm m_freem ,
+.Nm m_freemp ,
 .Nm m_purge ,
 .Nm m_reclaim ,
 .Nm m_copydata ,
 .Fn m_defrag "struct mbuf *m" "int wait"
 .Ft struct mbuf *
 .Fn m_freem "struct mbuf *m"
+.Ft struct mbuf *
+.Fn m_freem "struct mbuf **mp"
 .Ft void
 .Fn m_purge "struct mbuf *m"
 .Ft void
@@ -637,6 +640,11 @@ is a
 pointer, no action occurs and
 .Dv NULL
 is returned.
+.It Fn m_freemp "struct mbuf **mp"
+Set the input mbuf pointer to
+.Dv NULL
+and call
+.Fn m_freem .
 .It Fn m_purge "struct mbuf *m"
 Free the list of mbufs linked by m_nextpkt that is pointed to by
 .Fa m .
index 3816a4f..d087a63 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: bsd-comp.c,v 1.13 2015/11/24 13:37:16 mpi Exp $       */
+/*     $OpenBSD: bsd-comp.c,v 1.14 2017/06/19 17:58:49 bluhm Exp $     */
 /*     $NetBSD: bsd-comp.c,v 1.6 1996/10/13 02:10:58 christos Exp $    */
 
 /* Because this code is derived from the 4.3BSD compress source:
@@ -661,8 +661,7 @@ bsd_compress(state, mret, mp, slen, maxolen)
     ++db->uncomp_count;
     if (olen + PPP_HDRLEN + BSD_OVHD > maxolen) {
        /* throw away the compressed stuff if it is longer than uncompressed */
-       m_freem(*mret);
-       *mret = NULL;
+       m_freemp(mret);
 
        ++db->incomp_count;
        db->incomp_bytes += ilen;
index 446ccc9..051cbcf 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_gif.c,v 1.96 2017/05/18 10:56:45 bluhm Exp $       */
+/*     $OpenBSD: if_gif.c,v 1.97 2017/06/19 17:58:49 bluhm Exp $       */
 /*     $KAME: if_gif.c,v 1.43 2001/02/20 08:51:07 itojun Exp $ */
 
 /*
@@ -277,7 +277,7 @@ gif_encap(struct ifnet *ifp, struct mbuf **mp, sa_family_t af)
                break;
 #endif
        default:
-               m_freem(*mp);
+               m_freemp(mp);
                error = EAFNOSUPPORT;
                break;
        }
index e694b55..c1f8d5e 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf_norm.c,v 1.205 2017/06/05 22:18:28 sashan Exp $ */
+/*     $OpenBSD: pf_norm.c,v 1.206 2017/06/19 17:58:49 bluhm Exp $ */
 
 /*
  * Copyright 2001 Niels Provos <provos@citi.umich.edu>
@@ -773,8 +773,7 @@ pf_refragment6(struct mbuf **m0, struct m_tag *mtag, struct sockaddr_in6 *dst,
        (*m0)->m_nextpkt = NULL;
        if (error == 0) {
                /* The first mbuf contains the unfragmented packet */
-               m_freem(*m0);
-               *m0 = NULL;
+               m_freemp(m0);
                action = PF_PASS;
        } else {
                /* Drop expects an mbuf to free */
index 98fe311..cd75e9c 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ppp-deflate.c,v 1.12 2015/07/15 22:16:42 deraadt Exp $        */
+/*     $OpenBSD: ppp-deflate.c,v 1.13 2017/06/19 17:58:49 bluhm Exp $  */
 /*     $NetBSD: ppp-deflate.c,v 1.1 1996/03/15 02:28:09 paulus Exp $   */
 
 /*
@@ -342,8 +342,7 @@ z_compress(arg, mret, mp, orig_len, maxolen)
        state->stats.comp_bytes += olen;
        state->stats.comp_packets++;
     } else {
-       m_freem(*mret);
-       *mret = NULL;
+       m_freemp(mret);
 
        state->stats.inc_bytes += orig_len;
        state->stats.inc_packets++;
index ed7bba4..b4b2388 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: igmp.c,v 1.68 2017/05/16 12:24:01 mpi Exp $   */
+/*     $OpenBSD: igmp.c,v 1.69 2017/06/19 17:58:49 bluhm Exp $ */
 /*     $NetBSD: igmp.c,v 1.15 1996/02/13 23:41:25 christos Exp $       */
 
 /*
@@ -217,7 +217,7 @@ igmp_input(struct mbuf **mp, int *offp, int proto, int af)
 
        ifp = if_get((*mp)->m_pkthdr.ph_ifidx);
        if (ifp == NULL) {
-               m_freem(*mp);
+               m_freemp(mp);
                return IPPROTO_DONE;
        }
 
index 97af0d3..fc6f5a1 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_carp.c,v 1.312 2017/05/30 12:09:27 friehm Exp $    */
+/*     $OpenBSD: ip_carp.c,v 1.313 2017/06/19 17:58:49 bluhm Exp $     */
 
 /*
  * Copyright (c) 2002 Michael Shalayeff. All rights reserved.
@@ -421,7 +421,7 @@ carp_proto_input(struct mbuf **mp, int *offp, int proto, int af)
 
        ifp = if_get((*mp)->m_pkthdr.ph_ifidx);
        if (ifp == NULL) {
-               m_freem(*mp);
+               m_freemp(mp);
                return IPPROTO_DONE;
        }
 
@@ -517,7 +517,7 @@ carp6_proto_input(struct mbuf **mp, int *offp, int proto, int af)
 
        ifp = if_get((*mp)->m_pkthdr.ph_ifidx);
        if (ifp == NULL) {
-               m_freem(*mp);
+               m_freemp(mp);
                return IPPROTO_DONE;
        }
 
index 2fe409e..c90b614 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_ether.c,v 1.85 2017/04/14 20:46:31 bluhm Exp $  */
+/*     $OpenBSD: ip_ether.c,v 1.86 2017/06/19 17:58:49 bluhm Exp $  */
 /*
  * The author of this code is Angelos D. Keromytis (kermit@adk.gr)
  *
@@ -97,7 +97,7 @@ etherip_input(struct mbuf **mp, int *offp, int proto, int af)
                if (!etherip_allow && ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
                        DPRINTF(("etherip_input(): dropped due to policy\n"));
                        etheripstat.etherips_pdrops++;
-                       m_freem(*mp);
+                       m_freemp(mp);
                        return IPPROTO_DONE;
                }
                etherip_decap(*mp, *offp);
@@ -111,7 +111,7 @@ etherip_input(struct mbuf **mp, int *offp, int proto, int af)
        default:
                DPRINTF(("etherip_input(): dropped, unhandled protocol\n"));
                etheripstat.etherips_pdrops++;
-               m_freem(*mp);
+               m_freemp(mp);
                return IPPROTO_DONE;
        }
 }
index 1c32137..a30c96d 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_icmp.c,v 1.169 2017/05/30 12:09:27 friehm Exp $    */
+/*     $OpenBSD: ip_icmp.c,v 1.170 2017/06/19 17:58:49 bluhm Exp $     */
 /*     $NetBSD: ip_icmp.c,v 1.19 1996/02/13 23:42:22 christos Exp $    */
 
 /*
@@ -312,7 +312,7 @@ icmp_input(struct mbuf **mp, int *offp, int proto, int af)
 
        ifp = if_get((*mp)->m_pkthdr.ph_ifidx);
        if (ifp == NULL) {
-               m_freem(*mp);
+               m_freemp(mp);
                return IPPROTO_DONE;
        }
 
index c4bbed9..b24f378 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_input.c,v 1.311 2017/06/19 17:00:16 bluhm Exp $    */
+/*     $OpenBSD: ip_input.c,v 1.312 2017/06/19 17:58:49 bluhm Exp $    */
 /*     $NetBSD: ip_input.c,v 1.30 1996/03/16 23:53:58 christos Exp $   */
 
 /*
@@ -605,7 +605,7 @@ ip_deliver(struct mbuf **mp, int *offp, int nxt, int af)
 #ifdef IPSEC
  bad:
 #endif
-       m_freem(*mp);
+       m_freemp(mp);
 }
 
 int
index c1a839d..214d4c3 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_ipip.c,v 1.83 2017/06/11 19:59:57 bluhm Exp $ */
+/*     $OpenBSD: ip_ipip.c,v 1.84 2017/06/19 17:58:49 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -100,7 +100,7 @@ ipip_input(struct mbuf **mp, int *offp, int proto, int af)
        if (!ipip_allow && ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
                DPRINTF(("%s: dropped due to policy\n", __func__));
                ipipstat_inc(ipips_pdrops);
-               m_freem(*mp);
+               m_freemp(mp);
                return IPPROTO_DONE;
        }
 
@@ -324,8 +324,7 @@ ipip_input_gif(struct mbuf **mp, int *offp, int proto, int oaf,
 #endif
        }
  bad:
-       m_freem(*mp);
-       *mp = NULL;
+       m_freemp(mp);
        return IPPROTO_DONE;
 }
 
index a67539c..05c0ae4 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ipsec_input.c,v 1.154 2017/05/28 09:25:51 bluhm Exp $ */
+/*     $OpenBSD: ipsec_input.c,v 1.155 2017/06/19 17:58:49 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -867,8 +867,7 @@ ah6_input(struct mbuf **mp, int *offp, int proto, int af)
        if (*offp < sizeof(struct ip6_hdr)) {
                DPRINTF(("ah6_input(): bad offset\n"));
                ahstat.ahs_hdrops++;
-               m_freem(*mp);
-               *mp = NULL;
+               m_freemp(mp);
                return IPPROTO_DONE;
        } else if (*offp == sizeof(struct ip6_hdr)) {
                protoff = offsetof(struct ip6_hdr, ip6_nxt);
@@ -898,8 +897,7 @@ ah6_input(struct mbuf **mp, int *offp, int proto, int af)
                if (protoff + l != *offp) {
                        DPRINTF(("ah6_input(): bad packet header chain\n"));
                        ahstat.ahs_hdrops++;
-                       m_freem(*mp);
-                       *mp = NULL;
+                       m_freemp(mp);
                        return IPPROTO_DONE;
                }
                protoff += offsetof(struct ip6_ext, ip6e_nxt);
@@ -919,8 +917,7 @@ esp6_input(struct mbuf **mp, int *offp, int proto, int af)
        if (*offp < sizeof(struct ip6_hdr)) {
                DPRINTF(("esp6_input(): bad offset\n"));
                espstat.esps_hdrops++;
-               m_freem(*mp);
-               *mp = NULL;
+               m_freemp(mp);
                return IPPROTO_DONE;
        } else if (*offp == sizeof(struct ip6_hdr)) {
                protoff = offsetof(struct ip6_hdr, ip6_nxt);
@@ -950,8 +947,7 @@ esp6_input(struct mbuf **mp, int *offp, int proto, int af)
                if (protoff + l != *offp) {
                        DPRINTF(("esp6_input(): bad packet header chain\n"));
                        espstat.esps_hdrops++;
-                       m_freem(*mp);
-                       *mp = NULL;
+                       m_freemp(mp);
                        return IPPROTO_DONE;
                }
                protoff += offsetof(struct ip6_ext, ip6e_nxt);
@@ -972,8 +968,7 @@ ipcomp6_input(struct mbuf **mp, int *offp, int proto, int af)
        if (*offp < sizeof(struct ip6_hdr)) {
                DPRINTF(("ipcomp6_input(): bad offset\n"));
                ipcompstat.ipcomps_hdrops++;
-               m_freem(*mp);
-               *mp = NULL;
+               m_freemp(mp);
                return IPPROTO_DONE;
        } else if (*offp == sizeof(struct ip6_hdr)) {
                protoff = offsetof(struct ip6_hdr, ip6_nxt);
@@ -1002,8 +997,7 @@ ipcomp6_input(struct mbuf **mp, int *offp, int proto, int af)
                if (protoff + l != *offp) {
                        DPRINTF(("ipcomp6_input(): bad packet header chain\n"));
                        ipcompstat.ipcomps_hdrops++;
-                       m_freem(*mp);
-                       *mp = NULL;
+                       m_freemp(mp);
                        return IPPROTO_DONE;
                }
 
index b763c59..53dfe48 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip6_input.c,v 1.195 2017/06/19 17:00:16 bluhm Exp $   */
+/*     $OpenBSD: ip6_input.c,v 1.196 2017/06/19 17:58:49 bluhm Exp $   */
 /*     $KAME: ip6_input.c,v 1.188 2001/03/29 05:34:31 itojun Exp $     */
 
 /*
@@ -579,7 +579,7 @@ ip6_deliver(struct mbuf **mp, int *offp, int nxt, int af)
        }
        return;
  bad:
-       m_freem(*mp);
+       m_freemp(mp);
 }
 
 int
index 619951b..b94cbeb 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: nfs_socket.c,v 1.116 2017/05/17 08:59:05 mpi Exp $    */
+/*     $OpenBSD: nfs_socket.c,v 1.117 2017/06/19 17:58:49 bluhm Exp $  */
 /*     $NetBSD: nfs_socket.c,v 1.27 1996/04/15 20:20:00 thorpej Exp $  */
 
 /*
@@ -673,8 +673,7 @@ tryagain:
                }
 errout:
                if (error && error != EINTR && error != ERESTART) {
-                       m_freem(*mp);
-                       *mp = NULL;
+                       m_freemp(mp);
                        if (error != EPIPE)
                                log(LOG_INFO,
                                    "receive error %d from nfs server %s\n",
@@ -707,10 +706,8 @@ errout:
                } while (error == EWOULDBLOCK);
                len -= auio.uio_resid;
        }
-       if (error) {
-               m_freem(*mp);
-               *mp = NULL;
-       }
+       if (error)
+               m_freemp(mp);
        /*
         * Search for any mbufs that are not a multiple of 4 bytes long
         * or with m_data not longword aligned.
@@ -1418,7 +1415,7 @@ nfs_realign(struct mbuf **pm, int hsiz)
                        off += m->m_len;
                        m = m->m_next;
                }
-               m_freem(*pm);
+               m_freemp(pm);
                *pm = n;
        }
 }
index f6607a2..7f18404 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: mbuf.h,v 1.229 2017/05/30 12:09:27 friehm Exp $       */
+/*     $OpenBSD: mbuf.h,v 1.230 2017/06/19 17:58:49 bluhm Exp $        */
 /*     $NetBSD: mbuf.h,v 1.19 1996/02/09 18:25:14 christos Exp $       */
 
 /*
@@ -463,6 +463,15 @@ int        m_apply(struct mbuf *, int, int,
 struct mbuf *m_dup_pkt(struct mbuf *, unsigned int, int);
 int    m_dup_pkthdr(struct mbuf *, struct mbuf *, int);
 
+static inline struct mbuf *
+m_freemp(struct mbuf **mp)
+{
+       struct mbuf *m = *mp;
+
+       *mp = NULL;
+       return m_freem(m);
+}
+
 /* Packet tag routines */
 struct m_tag *m_tag_get(int, int, int);
 void   m_tag_prepend(struct mbuf *, struct m_tag *);