Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190409065612.32652-1-rdong.ge@gmail.com> <20190422083339.ptkxqb66pombgy5g@salvia>
In-Reply-To: <20190422083339.ptkxqb66pombgy5g@salvia>
From:   Rundong Ge <rdong.ge@gmail.com>
Date:   Mon, 22 Apr 2019 17:16:07 +0800
Message-ID: <CAN1Lvyo1CVopsB7iV=e-dAN9JDFg-SmQ8g6H6RrgD7qACAd4hQ@mail.gmail.com>
Subject: Re: [PATCH] netfilter: fix dangling pointer access of fake_rtable
To:     Pablo Neira Ayuso <pablo@netfilter.org>
Cc:     kadlec@blackhole.kfki.hu, fw@strlen.de,
        Roopa Prabhu <roopa@cumulusnetworks.com>, davem@davemloft.net,
        netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

br_nf_pre_routing will call the NF_INET_PRE_ROUTING hooks, at this
time both entry->state.in and entry->state.out are not bridge device.

NF_HOOK(NFPROTO_IPV4, NF_INET_PRE_ROUTING, state->net, state->sk, skb,
skb->dev, NULL,
br_nf_pre_routing_finish);

Pablo Neira Ayuso <pablo@netfilter.org> =E4=BA=8E2019=E5=B9=B44=E6=9C=8822=
=E6=97=A5=E5=91=A8=E4=B8=80 =E4=B8=8B=E5=8D=884:34=E5=86=99=E9=81=93=EF=BC=
=9A
>
> On Tue, Apr 09, 2019 at 06:56:12AM +0000, Rundong Ge wrote:
> [...]
> > diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_=
queue.c
> > index 0dcc359..57eb02d 100644
> > --- a/net/netfilter/nfnetlink_queue.c
> > +++ b/net/netfilter/nfnetlink_queue.c
> > @@ -905,13 +905,25 @@ static void free_entry(struct nf_queue_entry *ent=
ry)
> >  dev_cmp(struct nf_queue_entry *entry, unsigned long ifindex)
> >  {
> >  #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
> > -     int physinif, physoutif;
> > +     struct net_device *physindev, *physoutdev;
> > +     struct net_bridge_port *port;
> >
> > -     physinif =3D nf_bridge_get_physinif(entry->skb);
> > -     physoutif =3D nf_bridge_get_physoutif(entry->skb);
> > -
> > -     if (physinif =3D=3D ifindex || physoutif =3D=3D ifindex)
> > -             return 1;
> > +     physindev =3D nf_bridge_get_physindev(entry->skb);
> > +     physoutdev =3D nf_bridge_get_physoutdev(entry->skb);
> > +     if (physindev) {
> > +             if (physindev->ifindex =3D=3D ifindex)
> > +                     return 1;
> > +             port =3D br_port_get_rcu(physindev);
> > +             if (port && port->br->dev->ifindex =3D=3D ifindex)
> > +                     return 1;
> > +     }
> > +     if (physoutdev) {
> > +             if (physoutdev->ifindex =3D=3D ifindex)
> > +                     return 1;
> > +             port =3D br_port_get_rcu(physoutdev);
> > +             if (port && port->br->dev->ifindex =3D=3D ifindex)
> > +                     return 1;
> > +     }
>
> Either entry->state.in and entry->state.out point to the bridge
> device, after this #endif.
>
> >  #endif
> >       if (entry->state.in)
> >               if (entry->state.in->ifindex =3D=3D ifindex)
> > --
> > 1.8.3.1
> >