From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org,
syzbot+2e5de9e3ab986b71d2bf@syzkaller.appspotmail.com,
shuah@kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH net] net: netdevsim: try to close UDP port harness races
Date: Thu, 23 Jan 2025 07:40:29 +0100 [thread overview]
Message-ID: <Z5HkXdx3w9aMsozu@mev-dev.igk.intel.com> (raw)
In-Reply-To: <20250122224503.762705-1-kuba@kernel.org>
On Wed, Jan 22, 2025 at 02:45:03PM -0800, Jakub Kicinski wrote:
> syzbot discovered that we remove the debugfs files after we free
> the netdev. Try to clean up the relevant dir while the device
> is still around.
>
> Reported-by: syzbot+2e5de9e3ab986b71d2bf@syzkaller.appspotmail.com
> Fixes: 424be63ad831 ("netdevsim: add UDP tunnel port offload support")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: shuah@kernel.org
> CC: linux-kselftest@vger.kernel.org
> ---
> drivers/net/netdevsim/netdevsim.h | 1 +
> drivers/net/netdevsim/udp_tunnels.c | 23 +++++++++++--------
> .../drivers/net/netdevsim/udp_tunnel_nic.sh | 16 ++++++-------
> 3 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
> index dcf073bc4802..96d54c08043d 100644
> --- a/drivers/net/netdevsim/netdevsim.h
> +++ b/drivers/net/netdevsim/netdevsim.h
> @@ -134,6 +134,7 @@ struct netdevsim {
> u32 sleep;
> u32 __ports[2][NSIM_UDP_TUNNEL_N_PORTS];
> u32 (*ports)[NSIM_UDP_TUNNEL_N_PORTS];
> + struct dentry *ddir;
> struct debugfs_u32_array dfs_ports[2];
> } udp_ports;
>
> diff --git a/drivers/net/netdevsim/udp_tunnels.c b/drivers/net/netdevsim/udp_tunnels.c
> index 02dc3123eb6c..640b4983a9a0 100644
> --- a/drivers/net/netdevsim/udp_tunnels.c
> +++ b/drivers/net/netdevsim/udp_tunnels.c
> @@ -112,9 +112,11 @@ nsim_udp_tunnels_info_reset_write(struct file *file, const char __user *data,
> struct net_device *dev = file->private_data;
> struct netdevsim *ns = netdev_priv(dev);
>
> - memset(ns->udp_ports.ports, 0, sizeof(ns->udp_ports.__ports));
> rtnl_lock();
> - udp_tunnel_nic_reset_ntf(dev);
> + if (dev->reg_state == NETREG_REGISTERED) {
> + memset(ns->udp_ports.ports, 0, sizeof(ns->udp_ports.__ports));
> + udp_tunnel_nic_reset_ntf(dev);
> + }
> rtnl_unlock();
>
> return count;
> @@ -144,23 +146,23 @@ int nsim_udp_tunnels_info_create(struct nsim_dev *nsim_dev,
> else
> ns->udp_ports.ports = nsim_dev->udp_ports.__ports;
>
> - debugfs_create_u32("udp_ports_inject_error", 0600,
> - ns->nsim_dev_port->ddir,
> + ns->udp_ports.ddir = debugfs_create_dir("udp_ports",
> + ns->nsim_dev_port->ddir);
> +
> + debugfs_create_u32("inject_error", 0600, ns->udp_ports.ddir,
> &ns->udp_ports.inject_error);
>
> ns->udp_ports.dfs_ports[0].array = ns->udp_ports.ports[0];
> ns->udp_ports.dfs_ports[0].n_elements = NSIM_UDP_TUNNEL_N_PORTS;
> - debugfs_create_u32_array("udp_ports_table0", 0400,
> - ns->nsim_dev_port->ddir,
> + debugfs_create_u32_array("table0", 0400, ns->udp_ports.ddir,
> &ns->udp_ports.dfs_ports[0]);
>
> ns->udp_ports.dfs_ports[1].array = ns->udp_ports.ports[1];
> ns->udp_ports.dfs_ports[1].n_elements = NSIM_UDP_TUNNEL_N_PORTS;
> - debugfs_create_u32_array("udp_ports_table1", 0400,
> - ns->nsim_dev_port->ddir,
> + debugfs_create_u32_array("table1", 0400, ns->udp_ports.ddir,
> &ns->udp_ports.dfs_ports[1]);
>
> - debugfs_create_file("udp_ports_reset", 0200, ns->nsim_dev_port->ddir,
> + debugfs_create_file("reset", 0200, ns->udp_ports.ddir,
> dev, &nsim_udp_tunnels_info_reset_fops);
>
> /* Note: it's not normal to allocate the info struct like this!
> @@ -196,6 +198,9 @@ int nsim_udp_tunnels_info_create(struct nsim_dev *nsim_dev,
>
> void nsim_udp_tunnels_info_destroy(struct net_device *dev)
> {
> + struct netdevsim *ns = netdev_priv(dev);
> +
> + debugfs_remove_recursive(ns->udp_ports.ddir);
> kfree(dev->udp_tunnel_nic_info);
> dev->udp_tunnel_nic_info = NULL;
> }
> diff --git a/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh b/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
> index 384cfa3d38a6..92c2f0376c08 100755
> --- a/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
> +++ b/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
> @@ -142,7 +142,7 @@ function pre_ethtool {
> }
>
> function check_table {
> - local path=$NSIM_DEV_DFS/ports/$port/udp_ports_table$1
> + local path=$NSIM_DEV_DFS/ports/$port/udp_ports/table$1
> local -n expected=$2
> local last=$3
>
> @@ -212,7 +212,7 @@ function check_tables {
> }
>
> function print_table {
> - local path=$NSIM_DEV_DFS/ports/$port/udp_ports_table$1
> + local path=$NSIM_DEV_DFS/ports/$port/udp_ports/table$1
> read -a have < $path
>
> tree $NSIM_DEV_DFS/
> @@ -641,7 +641,7 @@ for port in 0 1; do
> NSIM_NETDEV=`get_netdev_name old_netdevs`
> ip link set dev $NSIM_NETDEV up
>
> - echo 110 > $NSIM_DEV_DFS/ports/$port/udp_ports_inject_error
> + echo 110 > $NSIM_DEV_DFS/ports/$port/udp_ports/inject_error
>
> msg="1 - create VxLANs v6"
> exp0=( 0 0 0 0 )
> @@ -663,7 +663,7 @@ for port in 0 1; do
> new_geneve gnv0 20000
>
> msg="2 - destroy GENEVE"
> - echo 2 > $NSIM_DEV_DFS/ports/$port/udp_ports_inject_error
> + echo 2 > $NSIM_DEV_DFS/ports/$port/udp_ports/inject_error
> exp1=( `mke 20000 2` 0 0 0 )
> del_dev gnv0
>
> @@ -764,7 +764,7 @@ for port in 0 1; do
> msg="create VxLANs v4"
> new_vxlan vxlan0 10000 $NSIM_NETDEV
>
> - echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports_reset
> + echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports/reset
> check_tables
>
> msg="NIC device goes down"
> @@ -775,7 +775,7 @@ for port in 0 1; do
> fi
> check_tables
>
> - echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports_reset
> + echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports/reset
> check_tables
>
> msg="NIC device goes up again"
> @@ -789,7 +789,7 @@ for port in 0 1; do
> del_dev vxlan0
> check_tables
>
> - echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports_reset
> + echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports/reset
> check_tables
>
> msg="destroy NIC"
> @@ -896,7 +896,7 @@ msg="vacate VxLAN in overflow table"
> exp0=( `mke 10000 1` `mke 10004 1` 0 `mke 10003 1` )
> del_dev vxlan2
>
> -echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports_reset
> +echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports/reset
> check_tables
>
> msg="tunnels destroyed 2"
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> --
> 2.48.1
next prev parent reply other threads:[~2025-01-23 6:43 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-22 22:45 [PATCH net] net: netdevsim: try to close UDP port harness races Jakub Kicinski
2025-01-23 6:40 ` Michal Swiatkowski [this message]
2025-01-27 22:30 ` 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=Z5HkXdx3w9aMsozu@mev-dev.igk.intel.com \
--to=michal.swiatkowski@linux.intel.com \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=shuah@kernel.org \
--cc=syzbot+2e5de9e3ab986b71d2bf@syzkaller.appspotmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.