svn commit: r286028 - head/sys/netinet
Ermal Luçi
eri at freebsd.org
Mon Aug 3 06:52:43 UTC 2015
On Sun, Aug 2, 2015 at 8:40 PM, Mark Johnston <markj at freebsd.org> wrote:
> 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.
>
>
Good catch just push it in.
Ok 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;
>
>
--
Ermal
More information about the svn-src-all
mailing list