netdev - Re: [PATCH net] macsec: avoid use-after-free in macsec_handle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iLcZit_0Og9MbW0x0bQ=-6pz18UpRN6RYOY12Czui1eMQ@mail.gmail.com>
Date:   Wed, 7 Oct 2020 16:31:05 +0200
From:   Eric Dumazet <edumazet@...gle.com>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net] macsec: avoid use-after-free in macsec_handle_frame()

On Wed, Oct 7, 2020 at 4:09 PM Paolo Abeni <pabeni@...hat.com> wrote:
>
> Hi,
>
> On Wed, 2020-10-07 at 01:42 -0700, Eric Dumazet wrote:
> > @@ -1232,9 +1233,10 @@ static rx_handler_result_t macsec_handle_frame(struct sk_buff **pskb)
> >       macsec_rxsc_put(rx_sc);
> >
> >       skb_orphan(skb);
> > +     len = skb->len;
> >       ret = gro_cells_receive(&macsec->gro_cells, skb);
> >       if (ret == NET_RX_SUCCESS)
> > -             count_rx(dev, skb->len);
> > +             count_rx(dev, len);
> >       else
> >               macsec->secy.netdev->stats.rx_dropped++;
>
> I'm sorry I'm low on coffee, but I can't see the race?!? here we are in
> a BH section, and the above code dereference the skb only if it's has
> been enqueued into the gro_cells napi. It could be dequeued/dropped
> only after we leave this section ?!?

We should think of this as an alias for napi_gro_receive(), and not
make any assumptions.
Semantically the skb has been given to another layer.

netif_rx() can absolutely queue the skb to another cpu backlog (RPS,
RFS...), and the other cpu might have consumed the skb right away.

We can avoid these subtle bugs by respecting simple rules ;)

>
> Thanks,
>
> Paolo
>