[PATCH net 0/3] net: stmmac: misc fixes

[PATCH net 0/3] net: stmmac: misc fixes

[Konrad Leszczynski]

This series adds three fixes addressing KASAN panic on ethtool usage,
Enhanced Descriptor printing and flow stop on TC block setup when
interface down.

Patchset has been created as a result of discussion at [1].

[1] https://lore.kernel.org/netdev/20250826113247.3481273-1-konrad.leszczynski@intel.com/

v1 -> v2:
- add missing Fixes lines
- add missing SoB lines
- removed all non-fix patches. These will be sent in a separate series

Karol Jurczenia (1):
  net: stmmac: check if interface is running before TC block setup

Konrad Leszczynski (1):
  net: stmmac: replace memcpy with strscpy in ethtool

Piotr Warpechowski (1):
  net: stmmac: correct Tx descriptors debugfs prints

 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  |  2 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 34 ++++++++++++++-----
 2 files changed, 27 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH net 1/3] net: stmmac: replace memcpy with strscpy in ethtool

[Konrad Leszczynski]

Fix kernel exception by replacing memcpy with strscpy 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]

Fixes: 8bf993a5877e8a0a ("net: stmmac: Add support for DWMAC5 and implement Safety Features")
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..0433be4bd0c4 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);
+					strscpy(p, desc, ETH_GSTRING_LEN);
 					p += ETH_GSTRING_LEN;
 				}
 			}
-- 
2.34.1

Re: [PATCH net 1/3] net: stmmac: replace memcpy with strscpy in ethtool

[Vadim Fedorenko]

On 28/08/2025 11:02, Konrad Leszczynski wrote:
> Fix kernel exception by replacing memcpy with strscpy 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]
> 
> Fixes: 8bf993a5877e8a0a ("net: stmmac: Add support for DWMAC5 and implement Safety Features")
> 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>

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Re: [PATCH net 1/3] net: stmmac: replace memcpy with strscpy in ethtool

[Jakub Kicinski]

On Thu, 28 Aug 2025 12:02:35 +0200 Konrad Leszczynski wrote:
> Fix kernel exception by replacing memcpy with strscpy 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

If you hit this with upstream code please mention which string 
is not padded. If this can't happen with upstream platforms --
there is no upstream bug. BTW ethtool_puts() is a better choice.

> Fixes: 8bf993a5877e8a0a ("net: stmmac: Add support for DWMAC5 and implement Safety Features")
> 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..0433be4bd0c4 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);
> +					strscpy(p, desc, ETH_GSTRING_LEN);
>  					p += ETH_GSTRING_LEN;
>  				}

Re: [PATCH net 1/3] net: stmmac: replace memcpy with strscpy in ethtool

[Sebastian Basierski]

On 9/1/2025 9:59 PM, Jakub Kicinski wrote:
> On Thu, 28 Aug 2025 12:02:35 +0200 Konrad Leszczynski wrote:
>> Fix kernel exception by replacing memcpy with strscpy 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
> If you hit this with upstream code please mention which string
> is not padded. If this can't happen with upstream platforms --
> there is no upstream bug. BTW ethtool_puts() is a better choice.
Hi Jakub,
Sorry for late answer to your review.
I double checked and made sure this bug reproduces on upstream platform.
Bug seems to appear on first string - i will add this information to 
commit message.
Also thanks for code change suggestion, indeed, it looks much better.

Best Regards,
Sebastian
>> Fixes: 8bf993a5877e8a0a ("net: stmmac: Add support for DWMAC5 and implement Safety Features")
>> 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..0433be4bd0c4 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);
>> +					strscpy(p, desc, ETH_GSTRING_LEN);
>>   					p += ETH_GSTRING_LEN;
>>   				}

Re: [PATCH net 1/3] net: stmmac: replace memcpy with strscpy in ethtool

[Andrew Lunn]

On Thu, Sep 04, 2025 at 08:53:03PM +0200, Sebastian Basierski wrote:
> 
> On 9/1/2025 9:59 PM, Jakub Kicinski wrote:
> > On Thu, 28 Aug 2025 12:02:35 +0200 Konrad Leszczynski wrote:
> > > Fix kernel exception by replacing memcpy with strscpy 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
> > If you hit this with upstream code please mention which string
> > is not padded. If this can't happen with upstream platforms --
> > there is no upstream bug. BTW ethtool_puts() is a better choice.
> Hi Jakub,
> Sorry for late answer to your review.
> I double checked and made sure this bug reproduces on upstream platform.
> Bug seems to appear on first string - i will add this information to commit
> message.

By first string, do you mean "Application Transmit Interface Parity
Check Error"?

I think it also would be better to change dwmac5_error_desc, so that
it uses char stat_string[ETH_GSTRING_LEN] __nonstring; like
stmmac_stats.

     Andrew

[PATCH net 2/3] net: stmmac: correct Tx descriptors debugfs prints

[Konrad Leszczynski]

From: Piotr Warpechowski <piotr.warpechowski@intel.com>

It was observed that extended descriptors are not printed out fully and
enhanced descriptors are completely omitted in stmmac_rings_status_show().

Correct printing according to documentation and other existing prints in
the driver.

Fixes: 79a4f4dfa69a8379 ("net: stmmac: reduce dma ring display code duplication")
Reviewed-by: Sebastian Basierski <sebastian.basierski@intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Piotr Warpechowski <piotr.warpechowski@intel.com>
Signed-off-by: Konrad Leszczynski <konrad.leszczynski@intel.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 31 ++++++++++++++-----
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 7b16d1207b80..70c3dd88a749 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6351,14 +6351,25 @@ static void sysfs_display_ring(void *head, int size, int extend_desc,
 	desc_size = extend_desc ? sizeof(*ep) : sizeof(*p);
 	for (i = 0; i < size; i++) {
 		dma_addr = dma_phy_addr + i * desc_size;
-		seq_printf(seq, "%d [%pad]: 0x%x 0x%x 0x%x 0x%x\n",
-				i, &dma_addr,
-				le32_to_cpu(p->des0), le32_to_cpu(p->des1),
-				le32_to_cpu(p->des2), le32_to_cpu(p->des3));
-		if (extend_desc)
-			p = &(++ep)->basic;
-		else
+		if (extend_desc) {
+			seq_printf(seq, "%d [%pad]: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n",
+				   i, &dma_addr,
+				   le32_to_cpu(ep->basic.des0),
+				   le32_to_cpu(ep->basic.des1),
+				   le32_to_cpu(ep->basic.des2),
+				   le32_to_cpu(ep->basic.des3),
+				   le32_to_cpu(ep->des4),
+				   le32_to_cpu(ep->des5),
+				   le32_to_cpu(ep->des6),
+				   le32_to_cpu(ep->des7));
+			ep++;
+		} else {
+			seq_printf(seq, "%d [%pad]: 0x%x 0x%x 0x%x 0x%x\n",
+				   i, &dma_addr,
+				   le32_to_cpu(p->des0), le32_to_cpu(p->des1),
+				   le32_to_cpu(p->des2), le32_to_cpu(p->des3));
 			p++;
+		}
 	}
 }
 
@@ -6398,7 +6409,11 @@ static int stmmac_rings_status_show(struct seq_file *seq, void *v)
 			seq_printf(seq, "Extended descriptor ring:\n");
 			sysfs_display_ring((void *)tx_q->dma_etx,
 					   priv->dma_conf.dma_tx_size, 1, seq, tx_q->dma_tx_phy);
-		} else if (!(tx_q->tbs & STMMAC_TBS_AVAIL)) {
+		} else if (tx_q->tbs & STMMAC_TBS_AVAIL) {
+			seq_printf(seq, "Enhanced descriptor ring:\n");
+			sysfs_display_ring((void *)tx_q->dma_entx,
+					   priv->dma_conf.dma_tx_size, 1, seq, tx_q->dma_tx_phy);
+		} else {
 			seq_printf(seq, "Descriptor ring:\n");
 			sysfs_display_ring((void *)tx_q->dma_tx,
 					   priv->dma_conf.dma_tx_size, 0, seq, tx_q->dma_tx_phy);
-- 
2.34.1

Re: [PATCH net 2/3] net: stmmac: correct Tx descriptors debugfs prints

[Vadim Fedorenko]

On 28/08/2025 11:02, Konrad Leszczynski wrote:
> From: Piotr Warpechowski <piotr.warpechowski@intel.com>
> 
> It was observed that extended descriptors are not printed out fully and
> enhanced descriptors are completely omitted in stmmac_rings_status_show().
> 
> Correct printing according to documentation and other existing prints in
> the driver.
> 
> Fixes: 79a4f4dfa69a8379 ("net: stmmac: reduce dma ring display code duplication")
> Reviewed-by: Sebastian Basierski <sebastian.basierski@intel.com>
> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Signed-off-by: Piotr Warpechowski <piotr.warpechowski@intel.com>
> Signed-off-by: Konrad Leszczynski <konrad.leszczynski@intel.com>

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Re: [PATCH net 2/3] net: stmmac: correct Tx descriptors debugfs prints

[Jakub Kicinski]

On Thu, 28 Aug 2025 12:02:36 +0200 Konrad Leszczynski wrote:
> It was observed that extended descriptors are not printed out fully and
> enhanced descriptors are completely omitted in stmmac_rings_status_show().
> 
> Correct printing according to documentation and other existing prints in
> the driver.
> 
> Fixes: 79a4f4dfa69a8379 ("net: stmmac: reduce dma ring display code duplication")

Sounds like an extension to me, so net-next and no Fixes

Re: [PATCH net 2/3] net: stmmac: correct Tx descriptors debugfs prints

[Sebastian Basierski]

On 9/1/2025 10:01 PM, Jakub Kicinski wrote:
> On Thu, 28 Aug 2025 12:02:36 +0200 Konrad Leszczynski wrote:
>> It was observed that extended descriptors are not printed out fully and
>> enhanced descriptors are completely omitted in stmmac_rings_status_show().
>>
>> Correct printing according to documentation and other existing prints in
>> the driver.
>>
>> Fixes: 79a4f4dfa69a8379 ("net: stmmac: reduce dma ring display code duplication")
> Sounds like an extension to me, so net-next and no Fixes
Sure, i will drop this patch from this patchset in next revision.

[PATCH net 3/3] net: stmmac: check if interface is running before TC block setup

[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.

Fixes: 4dbbe8dde8485b89 ("net: stmmac: Add support for U32 TC filter using Flexible RX Parser")
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>
Signed-off-by: Konrad Leszczynski <konrad.leszczynski@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 70c3dd88a749..202a157a1c90 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

Re: [PATCH net 3/3] net: stmmac: check if interface is running before TC block setup

[Vadim Fedorenko]

On 28/08/2025 11:02, Konrad Leszczynski wrote:
> 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.
> 
> Fixes: 4dbbe8dde8485b89 ("net: stmmac: Add support for U32 TC filter using Flexible RX Parser")
> 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>
> Signed-off-by: Konrad Leszczynski <konrad.leszczynski@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 70c3dd88a749..202a157a1c90 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;
> +

The check looks valid, but I'm not quite sure of the error code.
Anyways,
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Re: [PATCH net 3/3] net: stmmac: check if interface is running before TC block setup

[Jakub Kicinski]

On Thu, 28 Aug 2025 12:02:37 +0200 Konrad Leszczynski wrote:
> If the interface is down before setting a TC block, the queues are already
> disabled and setup cannot proceed.

More context would be useful. What's the user-visible behavior before
and after? Can the device handle installing the filters while down? 
Is it just an issue of us restarting the queues when we shouldn't?
-- 
pw-bot: cr

Re: [PATCH net 3/3] net: stmmac: check if interface is running before TC block setup

[Sebastian Basierski]

On 9/1/2025 10:03 PM, Jakub Kicinski wrote:
> More context would be useful. What's the user-visible behavior before
> and after? Can the device handle installing the filters while down?
> Is it just an issue of us restarting the queues when we shouldn't?

Before this patch driver couldn't be unloaded with tc filter applied.

Running those commands is enough to reproduce the issue:
   tc qdisc add dev enp0s29f2 ingress
   tc filter add dev enp0s29f2 ingress protocol all prio 1 u32
   rmmod dwmac_intel

in effect module would not unload.

Re: [PATCH net 3/3] net: stmmac: check if interface is running before TC block setup

[Jakub Kicinski]

On Thu, 4 Sep 2025 21:01:49 +0200 Sebastian Basierski wrote:
> On 9/1/2025 10:03 PM, Jakub Kicinski wrote:
> > More context would be useful. What's the user-visible behavior before
> > and after? Can the device handle installing the filters while down?
> > Is it just an issue of us restarting the queues when we shouldn't?  
> 
> Before this patch driver couldn't be unloaded with tc filter applied.
> 
> Running those commands is enough to reproduce the issue:
>    tc qdisc add dev enp0s29f2 ingress
>    tc filter add dev enp0s29f2 ingress protocol all prio 1 u32
>    rmmod dwmac_intel
> 
> in effect module would not unload.

Makes sense. Could you also confirm that the offload doesn't in fact
work if set up when device is down? I think block setup is when qdisc
is installed?

ip link set dev $x down
tc qdisc add dev enp0s29f2 ingress
ip link set dev $x up
tc filter add dev enp0s29f2 ingress protocol all prio 1 u32 ...

If it doesn't work we can feel safe we're not breaking anyone's
scripts, however questionable.