1. Patchew
  2. linux
  3. [PATCH net 0/3] net: stmmac: misc fixes
  4. View patch

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

Konrad Leszczynski posted 3 patches 1 month ago
There is a newer version of this series
[PATCH net 1/3] net: stmmac: replace memcpy with strscpy in ethtool
Posted by Konrad Leszczynski 1 month ago 
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
Posted by Jakub Kicinski 1 month ago 
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
Posted by Sebastian Basierski 4 weeks ago 
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
Posted by Andrew Lunn 4 weeks ago 
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
Re: [PATCH net 1/3] net: stmmac: replace memcpy with strscpy in ethtool
Posted by Konrad Leszczynski 2 weeks, 4 days ago 
On 04-Sep-25 21:18, Andrew Lunn wrote:
> 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

Hi Andrew,

Thanks for your comments. We can add the change as a new patch as part 
of this patchset. Would that be ok?
Re: [PATCH net 1/3] net: stmmac: replace memcpy with strscpy in ethtool
Posted by Vadim Fedorenko 1 month ago 
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>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.