[PATCH net 0/2] mlxsw: Various fixes

[PATCH net 0/2] mlxsw: Various fixes

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

Fix two issues found by syzkaller.

Patch #1 removes inappropriate usage of WARN_ON() following memory
allocation failure. Constantly triggered when syzkaller injects faults.

Patch #2 fixes a use-after-free that can be triggered by 'devlink dev
info' following a failed devlink reload.

Ido Schimmel (2):
  mlxsw: spectrum_router: Remove inappropriate usage of WARN_ON()
  mlxsw: pci: Fix use-after-free in case of failed devlink reload

 drivers/net/ethernet/mellanox/mlxsw/pci.c     | 54 +++++++++++++------
 .../ethernet/mellanox/mlxsw/spectrum_router.c |  2 +-
 2 files changed, 39 insertions(+), 17 deletions(-)

-- 
2.26.2

[PATCH net 1/2] mlxsw: spectrum_router: Remove inappropriate usage of WARN_ON()

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

We should not trigger a warning when a memory allocation fails. Remove
the WARN_ON().

The warning is constantly triggered by syzkaller when it is injecting
faults:

[ 2230.758664] FAULT_INJECTION: forcing a failure.
[ 2230.758664] name failslab, interval 1, probability 0, space 0, times 0
[ 2230.762329] CPU: 3 PID: 1407 Comm: syz-executor.0 Not tainted 5.8.0-rc2+ #28
...
[ 2230.898175] WARNING: CPU: 3 PID: 1407 at drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:6265 mlxsw_sp_router_fib_event+0xfad/0x13e0
[ 2230.898179] Kernel panic - not syncing: panic_on_warn set ...
[ 2230.898183] CPU: 3 PID: 1407 Comm: syz-executor.0 Not tainted 5.8.0-rc2+ #28
[ 2230.898190] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014

Fixes: 3057224e014c ("mlxsw: spectrum_router: Implement FIB offload in deferred work")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 770de0222e7b..019ed503aadf 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -6262,7 +6262,7 @@ static int mlxsw_sp_router_fib_event(struct notifier_block *nb,
 	}
 
 	fib_work = kzalloc(sizeof(*fib_work), GFP_ATOMIC);
-	if (WARN_ON(!fib_work))
+	if (!fib_work)
 		return NOTIFY_BAD;
 
 	fib_work->mlxsw_sp = router->mlxsw_sp;
-- 
2.26.2

[PATCH net 2/2] mlxsw: pci: Fix use-after-free in case of failed devlink reload

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

In case devlink reload failed, it is possible to trigger a
use-after-free when querying the kernel for device info via 'devlink dev
info' [1].

This happens because as part of the reload error path the PCI command
interface is de-initialized and its mailboxes are freed. When the
devlink '->info_get()' callback is invoked the device is queried via the
command interface and the freed mailboxes are accessed.

Fix this by initializing the command interface once during probe and not
during every reload.

This is consistent with the other bus used by mlxsw (i.e., 'mlxsw_i2c')
and also allows user space to query the running firmware version (for
example) from the device after a failed reload.

[1]
BUG: KASAN: use-after-free in memcpy include/linux/string.h:406 [inline]
BUG: KASAN: use-after-free in mlxsw_pci_cmd_exec+0x177/0xa60 drivers/net/ethernet/mellanox/mlxsw/pci.c:1675
Write of size 4096 at addr ffff88810ae32000 by task syz-executor.1/2355

CPU: 1 PID: 2355 Comm: syz-executor.1 Not tainted 5.8.0-rc2+ #29
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xf6/0x16e lib/dump_stack.c:118
 print_address_description.constprop.0+0x1c/0x250 mm/kasan/report.c:383
 __kasan_report mm/kasan/report.c:513 [inline]
 kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
 check_memory_region_inline mm/kasan/generic.c:186 [inline]
 check_memory_region+0x14e/0x1b0 mm/kasan/generic.c:192
 memcpy+0x39/0x60 mm/kasan/common.c:106
 memcpy include/linux/string.h:406 [inline]
 mlxsw_pci_cmd_exec+0x177/0xa60 drivers/net/ethernet/mellanox/mlxsw/pci.c:1675
 mlxsw_cmd_exec+0x249/0x550 drivers/net/ethernet/mellanox/mlxsw/core.c:2335
 mlxsw_cmd_access_reg drivers/net/ethernet/mellanox/mlxsw/cmd.h:859 [inline]
 mlxsw_core_reg_access_cmd drivers/net/ethernet/mellanox/mlxsw/core.c:1938 [inline]
 mlxsw_core_reg_access+0x2f6/0x540 drivers/net/ethernet/mellanox/mlxsw/core.c:1985
 mlxsw_reg_query drivers/net/ethernet/mellanox/mlxsw/core.c:2000 [inline]
 mlxsw_devlink_info_get+0x17f/0x6e0 drivers/net/ethernet/mellanox/mlxsw/core.c:1090
 devlink_nl_info_fill.constprop.0+0x13c/0x2d0 net/core/devlink.c:4588
 devlink_nl_cmd_info_get_dumpit+0x246/0x460 net/core/devlink.c:4648
 genl_lock_dumpit+0x85/0xc0 net/netlink/genetlink.c:575
 netlink_dump+0x515/0xe50 net/netlink/af_netlink.c:2245
 __netlink_dump_start+0x53d/0x830 net/netlink/af_netlink.c:2353
 genl_family_rcv_msg_dumpit.isra.0+0x296/0x300 net/netlink/genetlink.c:638
 genl_family_rcv_msg net/netlink/genetlink.c:733 [inline]
 genl_rcv_msg+0x78d/0x9d0 net/netlink/genetlink.c:753
 netlink_rcv_skb+0x152/0x440 net/netlink/af_netlink.c:2469
 genl_rcv+0x24/0x40 net/netlink/genetlink.c:764
 netlink_unicast_kernel net/netlink/af_netlink.c:1303 [inline]
 netlink_unicast+0x53a/0x750 net/netlink/af_netlink.c:1329
 netlink_sendmsg+0x850/0xd90 net/netlink/af_netlink.c:1918
 sock_sendmsg_nosec net/socket.c:652 [inline]
 sock_sendmsg+0x150/0x190 net/socket.c:672
 ____sys_sendmsg+0x6d8/0x840 net/socket.c:2363
 ___sys_sendmsg+0xff/0x170 net/socket.c:2417
 __sys_sendmsg+0xe5/0x1b0 net/socket.c:2450
 do_syscall_64+0x56/0xa0 arch/x86/entry/common.c:359
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Fixes: a9c8336f6544 ("mlxsw: core: Add support for devlink info command")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 54 ++++++++++++++++-------
 1 file changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index fd0e97de44e7..c04ec1a92826 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -1414,23 +1414,12 @@ static int mlxsw_pci_init(void *bus_priv, struct mlxsw_core *mlxsw_core,
 	u16 num_pages;
 	int err;
 
-	mutex_init(&mlxsw_pci->cmd.lock);
-	init_waitqueue_head(&mlxsw_pci->cmd.wait);
-
 	mlxsw_pci->core = mlxsw_core;
 
 	mbox = mlxsw_cmd_mbox_alloc();
 	if (!mbox)
 		return -ENOMEM;
 
-	err = mlxsw_pci_mbox_alloc(mlxsw_pci, &mlxsw_pci->cmd.in_mbox);
-	if (err)
-		goto mbox_put;
-
-	err = mlxsw_pci_mbox_alloc(mlxsw_pci, &mlxsw_pci->cmd.out_mbox);
-	if (err)
-		goto err_out_mbox_alloc;
-
 	err = mlxsw_pci_sw_reset(mlxsw_pci, mlxsw_pci->id);
 	if (err)
 		goto err_sw_reset;
@@ -1537,9 +1526,6 @@ static int mlxsw_pci_init(void *bus_priv, struct mlxsw_core *mlxsw_core,
 	mlxsw_pci_free_irq_vectors(mlxsw_pci);
 err_alloc_irq:
 err_sw_reset:
-	mlxsw_pci_mbox_free(mlxsw_pci, &mlxsw_pci->cmd.out_mbox);
-err_out_mbox_alloc:
-	mlxsw_pci_mbox_free(mlxsw_pci, &mlxsw_pci->cmd.in_mbox);
 mbox_put:
 	mlxsw_cmd_mbox_free(mbox);
 	return err;
@@ -1553,8 +1539,6 @@ static void mlxsw_pci_fini(void *bus_priv)
 	mlxsw_pci_aqs_fini(mlxsw_pci);
 	mlxsw_pci_fw_area_fini(mlxsw_pci);
 	mlxsw_pci_free_irq_vectors(mlxsw_pci);
-	mlxsw_pci_mbox_free(mlxsw_pci, &mlxsw_pci->cmd.out_mbox);
-	mlxsw_pci_mbox_free(mlxsw_pci, &mlxsw_pci->cmd.in_mbox);
 }
 
 static struct mlxsw_pci_queue *
@@ -1776,6 +1760,37 @@ static const struct mlxsw_bus mlxsw_pci_bus = {
 	.features		= MLXSW_BUS_F_TXRX | MLXSW_BUS_F_RESET,
 };
 
+static int mlxsw_pci_cmd_init(struct mlxsw_pci *mlxsw_pci)
+{
+	int err;
+
+	mutex_init(&mlxsw_pci->cmd.lock);
+	init_waitqueue_head(&mlxsw_pci->cmd.wait);
+
+	err = mlxsw_pci_mbox_alloc(mlxsw_pci, &mlxsw_pci->cmd.in_mbox);
+	if (err)
+		goto err_in_mbox_alloc;
+
+	err = mlxsw_pci_mbox_alloc(mlxsw_pci, &mlxsw_pci->cmd.out_mbox);
+	if (err)
+		goto err_out_mbox_alloc;
+
+	return 0;
+
+err_out_mbox_alloc:
+	mlxsw_pci_mbox_free(mlxsw_pci, &mlxsw_pci->cmd.in_mbox);
+err_in_mbox_alloc:
+	mutex_destroy(&mlxsw_pci->cmd.lock);
+	return err;
+}
+
+static void mlxsw_pci_cmd_fini(struct mlxsw_pci *mlxsw_pci)
+{
+	mlxsw_pci_mbox_free(mlxsw_pci, &mlxsw_pci->cmd.out_mbox);
+	mlxsw_pci_mbox_free(mlxsw_pci, &mlxsw_pci->cmd.in_mbox);
+	mutex_destroy(&mlxsw_pci->cmd.lock);
+}
+
 static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	const char *driver_name = pdev->driver->name;
@@ -1831,6 +1846,10 @@ static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mlxsw_pci->pdev = pdev;
 	pci_set_drvdata(pdev, mlxsw_pci);
 
+	err = mlxsw_pci_cmd_init(mlxsw_pci);
+	if (err)
+		goto err_pci_cmd_init;
+
 	mlxsw_pci->bus_info.device_kind = driver_name;
 	mlxsw_pci->bus_info.device_name = pci_name(mlxsw_pci->pdev);
 	mlxsw_pci->bus_info.dev = &pdev->dev;
@@ -1848,6 +1867,8 @@ static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	return 0;
 
 err_bus_device_register:
+	mlxsw_pci_cmd_fini(mlxsw_pci);
+err_pci_cmd_init:
 	iounmap(mlxsw_pci->hw_addr);
 err_ioremap:
 err_pci_resource_len_check:
@@ -1865,6 +1886,7 @@ static void mlxsw_pci_remove(struct pci_dev *pdev)
 	struct mlxsw_pci *mlxsw_pci = pci_get_drvdata(pdev);
 
 	mlxsw_core_bus_device_unregister(mlxsw_pci->core, false);
+	mlxsw_pci_cmd_fini(mlxsw_pci);
 	iounmap(mlxsw_pci->hw_addr);
 	pci_release_regions(mlxsw_pci->pdev);
 	pci_disable_device(mlxsw_pci->pdev);
-- 
2.26.2

Re: [PATCH net 0/2] mlxsw: Various fixes

[David Miller]

From: Ido Schimmel <idosch@idosch.org>
Date: Fri, 10 Jul 2020 16:41:37 +0300

> From: Ido Schimmel <idosch@mellanox.com>
> 
> Fix two issues found by syzkaller.
> 
> Patch #1 removes inappropriate usage of WARN_ON() following memory
> allocation failure. Constantly triggered when syzkaller injects faults.
> 
> Patch #2 fixes a use-after-free that can be triggered by 'devlink dev
> info' following a failed devlink reload.

Series applied and queued up for -stable, thanks.

Re: [PATCH net 0/2] mlxsw: Various fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Thu, 28 Jan 2021 16:48:18 +0200 you wrote:
> From: Ido Schimmel <idosch@nvidia.com>
> 
> Patch #1 fixes wrong invocation of mausezahn in a couple of selftests.
> The tests started failing after Fedora updated their libnet package from
> version 1.1.6 to 1.2.1. With the fix the tests pass regardless of libnet
> version.
> 
> [...]

Here is the summary with links:
  - [net,1/2] selftests: forwarding: Specify interface when invoking mausezahn
    https://git.kernel.org/netdev/net/c/11df27f7fdf0
  - [net,2/2] mlxsw: spectrum_span: Do not overwrite policer configuration
    https://git.kernel.org/netdev/net/c/b6f6881aaf23

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH net 0/2] mlxsw: Various fixes

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Patch #1 fixes wrong invocation of mausezahn in a couple of selftests.
The tests started failing after Fedora updated their libnet package from
version 1.1.6 to 1.2.1. With the fix the tests pass regardless of libnet
version.

Patch #2 fixes an issue in the mirroring to CPU code that results in
policer configuration being overwritten.

Danielle Ratson (1):
  selftests: forwarding: Specify interface when invoking mausezahn

Ido Schimmel (1):
  mlxsw: spectrum_span: Do not overwrite policer configuration

 drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c        | 6 ++++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum_span.h        | 1 +
 tools/testing/selftests/net/forwarding/router_mpath_nh.sh  | 2 +-
 tools/testing/selftests/net/forwarding/router_multipath.sh | 2 +-
 4 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.29.2

Re: [PATCH net 0/2] mlxsw: Various fixes

[David Miller]

From: Ido Schimmel <idosch@idosch.org>
Date: Thu, 21 May 2020 15:11:43 +0300

> From: Ido Schimmel <idosch@mellanox.com>
> 
> Patch #1 from Jiri fixes a use-after-free discovered while fuzzing mlxsw
> / devlink with syzkaller.
> 
> Patch #2 from Amit works around a limitation in new versions of arping,
> which is used in several selftests.

Series applied, thanks.

[PATCH net 0/2] mlxsw: Various fixes

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

Patch #1 from Jiri fixes a use-after-free discovered while fuzzing mlxsw
/ devlink with syzkaller.

Patch #2 from Amit works around a limitation in new versions of arping,
which is used in several selftests.

Amit Cohen (1):
  selftests: mlxsw: qos_mc_aware: Specify arping timeout as an integer

Jiri Pirko (1):
  mlxsw: spectrum: Fix use-after-free of split/unsplit/type_set in case
    reload fails

 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 14 ++++++++++++--
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c     |  8 ++++++++
 .../selftests/drivers/net/mlxsw/qos_mc_aware.sh    |  2 +-
 3 files changed, 21 insertions(+), 3 deletions(-)

-- 
2.26.2

Re: [PATCH net 0/2] mlxsw: Various fixes

[David Miller]

From: Ido Schimmel <idosch@mellanox.com>
Date: Tue, 12 Mar 2019 08:40:40 +0000

> Patch #1 fixes the recently introduced QSFP thermal zones to correctly
> work with split ports, where several ports are mapped to the same
> module.
> 
> Patch #2 initializes the base MAC in the minimal driver. The driver is
> using the base MAC as its parent ID and without initializing it, it is
> reported as all zeroes to user space.

Applied, thanks.

[PATCH net 0/2] mlxsw: Various fixes

[Ido Schimmel]

Patch #1 fixes the recently introduced QSFP thermal zones to correctly
work with split ports, where several ports are mapped to the same
module.

Patch #2 initializes the base MAC in the minimal driver. The driver is
using the base MAC as its parent ID and without initializing it, it is
reported as all zeroes to user space.

Jiri Pirko (1):
  mlxsw: minimal: Initialize base_mac

Vadim Pasternak (1):
  mlxsw: core: Prevent duplication during QSFP module initialization

 .../ethernet/mellanox/mlxsw/core_thermal.c    | 21 ++++++++++---------
 drivers/net/ethernet/mellanox/mlxsw/minimal.c | 18 ++++++++++++++++
 2 files changed, 29 insertions(+), 10 deletions(-)

-- 
2.20.1