| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CANn89i+a_DfERsqHbi6Uu9uzCsN+wKh7WXr6Xh957Cs86ThS9A@mail.gmail.com>
Date: Thu, 23 Jan 2025 17:15:55 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, pabeni@...hat.com,
andrew+netdev@...n.ch, horms@...nel.org, michael.chan@...adcom.com,
pavan.chebbi@...adcom.com, ap420073@...il.com
Subject: Re: [PATCH net-next v2 5/7] net: ethtool: populate the default HDS
params in the core
On Sun, Jan 19, 2025 at 3:05 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> The core has the current HDS config, it can pre-populate the values
> for the drivers. While at it, remove the zero-setting in netdevsim.
> Zero are the default values since the config is zalloc'ed.
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 1 -
> drivers/net/netdevsim/ethtool.c | 5 -----
> net/ethtool/rings.c | 4 ++++
> 3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index 0a6d47d4d66b..9c5820839514 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -835,7 +835,6 @@ static void bnxt_get_ringparam(struct net_device *dev,
> ering->rx_jumbo_pending = bp->rx_agg_ring_size;
> ering->tx_pending = bp->tx_ring_size;
>
> - kernel_ering->hds_thresh = dev->cfg->hds_thresh;
> kernel_ering->hds_thresh_max = BNXT_HDS_THRESHOLD_MAX;
> }
>
> diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
> index 189793debdb7..3b23f3d3ca2b 100644
> --- a/drivers/net/netdevsim/ethtool.c
> +++ b/drivers/net/netdevsim/ethtool.c
> @@ -72,8 +72,6 @@ static void nsim_get_ringparam(struct net_device *dev,
> struct netdevsim *ns = netdev_priv(dev);
>
> memcpy(ring, &ns->ethtool.ring, sizeof(ns->ethtool.ring));
> - kernel_ring->tcp_data_split = dev->cfg->hds_config;
> - kernel_ring->hds_thresh = dev->cfg->hds_thresh;
> kernel_ring->hds_thresh_max = NSIM_HDS_THRESHOLD_MAX;
>
> if (kernel_ring->tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_UNKNOWN)
> @@ -190,9 +188,6 @@ static void nsim_ethtool_ring_init(struct netdevsim *ns)
> ns->ethtool.ring.rx_jumbo_max_pending = 4096;
> ns->ethtool.ring.rx_mini_max_pending = 4096;
> ns->ethtool.ring.tx_max_pending = 4096;
> -
> - ns->netdev->cfg->hds_config = ETHTOOL_TCP_DATA_SPLIT_UNKNOWN;
> - ns->netdev->cfg->hds_thresh = 0;
> }
>
> void nsim_ethtool_init(struct netdevsim *ns)
> diff --git a/net/ethtool/rings.c b/net/ethtool/rings.c
> index 5e8ba81fbb3e..7839bfd1ac6a 100644
> --- a/net/ethtool/rings.c
> +++ b/net/ethtool/rings.c
> @@ -39,6 +39,10 @@ static int rings_prepare_data(const struct ethnl_req_info *req_base,
> ret = ethnl_ops_begin(dev);
> if (ret < 0)
> return ret;
> +
> + data->kernel_ringparam.tcp_data_split = dev->cfg->hds_config;
> + data->kernel_ringparam.hds_thresh = dev->cfg->hds_thresh;
> +
> dev->ethtool_ops->get_ringparam(dev, &data->ringparam,
> &data->kernel_ringparam, info->extack);
> ethnl_ops_complete(dev);
> --
> 2.48.1
This patch makes syzbot unhappy [1]
I am unsure how to fix this, should all callers to
dev->ethtool_ops->get_ringparam()
have to populate tcp_data_split and hds_thresh from dev->cfg,
or would the following fix be enough ?
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 7bb94875a7ec87b3e2d882cb5df2416b9fad9d9..70461ff5c54cb787c2047ac4d67c6b0305db2b6
100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -2060,7 +2060,7 @@ static int ethtool_get_ringparam(struct
net_device *dev, void __user *useraddr)
static int ethtool_set_ringparam(struct net_device *dev, void __user *useraddr)
{
struct ethtool_ringparam ringparam, max = { .cmd = ETHTOOL_GRINGPARAM };
- struct kernel_ethtool_ringparam kernel_ringparam;
+ struct kernel_ethtool_ringparam kernel_ringparam = {};
int ret;
if (!dev->ethtool_ops->set_ringparam ||
!dev->ethtool_ops->get_ringparam)
[1]
=====================================================
BUG: KMSAN: uninit-value in nsim_get_ringparam+0xa8/0xe0
drivers/net/netdevsim/ethtool.c:77
nsim_get_ringparam+0xa8/0xe0 drivers/net/netdevsim/ethtool.c:77
ethtool_set_ringparam+0x268/0x570 net/ethtool/ioctl.c:2072
__dev_ethtool net/ethtool/ioctl.c:3209 [inline]
dev_ethtool+0x126d/0x2a40 net/ethtool/ioctl.c:3398
dev_ioctl+0xb0e/0x1280 net/core/dev_ioctl.c:759
sock_do_ioctl+0x28c/0x540 net/socket.c:1208
sock_ioctl+0x721/0xd70 net/socket.c:1313
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:906 [inline]
__se_sys_ioctl+0x246/0x440 fs/ioctl.c:892
__x64_sys_ioctl+0x96/0xe0 fs/ioctl.c:892
x64_sys_call+0x19f0/0x3c30 arch/x86/include/generated/asm/syscalls_64.h:17
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Local variable kernel_ringparam created at:
ethtool_set_ringparam+0x96/0x570 net/ethtool/ioctl.c:2063
__dev_ethtool net/ethtool/ioctl.c:3209 [inline]
dev_ethtool+0x126d/0x2a40 net/ethtool/ioctl.c:3398
CPU: 0 UID: 0 PID: 5807 Comm: syz-executor164 Not tainted
6.13.0-syzkaller-04788-g7004a2e46d16 #0
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 12/27/2024
Powered by blists - more mailing lists