Hi,
ip_insertoptions() may prepend a mbuf. In this case "goto bad" has
to free the new chain. Currently we leak the new mbuf in front of
the old chain. NetBSD has fixed this bug here:
----------------------------
revision 1.33
date: 1996-10-11 18:19:08 +0000; author: is; state: Exp; lines: +2 -2;
Fix a mbuf leak in ip_output().
Scenario: If ip_insertoptions() prepends a new mbuf to the chain, the
bad: label's m_freem(m0) still would free only the original mbuf chain
if the transmission failed for, e.g., no route to host; resulting in
one lost mbuf per failed packet. (The original posting included a
demonstration program).
Original report of this bug was by jin...@isl.rdc.toshiba.co.jp
(JINMEI Tatuya) on comp.bugs.4bsd.
----------------------------
Free m instead of m0 in the bad case. This allows to simplify a
bunch of goto done.
ok?
bluhm
Index: netinet/ip_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.365
diff -u -p -r1.365 ip_output.c
--- netinet/ip_output.c 10 Feb 2021 18:28:06 -0000 1.365
+++ netinet/ip_output.c 22 Feb 2021 15:48:38 -0000
@@ -111,9 +111,6 @@ ip_output(struct mbuf *m0, struct mbuf *
#if NPF > 0
u_int orig_rtableid;
#endif
-#ifdef MROUTING
- int rv;
-#endif
NET_ASSERT_LOCKED();
@@ -250,8 +247,7 @@ reroute:
/* Should silently drop packet */
if (error == -EINVAL)
error = 0;
- m_freem(m);
- goto done;
+ goto bad;
}
if (tdb != NULL) {
/*
@@ -348,13 +344,13 @@ reroute:
*/
if (ipmforwarding && ip_mrouter[ifp->if_rdomain] &&
(flags & IP_FORWARDING) == 0) {
+ int rv;
+
KERNEL_LOCK();
rv = ip_mforward(m, ifp);
KERNEL_UNLOCK();
- if (rv != 0) {
- m_freem(m);
- goto done;
- }
+ if (rv != 0)
+ goto bad;
}
}
#endif
@@ -366,10 +362,8 @@ reroute:
* loop back a copy if this host actually belongs to the
* destination group on the loopback interface.
*/
- if (ip->ip_ttl == 0 || (ifp->if_flags & IFF_LOOPBACK) != 0) {
- m_freem(m);
- goto done;
- }
+ if (ip->ip_ttl == 0 || (ifp->if_flags & IFF_LOOPBACK) != 0)
+ goto bad;
goto sendit;
}
@@ -427,8 +421,7 @@ sendit:
if (pf_test(AF_INET, (flags & IP_FORWARDING) ? PF_FWD : PF_OUT,
ifp, &m) != PF_PASS) {
error = EACCES;
- m_freem(m);
- goto done;
+ goto bad;
}
if (m == NULL)
goto done;
@@ -453,8 +446,7 @@ sendit:
if (ipsec_in_use && (flags & IP_FORWARDING) && (ipforwarding == 2) &&
(m_tag_find(m, PACKET_TAG_IPSEC_IN_DONE, NULL) == NULL)) {
error = EHOSTUNREACH;
- m_freem(m);
- goto done;
+ goto bad;
}
#endif
@@ -534,7 +526,7 @@ done:
if_put(ifp);
return (error);
bad:
- m_freem(m0);
+ m_freem(m);
goto done;
}
