* [PATCH net 1/3] net/mlx4_core: Fix reset flow when in command polling mode
2019-03-12 15:05 [PATCH net 0/3] mlx4_core misc fixes Tariq Toukan
@ 2019-03-12 15:05 ` Tariq Toukan
2019-03-12 15:05 ` [PATCH net 2/3] net/mlx4_core: Fix locking in SRIOV mode when switching between events and polling Tariq Toukan
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Tariq Toukan @ 2019-03-12 15:05 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Jack Morgenstein, Tariq Toukan
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
As part of unloading a device, the driver switches from
FW command event mode to FW command polling mode.
Part of switching over to polling mode is freeing the command context array
memory (unfortunately, currently, without NULLing the command context array
pointer).
The reset flow calls "complete" to complete all outstanding fw commands
(if we are in event mode). The check for event vs. polling mode here
is to test if the command context array pointer is NULL.
If the reset flow is activated after the switch to polling mode, it will
attempt (incorrectly) to complete all the commands in the context array --
because the pointer was not NULLed when the driver switched over to polling
mode.
As a result, we have a use-after-free situation, which results in a
kernel crash.
For example:
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffff876c4a8e>] __wake_up_common+0x2e/0x90
PGD 0
Oops: 0000 [#1] SMP
Modules linked in: netconsole nfsv3 nfs_acl nfs lockd grace ...
CPU: 2 PID: 940 Comm: kworker/2:3 Kdump: loaded Not tainted 3.10.0-862.el7.x86_64 #1
Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS 090006 04/28/2016
Workqueue: events hv_eject_device_work [pci_hyperv]
task: ffff8d1734ca0fd0 ti: ffff8d17354bc000 task.ti: ffff8d17354bc000
RIP: 0010:[<ffffffff876c4a8e>] [<ffffffff876c4a8e>] __wake_up_common+0x2e/0x90
RSP: 0018:ffff8d17354bfa38 EFLAGS: 00010082
RAX: 0000000000000000 RBX: ffff8d17362d42c8 RCX: 0000000000000000
RDX: 0000000000000001 RSI: 0000000000000003 RDI: ffff8d17362d42c8
RBP: ffff8d17354bfa70 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000298 R11: ffff8d173610e000 R12: ffff8d17362d42d0
R13: 0000000000000246 R14: 0000000000000000 R15: 0000000000000003
FS: 0000000000000000(0000) GS:ffff8d1802680000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000000f16d8000 CR4: 00000000001406e0
Call Trace:
[<ffffffff876c7adc>] complete+0x3c/0x50
[<ffffffffc04242f0>] mlx4_cmd_wake_completions+0x70/0x90 [mlx4_core]
[<ffffffffc041e7b1>] mlx4_enter_error_state+0xe1/0x380 [mlx4_core]
[<ffffffffc041fa4b>] mlx4_comm_cmd+0x29b/0x360 [mlx4_core]
[<ffffffffc041ff51>] __mlx4_cmd+0x441/0x920 [mlx4_core]
[<ffffffff877f62b1>] ? __slab_free+0x81/0x2f0
[<ffffffff87951384>] ? __radix_tree_lookup+0x84/0xf0
[<ffffffffc043a8eb>] mlx4_free_mtt_range+0x5b/0xb0 [mlx4_core]
[<ffffffffc043a957>] mlx4_mtt_cleanup+0x17/0x20 [mlx4_core]
[<ffffffffc04272c7>] mlx4_free_eq+0xa7/0x1c0 [mlx4_core]
[<ffffffffc042803e>] mlx4_cleanup_eq_table+0xde/0x130 [mlx4_core]
[<ffffffffc0433e08>] mlx4_unload_one+0x118/0x300 [mlx4_core]
[<ffffffffc0434191>] mlx4_remove_one+0x91/0x1f0 [mlx4_core]
The fix is to set the command context array pointer to NULL after freeing
the array.
Fixes: f5aef5aa3506 ("net/mlx4_core: Activate reset flow upon fatal command cases")
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/cmd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index c19e74e6ac94..44081acd48fb 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -2689,6 +2689,7 @@ void mlx4_cmd_use_polling(struct mlx4_dev *dev)
down(&priv->cmd.event_sem);
kfree(priv->cmd.context);
+ priv->cmd.context = NULL;
up(&priv->cmd.poll_sem);
up_write(&priv->cmd.switch_sem);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH net 2/3] net/mlx4_core: Fix locking in SRIOV mode when switching between events and polling
2019-03-12 15:05 [PATCH net 0/3] mlx4_core misc fixes Tariq Toukan
2019-03-12 15:05 ` [PATCH net 1/3] net/mlx4_core: Fix reset flow when in command polling mode Tariq Toukan
@ 2019-03-12 15:05 ` Tariq Toukan
2019-03-12 15:05 ` [PATCH net 3/3] net/mlx4_core: Fix qp mtt size calculation Tariq Toukan
2019-03-12 22:01 ` [PATCH net 0/3] mlx4_core misc fixes David Miller
3 siblings, 0 replies; 5+ messages in thread
From: Tariq Toukan @ 2019-03-12 15:05 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Jack Morgenstein, Tariq Toukan
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
In procedures mlx4_cmd_use_events() and mlx4_cmd_use_polling(), we need to
guarantee that there are no FW commands in progress on the comm channel
(for VFs) or wrapped FW commands (on the PF) when SRIOV is active.
We do this by also taking the slave_cmd_mutex when SRIOV is active.
This is especially important when switching from event to polling, since we
free the command-context array during the switch. If there are FW commands
in progress (e.g., waiting for a completion event), the completion event
handler will access freed memory.
Since the decision to use comm_wait or comm_poll is taken before grabbing
the event_sem/poll_sem in mlx4_comm_cmd_wait/poll, we must take the
slave_cmd_mutex as well (to guarantee that the decision to use events or
polling and the call to the appropriate cmd function are atomic).
Fixes: a7e1f04905e5 ("net/mlx4_core: Fix deadlock when switching between polling and event fw commands")
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/cmd.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 44081acd48fb..a5d5d6fc1da0 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -2645,6 +2645,8 @@ int mlx4_cmd_use_events(struct mlx4_dev *dev)
if (!priv->cmd.context)
return -ENOMEM;
+ if (mlx4_is_mfunc(dev))
+ mutex_lock(&priv->cmd.slave_cmd_mutex);
down_write(&priv->cmd.switch_sem);
for (i = 0; i < priv->cmd.max_cmds; ++i) {
priv->cmd.context[i].token = i;
@@ -2670,6 +2672,8 @@ int mlx4_cmd_use_events(struct mlx4_dev *dev)
down(&priv->cmd.poll_sem);
priv->cmd.use_events = 1;
up_write(&priv->cmd.switch_sem);
+ if (mlx4_is_mfunc(dev))
+ mutex_unlock(&priv->cmd.slave_cmd_mutex);
return err;
}
@@ -2682,6 +2686,8 @@ void mlx4_cmd_use_polling(struct mlx4_dev *dev)
struct mlx4_priv *priv = mlx4_priv(dev);
int i;
+ if (mlx4_is_mfunc(dev))
+ mutex_lock(&priv->cmd.slave_cmd_mutex);
down_write(&priv->cmd.switch_sem);
priv->cmd.use_events = 0;
@@ -2693,6 +2699,8 @@ void mlx4_cmd_use_polling(struct mlx4_dev *dev)
up(&priv->cmd.poll_sem);
up_write(&priv->cmd.switch_sem);
+ if (mlx4_is_mfunc(dev))
+ mutex_unlock(&priv->cmd.slave_cmd_mutex);
}
struct mlx4_cmd_mailbox *mlx4_alloc_cmd_mailbox(struct mlx4_dev *dev)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH net 3/3] net/mlx4_core: Fix qp mtt size calculation
2019-03-12 15:05 [PATCH net 0/3] mlx4_core misc fixes Tariq Toukan
2019-03-12 15:05 ` [PATCH net 1/3] net/mlx4_core: Fix reset flow when in command polling mode Tariq Toukan
2019-03-12 15:05 ` [PATCH net 2/3] net/mlx4_core: Fix locking in SRIOV mode when switching between events and polling Tariq Toukan
@ 2019-03-12 15:05 ` Tariq Toukan
2019-03-12 22:01 ` [PATCH net 0/3] mlx4_core misc fixes David Miller
3 siblings, 0 replies; 5+ messages in thread
From: Tariq Toukan @ 2019-03-12 15:05 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Jack Morgenstein, Tariq Toukan
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
Calculation of qp mtt size (in function mlx4_RST2INIT_wrapper)
ultimately depends on function roundup_pow_of_two.
If the amount of memory required by the QP is less than one page,
roundup_pow_of_two is called with argument zero. In this case, the
roundup_pow_of_two result is undefined.
Calling roundup_pow_of_two with a zero argument resulted in the
following stack trace:
UBSAN: Undefined behaviour in ./include/linux/log2.h:61:13
shift exponent 64 is too large for 64-bit type 'long unsigned int'
CPU: 4 PID: 26939 Comm: rping Tainted: G OE 4.19.0-rc1
Hardware name: Supermicro X9DR3-F/X9DR3-F, BIOS 3.2a 07/09/2015
Call Trace:
dump_stack+0x9a/0xeb
ubsan_epilogue+0x9/0x7c
__ubsan_handle_shift_out_of_bounds+0x254/0x29d
? __ubsan_handle_load_invalid_value+0x180/0x180
? debug_show_all_locks+0x310/0x310
? sched_clock+0x5/0x10
? sched_clock+0x5/0x10
? sched_clock_cpu+0x18/0x260
? find_held_lock+0x35/0x1e0
? mlx4_RST2INIT_QP_wrapper+0xfb1/0x1440 [mlx4_core]
mlx4_RST2INIT_QP_wrapper+0xfb1/0x1440 [mlx4_core]
Fix this by explicitly testing for zero, and returning one if the
argument is zero (assuming that the next higher power of 2 in this case
should be one).
Fixes: c82e9aa0a8bc ("mlx4_core: resource tracking for HCA resources used by guests")
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index eb13d3618162..4356f3a58002 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -2719,13 +2719,13 @@ static int qp_get_mtt_size(struct mlx4_qp_context *qpc)
int total_pages;
int total_mem;
int page_offset = (be32_to_cpu(qpc->params2) >> 6) & 0x3f;
+ int tot;
sq_size = 1 << (log_sq_size + log_sq_sride + 4);
rq_size = (srq|rss|xrc) ? 0 : (1 << (log_rq_size + log_rq_stride + 4));
total_mem = sq_size + rq_size;
- total_pages =
- roundup_pow_of_two((total_mem + (page_offset << 6)) >>
- page_shift);
+ tot = (total_mem + (page_offset << 6)) >> page_shift;
+ total_pages = !tot ? 1 : roundup_pow_of_two(tot);
return total_pages;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net 0/3] mlx4_core misc fixes
2019-03-12 15:05 [PATCH net 0/3] mlx4_core misc fixes Tariq Toukan
` (2 preceding siblings ...)
2019-03-12 15:05 ` [PATCH net 3/3] net/mlx4_core: Fix qp mtt size calculation Tariq Toukan
@ 2019-03-12 22:01 ` David Miller
3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2019-03-12 22:01 UTC (permalink / raw)
To: tariqt; +Cc: netdev, eranbe
From: Tariq Toukan <tariqt@mellanox.com>
Date: Tue, 12 Mar 2019 17:05:46 +0200
> This patchset by Jack contains misc fixes to the mlx4 Core driver.
>
> Patch 1 fixes a use-after-free situation by marking (nullifying) the pointer,
> please queue for -stable >= v4.0.
> Patch 2 adds a missing lock acquire and release in SRIOV command interface,
> please queue for -stable >= v4.9.
> Patch 3 avoids calling roundup_pow_of_two when argument is zero,
> please queue for -stable >= v3.3.
>
> Series generated against net commit:
> a3b1933d34d5 Merge tag 'mlx5-fixes-2019-03-11' of
> git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux
Series applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread