netdev - RE: [PATCH] Revert "dpaa_eth: add XDP

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] [day] [month] [year] [list]

Date:   Thu, 18 Feb 2021 13:43:03 +0000
From:   Camelia Alexandra Groza <camelia.groza@....com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>,
        Sascha Hauer <s.hauer@...gutronix.de>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Madalin Bucur <madalin.bucur@....com>,
        Jakub Kicinski <kuba@...nel.org>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Ioana Ciocoi Radulescu <ruxandra.radulescu@....com>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>
Subject: RE: [PATCH] Revert "dpaa_eth: add XDP_REDIRECT support"

> -----Original Message-----
> From: Jesper Dangaard Brouer <brouer@...hat.com>
> Sent: Thursday, February 18, 2021 13:43
> To: Sascha Hauer <s.hauer@...gutronix.de>
> Cc: brouer@...hat.com; netdev@...r.kernel.org; Camelia Alexandra Groza
> <camelia.groza@....com>; Madalin Bucur <madalin.bucur@....com>;
> Jakub Kicinski <kuba@...nel.org>; kernel@...gutronix.de; Ioana Ciornei
> <ioana.ciornei@....com>; Ioana Ciocoi Radulescu
> <ruxandra.radulescu@....com>; Ilias Apalodimas
> <ilias.apalodimas@...aro.org>
> Subject: Re: [PATCH] Revert "dpaa_eth: add XDP_REDIRECT support"
> 
> On Wed, 17 Feb 2021 16:17:58 +0100
> Sascha Hauer <s.hauer@...gutronix.de> wrote:
> 
> > This reverts commit a1e031ffb422bb89df9ad9c018420d0deff7f2e3.
> >
> > This commit introduces a:
> >
> > 	np = container_of(&portal, struct dpaa_napi_portal, p);
> >
> > Using container_of() on the address of a pointer doesn't make sense as
> > the pointer is not embedded into the desired struct.
> >
> > KASAN complains about it like this:
> >
> > [   17.703277]
> ==========================================================
> ========
> > [   17.710517] BUG: KASAN: stack-out-of-bounds in
> rx_default_dqrr+0x994/0x14a0
> > [   17.717504] Read of size 4 at addr ffff0009336495fc by task systemd/1
> > [   17.723955]
> > [   17.725447] CPU: 0 PID: 1 Comm: systemd Not tainted 5.11.0-rc6-
> 20210204-2-00033-gfd6caa9c7514-dirty #63
> > [   17.734857] Hardware name: TQ TQMLS1046A SoM
> > [   17.742176] Call trace:
> > [   17.744621]  dump_backtrace+0x0/0x2e8
> > [   17.748298]  show_stack+0x1c/0x68
> > [   17.751622]  dump_stack+0xe8/0x14c
> > [   17.755033]  print_address_description.constprop.0+0x68/0x304
> > [   17.760794]  kasan_report+0x1d4/0x238
> > [   17.764466]  __asan_load4+0x88/0xc0
> > [   17.767962]  rx_default_dqrr+0x994/0x14a0
> > [   17.771980]  qman_p_poll_dqrr+0x254/0x278
> > [   17.776000]  dpaa_eth_poll+0x4c/0xe0
> > ...
> >
> > It's not clear to me how a the struct dpaa_napi_portal * should be
> > derived from the struct qman_portal *, so revert the patch for now.
> 
> Can we please get a response from NXP people?
> 
> Are you saying XDP_REDIRECT feature is completely broken on dpaa driver?
> 
> (I only have access to dpaa2 hardware)

Hi,

Thank you for pointing out this issue. The correct fix is the following. I'll send the patch for review.

As for the impact, the xdp_do_flush() call might be skipped due to this bug. This doesn't impact dpaa2 since the drivers are different.

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 6faa20bed488..9905caeaeee3 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2672,7 +2672,6 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,
        u32 hash;
        u64 ns;

-       np = container_of(&portal, struct dpaa_napi_portal, p);
        dpaa_fq = container_of(fq, struct dpaa_fq, fq_base);
        fd_status = be32_to_cpu(fd->status);
        fd_format = qm_fd_get_format(fd);
@@ -2687,6 +2686,7 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,

        percpu_priv = this_cpu_ptr(priv->percpu_priv);
        percpu_stats = &percpu_priv->stats;
+       np = &percpu_priv->np;

        if (unlikely(dpaa_eth_napi_schedule(percpu_priv, portal, sched_napi)))
                return qman_cb_dqrr_stop;

Regards,
Camelia