On Wed, Jul 29, 2015 at 06:04:02PM +0000, Ermal Luçi wrote:
> Author: eri
> Date: Wed Jul 29 18:04:01 2015
> New Revision: 286028
> URL: https://svnweb.freebsd.org/changeset/base/286028
>
> Log:
> ip_output normalization and fixes
>
> ip_output has a big chunk of code used to handle special cases with pfil
> consumers which also forces a reloop on it.
> Gather all this code together to make it readable and properly handle the
> reloop cases.
>
> Some of the issues identified:
>
> M_IP_NEXTHOP is not handled properly in existing code.
> route reference leaking is possible with in FIB number change
> route flags checking is not consistent in the function
>
> Differential Revision: https://reviews.freebsd.org/D3022
> Reviewed by: gnn
> Approved by: gnn(mentor)
> MFC after: 4 weeks
>
> Modified:
> head/sys/netinet/ip_output.c
>
> Modified: head/sys/netinet/ip_output.c
> ==============================================================================
> --- head/sys/netinet/ip_output.c Wed Jul 29 17:59:13 2015
> (r286027)
> +++ head/sys/netinet/ip_output.c Wed Jul 29 18:04:01 2015
> (r286028)
> @@ -106,6 +106,94 @@ static void ip_mloopback
> extern int in_mcast_loop;
> extern struct protosw inetsw[];
>
> +static inline int
> +ip_output_pfil(struct mbuf *m, struct ifnet *ifp, struct inpcb *inp,
> + struct sockaddr_in *dst, int *fibnum, int *error)
> +{
> + struct m_tag *fwd_tag = NULL;
> + struct in_addr odst;
> + struct ip *ip;
> +
> + ip = mtod(m, struct ip *);
> +
> + /* Run through list of hooks for output packets. */
> + odst.s_addr = ip->ip_dst.s_addr;
> + *error = pfil_run_hooks(&V_inet_pfil_hook, &m, ifp, PFIL_OUT, inp);
> + if ((*error) != 0 || m == NULL)
> + return 1; /* Finished */
> +
This can result in a use-after-free in ip_output() if a pfil hook
consumes the first mbuf in the chain. This happens for example when ipfw
nat is in use: m_megapullup() copies the input packet into a single
cluster, which is returned above. However, ip_output() will continue to
reference the original mbuf chain.
The patch below fixes the problem for me.
Thanks,
-Mark
diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
index 0790777..086a8c9 100644
--- a/sys/netinet/ip_output.c
+++ b/sys/netinet/ip_output.c
@@ -107,18 +107,21 @@ extern int in_mcast_loop;
extern struct protosw inetsw[];
static inline int
-ip_output_pfil(struct mbuf *m, struct ifnet *ifp, struct inpcb *inp,
- struct sockaddr_in *dst, int *fibnum, int *error)
+ip_output_pfil(struct mbuf **mp, struct ifnet *ifp, struct inpcb *inp,
+ struct sockaddr_in *dst, int *fibnum, int *error)
{
struct m_tag *fwd_tag = NULL;
+ struct mbuf *m;
struct in_addr odst;
struct ip *ip;
+ m = *mp;
ip = mtod(m, struct ip *);
/* Run through list of hooks for output packets. */
odst.s_addr = ip->ip_dst.s_addr;
- *error = pfil_run_hooks(&V_inet_pfil_hook, &m, ifp, PFIL_OUT, inp);
+ *error = pfil_run_hooks(&V_inet_pfil_hook, mp, ifp, PFIL_OUT, inp);
+ m = *mp;
if ((*error) != 0 || m == NULL)
return 1; /* Finished */
@@ -552,7 +555,7 @@ sendit:
/* Jump over all PFIL processing if hooks are not active. */
if (PFIL_HOOKED(&V_inet_pfil_hook)) {
- switch (ip_output_pfil(m, ifp, inp, dst, &fibnum, &error)) {
+ switch (ip_output_pfil(&m, ifp, inp, dst, &fibnum, &error)) {
case 1: /* Finished */
goto done;
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
