* [PATCH net v4 0/2] net: stmmac: misc fixes @ 2025-09-16 12:09 Konrad Leszczynski 2025-09-16 12:09 ` [PATCH net v4 1/2] net: stmmac: replace memcpy with ethtool_puts in ethtool Konrad Leszczynski 2025-09-16 12:09 ` [PATCH net v4 2/2] net: stmmac: check if interface is running before TC block setup Konrad Leszczynski 0 siblings, 2 replies; 8+ messages in thread From: Konrad Leszczynski @ 2025-09-16 12:09 UTC (permalink / raw) To: davem, andrew+netdev, edumazet, kuba, pabeni Cc: netdev, linux-kernel, cezary.rojewski, sebastian.basierski, Konrad Leszczynski This series adds two fixes addressing KASAN panic on ethtool usage and flow stop on TC block setup when interface down. Patchset has been created as a result of discussion at [1]. v2 can be found at [2]. v3 can be found at [3]. [1] https://lore.kernel.org/netdev/20250826113247.3481273-1-konrad.leszczynski@intel.com/ [2] https://lore.kernel.org/netdev/20250828100237.4076570-1-konrad.leszczynski@intel.com/ [3] https://lore.kernel.org/netdev/20250916092507.216613-1-konrad.leszczynski@intel.com/ v3 -> v4: - add missing Reviewed-by lines v2 -> v3: - replace strcpy with ethtool_puts per suggestion - removed extension patch for printing enhanced descriptors v1 -> v2: - add missing Fixes lines - add missing SoB lines - removed all non-fix patches. These will be sent in separate series Karol Jurczenia (1): net: stmmac: check if interface is running before TC block setup Konrad Leszczynski (1): net: stmmac: replace memcpy with ethtool_puts in ethtool drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net v4 1/2] net: stmmac: replace memcpy with ethtool_puts in ethtool 2025-09-16 12:09 [PATCH net v4 0/2] net: stmmac: misc fixes Konrad Leszczynski @ 2025-09-16 12:09 ` Konrad Leszczynski 2025-09-16 16:45 ` Simon Horman 2025-09-16 12:09 ` [PATCH net v4 2/2] net: stmmac: check if interface is running before TC block setup Konrad Leszczynski 1 sibling, 1 reply; 8+ messages in thread From: Konrad Leszczynski @ 2025-09-16 12:09 UTC (permalink / raw) To: davem, andrew+netdev, edumazet, kuba, pabeni Cc: netdev, linux-kernel, cezary.rojewski, sebastian.basierski, Konrad Leszczynski Fix kernel exception by replacing memcpy with ethtool_puts when used with safety feature strings in ethtool logic. [ +0.000023] BUG: KASAN: global-out-of-bounds in stmmac_get_strings+0x17d/0x520 [stmmac] [ +0.000115] Read of size 32 at addr ffffffffc0cfab20 by task ethtool/2571 [ +0.000005] Call Trace: [ +0.000004] <TASK> [ +0.000003] dump_stack_lvl+0x6c/0x90 [ +0.000016] print_report+0xce/0x610 [ +0.000011] ? stmmac_get_strings+0x17d/0x520 [stmmac] [ +0.000108] ? kasan_addr_to_slab+0xd/0xa0 [ +0.000008] ? stmmac_get_strings+0x17d/0x520 [stmmac] [ +0.000101] kasan_report+0xd4/0x110 [ +0.000010] ? stmmac_get_strings+0x17d/0x520 [stmmac] [ +0.000102] kasan_check_range+0x3a/0x1c0 [ +0.000010] __asan_memcpy+0x24/0x70 [ +0.000008] stmmac_get_strings+0x17d/0x520 [stmmac] Reviewed-by: Sebastian Basierski <sebastian.basierski@intel.com> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com> Signed-off-by: Konrad Leszczynski <konrad.leszczynski@intel.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index 77758a7299b4..d5a2b7e9b2a9 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -752,7 +752,7 @@ static void stmmac_get_strings(struct net_device *dev, u32 stringset, u8 *data) if (!stmmac_safety_feat_dump(priv, &priv->sstats, i, NULL, &desc)) { - memcpy(p, desc, ETH_GSTRING_LEN); + ethtool_puts(&p, desc); p += ETH_GSTRING_LEN; } } -- 2.34.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net v4 1/2] net: stmmac: replace memcpy with ethtool_puts in ethtool 2025-09-16 12:09 ` [PATCH net v4 1/2] net: stmmac: replace memcpy with ethtool_puts in ethtool Konrad Leszczynski @ 2025-09-16 16:45 ` Simon Horman 2025-09-16 17:12 ` Andrew Lunn 0 siblings, 1 reply; 8+ messages in thread From: Simon Horman @ 2025-09-16 16:45 UTC (permalink / raw) To: Konrad Leszczynski Cc: davem, andrew+netdev, edumazet, kuba, pabeni, netdev, linux-kernel, cezary.rojewski, sebastian.basierski On Tue, Sep 16, 2025 at 02:09:31PM +0200, Konrad Leszczynski wrote: > Fix kernel exception by replacing memcpy with ethtool_puts when used with > safety feature strings in ethtool logic. > > [ +0.000023] BUG: KASAN: global-out-of-bounds in stmmac_get_strings+0x17d/0x520 [stmmac] > [ +0.000115] Read of size 32 at addr ffffffffc0cfab20 by task ethtool/2571 > > [ +0.000005] Call Trace: > [ +0.000004] <TASK> > [ +0.000003] dump_stack_lvl+0x6c/0x90 > [ +0.000016] print_report+0xce/0x610 > [ +0.000011] ? stmmac_get_strings+0x17d/0x520 [stmmac] > [ +0.000108] ? kasan_addr_to_slab+0xd/0xa0 > [ +0.000008] ? stmmac_get_strings+0x17d/0x520 [stmmac] > [ +0.000101] kasan_report+0xd4/0x110 > [ +0.000010] ? stmmac_get_strings+0x17d/0x520 [stmmac] > [ +0.000102] kasan_check_range+0x3a/0x1c0 > [ +0.000010] __asan_memcpy+0x24/0x70 > [ +0.000008] stmmac_get_strings+0x17d/0x520 [stmmac] > > Reviewed-by: Sebastian Basierski <sebastian.basierski@intel.com> > Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com> > Signed-off-by: Konrad Leszczynski <konrad.leszczynski@intel.com> As per my comment on v3 (sorry, I missed that you had already posted v4): I think it would be good to explain why using memcpy() is a problem here. I looked and it seems to me that one reason is that the strings may be too long. But are there others? > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > index 77758a7299b4..d5a2b7e9b2a9 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > @@ -752,7 +752,7 @@ static void stmmac_get_strings(struct net_device *dev, u32 stringset, u8 *data) > if (!stmmac_safety_feat_dump(priv, > &priv->sstats, i, > NULL, &desc)) { > - memcpy(p, desc, ETH_GSTRING_LEN); > + ethtool_puts(&p, desc); > p += ETH_GSTRING_LEN; ethtool_puts() increments p, so I think the line above should be removed. > } > } > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v4 1/2] net: stmmac: replace memcpy with ethtool_puts in ethtool 2025-09-16 16:45 ` Simon Horman @ 2025-09-16 17:12 ` Andrew Lunn 2025-09-16 23:25 ` Jakub Kicinski 0 siblings, 1 reply; 8+ messages in thread From: Andrew Lunn @ 2025-09-16 17:12 UTC (permalink / raw) To: Simon Horman Cc: Konrad Leszczynski, davem, andrew+netdev, edumazet, kuba, pabeni, netdev, linux-kernel, cezary.rojewski, sebastian.basierski On Tue, Sep 16, 2025 at 05:45:30PM +0100, Simon Horman wrote: > On Tue, Sep 16, 2025 at 02:09:31PM +0200, Konrad Leszczynski wrote: > > Fix kernel exception by replacing memcpy with ethtool_puts when used with > > safety feature strings in ethtool logic. > > > > [ +0.000023] BUG: KASAN: global-out-of-bounds in stmmac_get_strings+0x17d/0x520 [stmmac] > > [ +0.000115] Read of size 32 at addr ffffffffc0cfab20 by task ethtool/2571 > > > > [ +0.000005] Call Trace: > > [ +0.000004] <TASK> > > [ +0.000003] dump_stack_lvl+0x6c/0x90 > > [ +0.000016] print_report+0xce/0x610 > > [ +0.000011] ? stmmac_get_strings+0x17d/0x520 [stmmac] > > [ +0.000108] ? kasan_addr_to_slab+0xd/0xa0 > > [ +0.000008] ? stmmac_get_strings+0x17d/0x520 [stmmac] > > [ +0.000101] kasan_report+0xd4/0x110 > > [ +0.000010] ? stmmac_get_strings+0x17d/0x520 [stmmac] > > [ +0.000102] kasan_check_range+0x3a/0x1c0 > > [ +0.000010] __asan_memcpy+0x24/0x70 > > [ +0.000008] stmmac_get_strings+0x17d/0x520 [stmmac] > > > > Reviewed-by: Sebastian Basierski <sebastian.basierski@intel.com> > > Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com> > > Signed-off-by: Konrad Leszczynski <konrad.leszczynski@intel.com> > > As per my comment on v3 (sorry, I missed that you had already posted v4): > > I think it would be good to explain why using memcpy() is a problem here. > > I looked and it seems to me that one reason is that the strings may be > too long. I actually think the strings are too short. But i could be wrong. In general, stmmac uses: struct stmmac_stats { char stat_string[ETH_GSTRING_LEN] __nonstring; int sizeof_stat; int stat_offset; }; so the string is guaranteed to be long enough. However, these 'safety' features make use of struct dwmac5_error_desc { bool valid; const char *desc; const char *detailed_desc; }; where desc can be any length, ironically making dwmac5_safety_feat_dump() unsafe. This is why i asked that this be changed to be the same stmmac_stats, so [ETH_GSTRING_LEN] __nonstring. But that seems to of fallen on deaf ears. Andrew ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v4 1/2] net: stmmac: replace memcpy with ethtool_puts in ethtool 2025-09-16 17:12 ` Andrew Lunn @ 2025-09-16 23:25 ` Jakub Kicinski 2025-09-17 15:54 ` Tony Nguyen 0 siblings, 1 reply; 8+ messages in thread From: Jakub Kicinski @ 2025-09-16 23:25 UTC (permalink / raw) To: Andrew Lunn Cc: Simon Horman, Konrad Leszczynski, davem, andrew+netdev, edumazet, pabeni, netdev, linux-kernel, cezary.rojewski, sebastian.basierski, Przemek Kitszel, Tony Nguyen On Tue, 16 Sep 2025 19:12:22 +0200 Andrew Lunn wrote: > where desc can be any length, ironically making > dwmac5_safety_feat_dump() unsafe. This is why i asked that this be > changed to be the same stmmac_stats, so [ETH_GSTRING_LEN] > __nonstring. But that seems to of fallen on deaf ears. Yes, very strange. Tony, Przemek, I suspect the folks here are from "different part of Intel" but I think they need some guidance.. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v4 1/2] net: stmmac: replace memcpy with ethtool_puts in ethtool 2025-09-16 23:25 ` Jakub Kicinski @ 2025-09-17 15:54 ` Tony Nguyen 0 siblings, 0 replies; 8+ messages in thread From: Tony Nguyen @ 2025-09-17 15:54 UTC (permalink / raw) To: Jakub Kicinski, Andrew Lunn Cc: Simon Horman, Konrad Leszczynski, davem, andrew+netdev, edumazet, pabeni, netdev, linux-kernel, cezary.rojewski, sebastian.basierski, Przemek Kitszel On 9/16/2025 4:25 PM, Jakub Kicinski wrote: > On Tue, 16 Sep 2025 19:12:22 +0200 Andrew Lunn wrote: >> where desc can be any length, ironically making >> dwmac5_safety_feat_dump() unsafe. This is why i asked that this be >> changed to be the same stmmac_stats, so [ETH_GSTRING_LEN] >> __nonstring. But that seems to of fallen on deaf ears. > > Yes, very strange. > > Tony, Przemek, I suspect the folks here are from "different part of > Intel" but I think they need some guidance.. I'll reach out and see if we can provide them some help. Thanks, Tony ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net v4 2/2] net: stmmac: check if interface is running before TC block setup 2025-09-16 12:09 [PATCH net v4 0/2] net: stmmac: misc fixes Konrad Leszczynski 2025-09-16 12:09 ` [PATCH net v4 1/2] net: stmmac: replace memcpy with ethtool_puts in ethtool Konrad Leszczynski @ 2025-09-16 12:09 ` Konrad Leszczynski 2025-09-16 23:25 ` Jakub Kicinski 1 sibling, 1 reply; 8+ messages in thread From: Konrad Leszczynski @ 2025-09-16 12:09 UTC (permalink / raw) To: davem, andrew+netdev, edumazet, kuba, pabeni Cc: netdev, linux-kernel, cezary.rojewski, sebastian.basierski, Karol Jurczenia, Konrad Leszczynski From: Karol Jurczenia <karol.jurczenia@intel.com> If the interface is down before setting a TC block, the queues are already disabled and setup cannot proceed. Reviewed-by: Konrad Leszczynski <konrad.leszczynski@intel.com> Reviewed-by: Sebastian Basierski <sebastian.basierski@intel.com> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com> Signed-off-by: Karol Jurczenia <karol.jurczenia@intel.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 7b16d1207b80..133ffdc91153 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -6247,6 +6247,9 @@ static int stmmac_setup_tc_block_cb(enum tc_setup_type type, void *type_data, struct stmmac_priv *priv = cb_priv; int ret = -EOPNOTSUPP; + if (!netif_running(priv->dev)) + return -EINVAL; + if (!tc_cls_can_offload_and_chain0(priv->dev, type_data)) return ret; -- 2.34.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net v4 2/2] net: stmmac: check if interface is running before TC block setup 2025-09-16 12:09 ` [PATCH net v4 2/2] net: stmmac: check if interface is running before TC block setup Konrad Leszczynski @ 2025-09-16 23:25 ` Jakub Kicinski 0 siblings, 0 replies; 8+ messages in thread From: Jakub Kicinski @ 2025-09-16 23:25 UTC (permalink / raw) To: Konrad Leszczynski Cc: davem, andrew+netdev, edumazet, pabeni, netdev, linux-kernel, cezary.rojewski, sebastian.basierski, Karol Jurczenia On Tue, 16 Sep 2025 14:09:32 +0200 Konrad Leszczynski wrote: > If the interface is down before setting a TC block, the queues are already > disabled and setup cannot proceed. I asked for more details in the commit message earlier. Also where did the Fixes tags go? The Fixes tags should only be removed for the patches which end up going to net-next. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-17 15:55 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-16 12:09 [PATCH net v4 0/2] net: stmmac: misc fixes Konrad Leszczynski 2025-09-16 12:09 ` [PATCH net v4 1/2] net: stmmac: replace memcpy with ethtool_puts in ethtool Konrad Leszczynski 2025-09-16 16:45 ` Simon Horman 2025-09-16 17:12 ` Andrew Lunn 2025-09-16 23:25 ` Jakub Kicinski 2025-09-17 15:54 ` Tony Nguyen 2025-09-16 12:09 ` [PATCH net v4 2/2] net: stmmac: check if interface is running before TC block setup Konrad Leszczynski 2025-09-16 23:25 ` Jakub Kicinski
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).