From: Lorenzo Bianconi <lorenzo@kernel.org>
To: Sven Auhagen <sven.auhagen@voleatech.de>
Cc: netdev@vger.kernel.org, thomas.petazzoni@bootlin.com,
brouer@redhat.com, ilias.apalodimas@linaro.org,
mcroce@microsoft.com, leon@kernel.org, kuba@kernel.org
Subject: Re: [PATCH v5] net: mvneta: fix calls to page_pool_get_stats
Date: Sat, 11 Nov 2023 10:49:07 +0100 [thread overview]
Message-ID: <ZU9OEwz7GoHbBE1m@lore-desk> (raw)
In-Reply-To: <ydvyjmjgpppf2hd7rzb6iu2hi6aiuxoa7sq5qnorknwk5txuca@7fgznkjwynsf>
[-- Attachment #1: Type: text/plain, Size: 7531 bytes --]
> Calling page_pool_get_stats in the mvneta driver without checks
> leads to kernel crashes.
> First the page pool is only available if the bm is not used.
> The page pool is also not allocated when the port is stopped.
> It can also be not allocated in case of errors.
>
> The current implementation leads to the following crash calling
> ethstats on a port that is down or when calling it at the wrong moment:
>
> ble to handle kernel NULL pointer dereference at virtual address 00000070
> [00000070] *pgd=00000000
> Internal error: Oops: 5 [#1] SMP ARM
> Hardware name: Marvell Armada 380/385 (Device Tree)
> PC is at page_pool_get_stats+0x18/0x1cc
> LR is at mvneta_ethtool_get_stats+0xa0/0xe0 [mvneta]
> pc : [<c0b413cc>] lr : [<bf0a98d8>] psr: a0000013
> sp : f1439d48 ip : f1439dc0 fp : 0000001d
> r10: 00000100 r9 : c4816b80 r8 : f0d75150
> r7 : bf0b400c r6 : c238f000 r5 : 00000000 r4 : f1439d68
> r3 : c2091040 r2 : ffffffd8 r1 : f1439d68 r0 : 00000000
> Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 10c5387d Table: 066b004a DAC: 00000051
> Register r0 information: NULL pointer
> Register r1 information: 2-page vmalloc region starting at 0xf1438000 allocated at kernel_clone+0x9c/0x390
> Register r2 information: non-paged memory
> Register r3 information: slab kmalloc-2k start c2091000 pointer offset 64 size 2048
> Register r4 information: 2-page vmalloc region starting at 0xf1438000 allocated at kernel_clone+0x9c/0x390
> Register r5 information: NULL pointer
> Register r6 information: slab kmalloc-cg-4k start c238f000 pointer offset 0 size 4096
> Register r7 information: 15-page vmalloc region starting at 0xbf0a8000 allocated at load_module+0xa30/0x219c
> Register r8 information: 1-page vmalloc region starting at 0xf0d75000 allocated at ethtool_get_stats+0x138/0x208
> Register r9 information: slab task_struct start c4816b80 pointer offset 0
> Register r10 information: non-paged memory
> Register r11 information: non-paged memory
> Register r12 information: 2-page vmalloc region starting at 0xf1438000 allocated at kernel_clone+0x9c/0x390
> Process snmpd (pid: 733, stack limit = 0x38de3a88)
> Stack: (0xf1439d48 to 0xf143a000)
> 9d40: 000000c0 00000001 c238f000 bf0b400c f0d75150 c4816b80
> 9d60: 00000100 bf0a98d8 00000000 00000000 00000000 00000000 00000000 00000000
> 9d80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 9da0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 9dc0: 00000dc0 5335509c 00000035 c238f000 bf0b2214 01067f50 f0d75000 c0b9b9c8
> 9de0: 0000001d 00000035 c2212094 5335509c c4816b80 c238f000 c5ad6e00 01067f50
> 9e00: c1b0be80 c4816b80 00014813 c0b9d7f0 00000000 00000000 0000001d 0000001d
> 9e20: 00000000 00001200 00000000 00000000 c216ed90 c73943b8 00000000 00000000
> 9e40: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 9e60: 00000000 c0ad9034 00000000 00000000 00000000 00000000 00000000 00000000
> 9e80: 00000000 00000000 00000000 5335509c c1b0be80 f1439ee4 00008946 c1b0be80
> 9ea0: 01067f50 f1439ee3 00000000 00000046 b6d77ae0 c0b383f0 00008946 becc83e8
> 9ec0: c1b0be80 00000051 0000000b c68ca480 c7172d00 c0ad8ff0 f1439ee3 cf600e40
> 9ee0: 01600e40 32687465 00000000 00000000 00000000 01067f50 00000000 00000000
> 9f00: 00000000 5335509c 00008946 00008946 00000000 c68ca480 becc83e8 c05e2de0
> 9f20: f1439fb0 c03002f0 00000006 5ac3c35a c4816b80 00000006 b6d77ae0 c030caf0
> 9f40: c4817350 00000014 f1439e1c 0000000c 00000000 00000051 01000000 00000014
> 9f60: 00003fec f1439edc 00000001 c0372abc b6d77ae0 c0372abc cf600e40 5335509c
> 9f80: c21e6800 01015c9c 0000000b 00008946 00000036 c03002f0 c4816b80 00000036
> 9fa0: b6d77ae0 c03000c0 01015c9c 0000000b 0000000b 00008946 becc83e8 00000000
> 9fc0: 01015c9c 0000000b 00008946 00000036 00000035 010678a0 b6d797ec b6d77ae0
> 9fe0: b6dbf738 becc838c b6d186d7 b6baa858 40000030 0000000b 00000000 00000000
> page_pool_get_stats from mvneta_ethtool_get_stats+0xa0/0xe0 [mvneta]
> mvneta_ethtool_get_stats [mvneta] from ethtool_get_stats+0x154/0x208
> ethtool_get_stats from dev_ethtool+0xf48/0x2480
> dev_ethtool from dev_ioctl+0x538/0x63c
> dev_ioctl from sock_ioctl+0x49c/0x53c
> sock_ioctl from sys_ioctl+0x134/0xbd8
> sys_ioctl from ret_fast_syscall+0x0/0x1c
> Exception stack(0xf1439fa8 to 0xf1439ff0)
> 9fa0: 01015c9c 0000000b 0000000b 00008946 becc83e8 00000000
> 9fc0: 01015c9c 0000000b 00008946 00000036 00000035 010678a0 b6d797ec b6d77ae0
> 9fe0: b6dbf738 becc838c b6d186d7 b6baa858
> Code: e28dd004 e1a05000 e2514000 0a00006a (e5902070)
>
> This commit adds the proper checks before calling page_pool_get_stats.
>
> Fixes: b3fc79225f05 ("net: mvneta: add support for page_pool_get_stats")
> Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
> Reported-by: Paulo Da Silva <Paulo.DaSilva@kyberna.com>
Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> Change from v4:
> * Remove is_stopped check
> * Variable ordering
>
> Change from v3:
> * Move the page pool check back to mvneta
>
> Change from v2:
> * Fix the fixes tag
>
> Change from v1:
> * Add cover letter
> * Move the page pool check in mvneta to the ethtool stats
> function
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 8b0f12a0e0f2..c498ef831d61 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -4733,14 +4733,17 @@ static void mvneta_ethtool_get_strings(struct net_device *netdev, u32 sset,
> u8 *data)
> {
> if (sset == ETH_SS_STATS) {
> + struct mvneta_port *pp = netdev_priv(netdev);
> int i;
>
> for (i = 0; i < ARRAY_SIZE(mvneta_statistics); i++)
> memcpy(data + i * ETH_GSTRING_LEN,
> mvneta_statistics[i].name, ETH_GSTRING_LEN);
>
> - data += ETH_GSTRING_LEN * ARRAY_SIZE(mvneta_statistics);
> - page_pool_ethtool_stats_get_strings(data);
> + if (!pp->bm_priv) {
> + data += ETH_GSTRING_LEN * ARRAY_SIZE(mvneta_statistics);
> + page_pool_ethtool_stats_get_strings(data);
> + }
> }
> }
>
> @@ -4858,8 +4861,10 @@ static void mvneta_ethtool_pp_stats(struct mvneta_port *pp, u64 *data)
> struct page_pool_stats stats = {};
> int i;
>
> - for (i = 0; i < rxq_number; i++)
> - page_pool_get_stats(pp->rxqs[i].page_pool, &stats);
> + for (i = 0; i < rxq_number; i++) {
> + if (pp->rxqs[i].page_pool)
> + page_pool_get_stats(pp->rxqs[i].page_pool, &stats);
> + }
>
> page_pool_ethtool_stats_get(data, &stats);
> }
> @@ -4875,14 +4880,21 @@ static void mvneta_ethtool_get_stats(struct net_device *dev,
> for (i = 0; i < ARRAY_SIZE(mvneta_statistics); i++)
> *data++ = pp->ethtool_stats[i];
>
> - mvneta_ethtool_pp_stats(pp, data);
> + if (!pp->bm_priv)
> + mvneta_ethtool_pp_stats(pp, data);
> }
>
> static int mvneta_ethtool_get_sset_count(struct net_device *dev, int sset)
> {
> - if (sset == ETH_SS_STATS)
> - return ARRAY_SIZE(mvneta_statistics) +
> - page_pool_ethtool_stats_get_count();
> + if (sset == ETH_SS_STATS) {
> + int count = ARRAY_SIZE(mvneta_statistics);
> + struct mvneta_port *pp = netdev_priv(dev);
> +
> + if (!pp->bm_priv)
> + count += page_pool_ethtool_stats_get_count();
> +
> + return count;
> + }
>
> return -EOPNOTSUPP;
> }
> --
> 2.42.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2023-11-11 9:49 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-11 4:41 [PATCH v5] net: mvneta: fix calls to page_pool_get_stats Sven Auhagen
2023-11-11 9:49 ` Lorenzo Bianconi [this message]
2023-11-13 11:10 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZU9OEwz7GoHbBE1m@lore-desk \
--to=lorenzo@kernel.org \
--cc=brouer@redhat.com \
--cc=ilias.apalodimas@linaro.org \
--cc=kuba@kernel.org \
--cc=leon@kernel.org \
--cc=mcroce@microsoft.com \
--cc=netdev@vger.kernel.org \
--cc=sven.auhagen@voleatech.de \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).