Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Alexander Lobakin <alexandr.lobakin@intel.com>
To:     Shenwei Wang <shenwei.wang@nxp.com>
Cc:     Alexander Lobakin <alexandr.lobakin@intel.com>,
        Paolo Abeni <pabeni@redhat.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        John Fastabend <john.fastabend@gmail.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "imx@lists.linux.dev" <imx@lists.linux.dev>
Subject: Re: [PATCH v2 RESEND 1/1] net: fec: add xdp and page pool statistics
Date:   Thu, 10 Nov 2022 17:43:21 +0100
Message-Id: <20221110164321.3534977-1-alexandr.lobakin@intel.com>
In-Reply-To: <PAXPR04MB91853A6A1DDDBB06F33C975E89019@PAXPR04MB9185.eurprd04.prod.outlook.com>
References: <20221109023147.242904-1-shenwei.wang@nxp.com> <4349bc93a5f2130a95305287141fde369245f921.camel@redhat.com> <PAXPR04MB91853A6A1DDDBB06F33C975E89019@PAXPR04MB9185.eurprd04.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Shenwei Wang <shenwei.wang@nxp.com>
Date: Thu, 10 Nov 2022 13:29:56 +0000

> > -----Original Message-----
> > From: Paolo Abeni <pabeni@redhat.com>
> > Sent: Thursday, November 10, 2022 5:54 AM
> > To: Shenwei Wang <shenwei.wang@nxp.com>; David S. Miller
> > <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> > Kicinski <kuba@kernel.org>
> > >       case ETH_SS_STATS:
> > > -             for (i = 0; i < ARRAY_SIZE(fec_stats); i++)
> > > -                     memcpy(data + i * ETH_GSTRING_LEN,
> > > -                             fec_stats[i].name, ETH_GSTRING_LEN);
> > > +             for (i = 0; i < ARRAY_SIZE(fec_stats); i++) {
> > > +                     memcpy(data, fec_stats[i].name, ETH_GSTRING_LEN);
> > > +                     data += ETH_GSTRING_LEN;
> > > +             }
> > > +             for (i = 0; i < ARRAY_SIZE(fec_xdp_stat_strs); i++) {
> > > +                     memcpy(data, fec_xdp_stat_strs[i], ETH_GSTRING_LEN);
> > > +                     data += ETH_GSTRING_LEN;
> >
> > The above triggers a warning:
> >
> > In function 'fortify_memcpy_chk',
> >     inlined from 'fec_enet_get_strings'
> > at ../drivers/net/ethernet/freescale/fec_main.c:2788:4:
> > ../include/linux/fortify-string.h:413:25: warning: call to '__read_overflow2_field'
> > declared with attribute warning: detected read beyond size of field (2nd
> > parameter); maybe use struct_group()? [-Wattribute-warning]
> >   413 |                         __read_overflow2_field(q_size_field, size);
> >       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > I think you can address it changing fec_xdp_stat_strs definition to:
> >
> > static const char fec_xdp_stat_strs[XDP_STATS_TOTAL][ETH_GSTRING_LEN] =
> 
> That does a problem. How about just change the memcpy to strncpy?

Don't use a static char array, it would consume more memory than the
current code. Just replace memcpy()s with strscpy().

Why u32 for the stats tho? It will overflow sooner or later. "To
keep it simple and compatible" you can use u64_stats API :)

> 
> Regards,
> Shenwei
> 
> > { // ...
> >
> > Cheers,
> >
> > Paolo

Thanks,
Olek