Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/net/npf - Fix double-free case on ICMP return case.



details:   https://anonhg.NetBSD.org/src/rev/996265a197ae
branches:  trunk
changeset: 779140:996265a197ae
user:      rmind <rmind%NetBSD.org@localhost>
date:      Sun May 06 02:45:25 2012 +0000

description:
- Fix double-free case on ICMP return case.
- npf_pfil_register: handle kernels without INET6 option correctly.
- Reduce some #ifdefs.

diffstat:

 sys/net/npf/npf_handler.c |  50 +++++++++++++++++++++++++++------------------
 sys/net/npf/npf_impl.h    |   4 +-
 sys/net/npf/npf_sendpkt.c |  51 +++++++++++++++++++++++-----------------------
 3 files changed, 57 insertions(+), 48 deletions(-)

diffs (244 lines):

diff -r b2e9843852a7 -r 996265a197ae sys/net/npf/npf_handler.c
--- a/sys/net/npf/npf_handler.c Sat May 05 22:07:57 2012 +0000
+++ b/sys/net/npf/npf_handler.c Sun May 06 02:45:25 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npf_handler.c,v 1.15 2012/03/11 18:27:59 rmind Exp $   */
+/*     $NetBSD: npf_handler.c,v 1.16 2012/05/06 02:45:25 rmind Exp $   */
 
 /*-
  * Copyright (c) 2009-2012 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_handler.c,v 1.15 2012/03/11 18:27:59 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_handler.c,v 1.16 2012/05/06 02:45:25 rmind Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -236,17 +236,20 @@
         * Depending on the flags and protocol, return TCP reset (RST) or
         * ICMP destination unreachable.
         */
-       if (retfl) {
-               npf_return_block(&npc, nbuf, retfl);
+       if (retfl && npf_return_block(&npc, nbuf, retfl)) {
+               *mp = NULL;
        }
+
        if (error) {
                npf_stats_inc(NPF_STAT_ERROR);
        } else {
                error = ENETUNREACH;
        }
-       m_freem(*mp);
-       *mp = NULL;
 
+       if (*mp) {
+               m_freem(*mp);
+               *mp = NULL;
+       }
        return error;
 }
 
@@ -271,7 +274,7 @@
        npf_ph_if = pfil_head_get(PFIL_TYPE_IFNET, 0);
        npf_ph_inet = pfil_head_get(PFIL_TYPE_AF, AF_INET);
        npf_ph_inet6 = pfil_head_get(PFIL_TYPE_AF, AF_INET6);
-       if (npf_ph_if == NULL || npf_ph_inet == NULL || npf_ph_inet6 == NULL) {
+       if (!npf_ph_if || (!npf_ph_inet && !npf_ph_inet6)) {
                npf_ph_if = NULL;
                error = ENOENT;
                goto fail;
@@ -283,13 +286,16 @@
        KASSERT(error == 0);
 
        /* Packet IN/OUT handler on all interfaces and IP layer. */
-       error = pfil_add_hook(npf_packet_handler, NULL,
-           PFIL_WAITOK | PFIL_ALL, npf_ph_inet);
-       KASSERT(error == 0);
-
-       error = pfil_add_hook(npf_packet_handler, NULL,
-           PFIL_WAITOK | PFIL_ALL, npf_ph_inet6);
-       KASSERT(error == 0);
+       if (npf_ph_inet) {
+               error = pfil_add_hook(npf_packet_handler, NULL,
+                   PFIL_WAITOK | PFIL_ALL, npf_ph_inet);
+               KASSERT(error == 0);
+       }
+       if (npf_ph_inet6) {
+               error = pfil_add_hook(npf_packet_handler, NULL,
+                   PFIL_WAITOK | PFIL_ALL, npf_ph_inet6);
+               KASSERT(error == 0);
+       }
 fail:
        KERNEL_UNLOCK_ONE(NULL);
        mutex_exit(softnet_lock);
@@ -308,15 +314,19 @@
        KERNEL_LOCK(1, NULL);
 
        if (npf_ph_if) {
-               (void)pfil_remove_hook(npf_packet_handler, NULL,
-                   PFIL_ALL, npf_ph_inet6);
+               (void)pfil_remove_hook(npf_ifhook, NULL,
+                   PFIL_IFADDR | PFIL_IFNET, npf_ph_if);
+       }
+       if (npf_ph_inet) {
                (void)pfil_remove_hook(npf_packet_handler, NULL,
                    PFIL_ALL, npf_ph_inet);
-               (void)pfil_remove_hook(npf_ifhook, NULL,
-                   PFIL_IFADDR | PFIL_IFNET, npf_ph_if);
+       }
+       if (npf_ph_inet6) {
+               (void)pfil_remove_hook(npf_packet_handler, NULL,
+                   PFIL_ALL, npf_ph_inet6);
+       }
 
-               npf_ph_if = NULL;
-       }
+       npf_ph_if = NULL;
 
        KERNEL_UNLOCK_ONE(NULL);
        mutex_exit(softnet_lock);
diff -r b2e9843852a7 -r 996265a197ae sys/net/npf/npf_impl.h
--- a/sys/net/npf/npf_impl.h    Sat May 05 22:07:57 2012 +0000
+++ b/sys/net/npf/npf_impl.h    Sun May 06 02:45:25 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npf_impl.h,v 1.13 2012/04/14 19:01:21 rmind Exp $      */
+/*     $NetBSD: npf_impl.h,v 1.14 2012/05/06 02:45:25 rmind Exp $      */
 
 /*-
  * Copyright (c) 2009-2012 The NetBSD Foundation, Inc.
@@ -173,7 +173,7 @@
 bool           npf_fetch_tcpopts(const npf_cache_t *, nbuf_t *,
                    uint16_t *, int *);
 bool           npf_normalize(npf_cache_t *, nbuf_t *, bool, bool, u_int, u_int);
-void           npf_return_block(npf_cache_t *, nbuf_t *, const int);
+bool           npf_return_block(npf_cache_t *, nbuf_t *, const int);
 
 /* Complex instructions. */
 int            npf_match_ether(nbuf_t *, int, int, uint16_t, uint32_t *);
diff -r b2e9843852a7 -r 996265a197ae sys/net/npf/npf_sendpkt.c
--- a/sys/net/npf/npf_sendpkt.c Sat May 05 22:07:57 2012 +0000
+++ b/sys/net/npf/npf_sendpkt.c Sun May 06 02:45:25 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npf_sendpkt.c,v 1.9 2012/02/20 00:18:20 rmind Exp $    */
+/*     $NetBSD: npf_sendpkt.c,v 1.10 2012/05/06 02:45:25 rmind Exp $   */
 
 /*-
  * Copyright (c) 2010-2011 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_sendpkt.c,v 1.9 2012/02/20 00:18:20 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_sendpkt.c,v 1.10 2012/05/06 02:45:25 rmind Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -54,6 +54,12 @@
 
 #define        DEFAULT_IP_TTL          (ip_defttl)
 
+#ifndef INET6
+#define        in6_cksum(...)          0
+#define        ip6_output(...)         0
+#define        icmp6_error(m, ...)     m_freem(m)
+#endif
+
 /*
  * npf_return_tcp: return a TCP reset (RST) packet.
  */
@@ -81,9 +87,10 @@
        /* Create and setup a network buffer. */
        if (npf_iscached(npc, NPC_IP4)) {
                len = sizeof(struct ip) + sizeof(struct tcphdr);
+       } else if (npf_iscached(npc, NPC_IP6)) {
+               len = sizeof(struct ip6_hdr) + sizeof(struct tcphdr);
        } else {
-               KASSERT(npf_iscached(npc, NPC_IP6));
-               len = sizeof(struct ip6_hdr) + sizeof(struct tcphdr);
+               return EINVAL;
        }
 
        m = m_gethdr(M_DONTWAIT, MT_HEADER);
@@ -149,24 +156,15 @@
                ip->ip_ttl = DEFAULT_IP_TTL;
        } else {
                KASSERT(npf_iscached(npc, NPC_IP6));
-#ifdef INET6
                th->th_sum = in6_cksum(m, IPPROTO_TCP, sizeof(struct ip6_hdr),
                    sizeof(struct tcphdr));
-#else
-               KASSERT(false);
-#endif
        }
 
        /* Pass to IP layer. */
        if (npf_iscached(npc, NPC_IP4)) {
                return ip_output(m, NULL, NULL, IP_FORWARDING, NULL, NULL);
-       } else {
-#ifdef INET6
-               return ip6_output(m, NULL, NULL, IPV6_FORWARDING, NULL, NULL, NULL);
-#else
-               return 0;
-#endif
        }
+       return ip6_output(m, NULL, NULL, IPV6_FORWARDING, NULL, NULL, NULL);
 }
 
 /*
@@ -179,40 +177,41 @@
 
        if (npf_iscached(npc, NPC_IP4)) {
                icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_ADMIN_PROHIBIT, 0, 0);
-       } else {
-               KASSERT(npf_iscached(npc, NPC_IP6));
-#ifdef INET6
+               return 0;
+       } else if (npf_iscached(npc, NPC_IP6)) {
                icmp6_error(m, ICMP6_DST_UNREACH, ICMP6_DST_UNREACH_ADMIN, 0);
-#endif
+               return 0;
        }
-       return 0;
+       return EINVAL;
 }
 
 /*
  * npf_return_block: return TCP reset or ICMP host unreachable packet.
- * TODO: user should be able to specify exact ICMP error codes in config
+ *
+ * => Returns true if the buffer was consumed (freed) and false otherwise.
  */
-void
+bool
 npf_return_block(npf_cache_t *npc, nbuf_t *nbuf, const int retfl)
 {
        void *n_ptr = nbuf_dataptr(nbuf);
 
        if (!npf_iscached(npc, NPC_IP46) && !npf_fetch_ip(npc, nbuf, n_ptr)) {
-               return;
+               return false;
        }
        switch (npf_cache_ipproto(npc)) {
        case IPPROTO_TCP:
                if (retfl & NPF_RULE_RETRST) {
                        if (!npf_fetch_tcp(npc, nbuf, n_ptr)) {
-                               return;
+                               return false;
                        }
                        (void)npf_return_tcp(npc);
                }
                break;
        case IPPROTO_UDP:
-               if (retfl & NPF_RULE_RETICMP) {
-                       (void)npf_return_icmp(npc, nbuf);
-               }
+               if (retfl & NPF_RULE_RETICMP)
+                       if (npf_return_icmp(npc, nbuf) == 0)
+                               return true;
                break;
        }
+       return false;
 }



Home | Main Index | Thread Index | Old Index