netdev - Re: [PATCH net-next] stmmac: align RX buffers

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]

Date:   Fri, 20 Aug 2021 17:26:21 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Matteo Croce <mcroce@...ux.microsoft.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        Thierry Reding <thierry.reding@...il.com>,
        netdev@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Drew Fustini <drew@...gleboard.org>,
        Emil Renner Berthing <kernel@...il.dk>,
        Jon Hunter <jonathanh@...dia.com>,
        Will Deacon <will@...nel.org>
Subject: Re: [PATCH net-next] stmmac: align RX buffers

On Fri, 20 Aug 2021 11:37:03 +0100,
Matteo Croce <mcroce@...ux.microsoft.com> wrote:
> 
> On Thu, Aug 19, 2021 at 6:29 PM Marc Zyngier <maz@...nel.org> wrote:

[...]

> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> > index fcdb1d20389b..244aa6579ef4 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> > @@ -341,7 +341,7 @@ static inline unsigned int stmmac_rx_offset(struct stmmac_priv *priv)
> >         if (stmmac_xdp_is_enabled(priv))
> >                 return XDP_PACKET_HEADROOM + NET_IP_ALIGN;
> >
> > -       return NET_SKB_PAD + NET_IP_ALIGN;
> > +       return 8 + NET_IP_ALIGN;
> >  }
> >
> >  void stmmac_disable_rx_queue(struct stmmac_priv *priv, u32 queue);
> >
> > I don't see the system corrupting packets anymore. Is that exactly
> > what you had in mind? This really seems to point to a basic buffer
> > overflow.

[...]

> Sorry, I meant something like:
> 
> -       return NET_SKB_PAD + NET_IP_ALIGN;
> +       return 8;
> 
> I had some hardware which DMA fails if the receive buffer was not word
> aligned, but this seems not the case, as 8 + NET_IP_ALIGN = 10, and
> it's not aligned too.

No error in that case either, as expected. Given that NET_SKB_PAD is
likely to expand to 64, it is likely a DMA buffer overflow which
probably only triggers for large-ish packets.

Now, we're almost at -rc7, and we don't have a solution in sight.

Can we please revert this until we have an understanding of what is
happening? I'll hopefully have more cycles to work on the issue once
5.14 is out, and hopefully the maintainers of this driver can chime in
(they have been pretty quiet so far).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.