Skip to content

Commit a632b2a

Browse files
agerasymanguy11
authored andcommitted
ice: ethtool: Prohibit improper channel config for DCB
Do not allow setting less channels, than Traffic Classes there are via ethtool. There must be at least one channel per Traffic Class. If you set less channels, than Traffic Classes there are, then during ice_vsi_rebuild there would be allocated only the requested amount of tx/rx rings in ice_vsi_alloc_arrays. But later in ice_vsi_setup_q_map there would be requested at least one channel per Traffic Class. This results in setting num_rxq > alloc_rxq and num_txq > alloc_txq. Later, there would be a NULL pointer dereference in ice_vsi_map_rings_to_vectors, because we go beyond of rx_rings or tx_rings arrays. Change ice_set_channels() to return error if you try to allocate less channels, than Traffic Classes there are. Change ice_vsi_setup_q_map() and ice_vsi_setup_q_map_mqprio() to return status code instead of void. Add error handling for ice_vsi_setup_q_map() and ice_vsi_setup_q_map_mqprio() in ice_vsi_init() and ice_vsi_cfg_tc(). [53753.889983] INFO: Flow control is disabled for this traffic class (0) on this vsi. [53763.984862] BUG: unable to handle kernel NULL pointer dereference at 0000000000000028 [53763.992915] PGD 14b45f5067 P4D 0 [53763.996444] Oops: 0002 [#1] SMP NOPTI [53764.000312] CPU: 12 PID: 30661 Comm: ethtool Kdump: loaded Tainted: GOE --------- - - 4.18.0-240.el8.x86_64 #1 [53764.011825] Hardware name: Intel Corporation WilsonCity/WilsonCity, BIOS WLYDCRB1.SYS.0020.P21.2012150710 12/15/2020 [53764.022584] RIP: 0010:ice_vsi_map_rings_to_vectors+0x7e/0x120 [ice] [53764.029089] Code: 41 0d 0f b7 b7 12 05 00 00 0f b6 d0 44 29 de 44 0f b7 c6 44 01 c2 41 39 d0 7d 2d 4c 8b 47 28 44 0f b7 ce 83 c6 01 4f 8b 04 c8 <49> 89 48 28 4 c 8b 89 b8 01 00 00 4d 89 08 4c 89 81 b8 01 00 00 44 [53764.048379] RSP: 0018:ff550dd88ea47b20 EFLAGS: 00010206 [53764.053884] RAX: 0000000000000002 RBX: 0000000000000004 RCX: ff385ea42fa4a018 [53764.061301] RDX: 0000000000000006 RSI: 0000000000000005 RDI: ff385e9baeedd018 [53764.068717] RBP: 0000000000000010 R08: 0000000000000000 R09: 0000000000000004 [53764.076133] R10: 0000000000000002 R11: 0000000000000004 R12: 0000000000000000 [53764.083553] R13: 0000000000000000 R14: ff385e658fdd9000 R15: ff385e9baeedd018 [53764.090976] FS: 000014872c5b5740(0000) GS:ff385e847f100000(0000) knlGS:0000000000000000 [53764.099362] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [53764.105409] CR2: 0000000000000028 CR3: 0000000a820fa002 CR4: 0000000000761ee0 [53764.112851] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [53764.120301] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [53764.127747] PKRU: 55555554 [53764.130781] Call Trace: [53764.133564] ice_vsi_rebuild+0x611/0x870 [ice] [53764.138341] ice_vsi_recfg_qs+0x94/0x100 [ice] [53764.143116] ice_set_channels+0x1a8/0x3e0 [ice] [53764.147975] ethtool_set_channels+0x14e/0x240 [53764.152667] dev_ethtool+0xd74/0x2a10 [53764.156665] ? __mod_lruvec_state+0x44/0x110 [53764.161280] ? __mod_lruvec_state+0x44/0x110 [53764.165893] ? page_add_file_rmap+0x15/0x170 [53764.170518] ? inet_ioctl+0xd1/0x220 [53764.174445] ? netdev_run_todo+0x5e/0x290 [53764.178808] dev_ioctl+0xb5/0x550 [53764.182485] sock_do_ioctl+0xa0/0x140 [53764.186512] sock_ioctl+0x1a8/0x300 [53764.190367] ? selinux_file_ioctl+0x161/0x200 [53764.195090] do_vfs_ioctl+0xa4/0x640 [53764.199035] ksys_ioctl+0x60/0x90 [53764.202722] __x64_sys_ioctl+0x16/0x20 [53764.206845] do_syscall_64+0x5b/0x1a0 [53764.210887] entry_SYSCALL_64_after_hwframe+0x65/0xca Fixes: 87324e7 ("ice: Implement ethtool ops for channels") Signed-off-by: Anatolii Gerasymenko <[email protected]> Tested-by: Gurucharan <[email protected]> (A Contingent worker at Intel) Signed-off-by: Tony Nguyen <[email protected]>
1 parent c3d184c commit a632b2a

File tree

2 files changed

+47
-5
lines changed

2 files changed

+47
-5
lines changed

drivers/net/ethernet/intel/ice/ice_ethtool.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3507,6 +3507,16 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch)
35073507
new_rx = ch->combined_count + ch->rx_count;
35083508
new_tx = ch->combined_count + ch->tx_count;
35093509

3510+
if (new_rx < vsi->tc_cfg.numtc) {
3511+
netdev_err(dev, "Cannot set less Rx channels, than Traffic Classes you have (%u)\n",
3512+
vsi->tc_cfg.numtc);
3513+
return -EINVAL;
3514+
}
3515+
if (new_tx < vsi->tc_cfg.numtc) {
3516+
netdev_err(dev, "Cannot set less Tx channels, than Traffic Classes you have (%u)\n",
3517+
vsi->tc_cfg.numtc);
3518+
return -EINVAL;
3519+
}
35103520
if (new_rx > ice_get_max_rxq(pf)) {
35113521
netdev_err(dev, "Maximum allowed Rx channels is %d\n",
35123522
ice_get_max_rxq(pf));

drivers/net/ethernet/intel/ice/ice_lib.c

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -909,7 +909,7 @@ static void ice_set_dflt_vsi_ctx(struct ice_hw *hw, struct ice_vsi_ctx *ctxt)
909909
* @vsi: the VSI being configured
910910
* @ctxt: VSI context structure
911911
*/
912-
static void ice_vsi_setup_q_map(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt)
912+
static int ice_vsi_setup_q_map(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt)
913913
{
914914
u16 offset = 0, qmap = 0, tx_count = 0, pow = 0;
915915
u16 num_txq_per_tc, num_rxq_per_tc;
@@ -982,7 +982,18 @@ static void ice_vsi_setup_q_map(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt)
982982
else
983983
vsi->num_rxq = num_rxq_per_tc;
984984

985+
if (vsi->num_rxq > vsi->alloc_rxq) {
986+
dev_err(ice_pf_to_dev(vsi->back), "Trying to use more Rx queues (%u), than were allocated (%u)!\n",
987+
vsi->num_rxq, vsi->alloc_rxq);
988+
return -EINVAL;
989+
}
990+
985991
vsi->num_txq = tx_count;
992+
if (vsi->num_txq > vsi->alloc_txq) {
993+
dev_err(ice_pf_to_dev(vsi->back), "Trying to use more Tx queues (%u), than were allocated (%u)!\n",
994+
vsi->num_txq, vsi->alloc_txq);
995+
return -EINVAL;
996+
}
986997

987998
if (vsi->type == ICE_VSI_VF && vsi->num_txq != vsi->num_rxq) {
988999
dev_dbg(ice_pf_to_dev(vsi->back), "VF VSI should have same number of Tx and Rx queues. Hence making them equal\n");
@@ -1000,6 +1011,8 @@ static void ice_vsi_setup_q_map(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt)
10001011
*/
10011012
ctxt->info.q_mapping[0] = cpu_to_le16(vsi->rxq_map[0]);
10021013
ctxt->info.q_mapping[1] = cpu_to_le16(vsi->num_rxq);
1014+
1015+
return 0;
10031016
}
10041017

10051018
/**
@@ -1187,7 +1200,10 @@ static int ice_vsi_init(struct ice_vsi *vsi, bool init_vsi)
11871200
if (vsi->type == ICE_VSI_CHNL) {
11881201
ice_chnl_vsi_setup_q_map(vsi, ctxt);
11891202
} else {
1190-
ice_vsi_setup_q_map(vsi, ctxt);
1203+
ret = ice_vsi_setup_q_map(vsi, ctxt);
1204+
if (ret)
1205+
goto out;
1206+
11911207
if (!init_vsi) /* means VSI being updated */
11921208
/* must to indicate which section of VSI context are
11931209
* being modified
@@ -3464,7 +3480,7 @@ void ice_vsi_cfg_netdev_tc(struct ice_vsi *vsi, u8 ena_tc)
34643480
*
34653481
* Prepares VSI tc_config to have queue configurations based on MQPRIO options.
34663482
*/
3467-
static void
3483+
static int
34683484
ice_vsi_setup_q_map_mqprio(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt,
34693485
u8 ena_tc)
34703486
{
@@ -3513,7 +3529,18 @@ ice_vsi_setup_q_map_mqprio(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt,
35133529

35143530
/* Set actual Tx/Rx queue pairs */
35153531
vsi->num_txq = offset + qcount_tx;
3532+
if (vsi->num_txq > vsi->alloc_txq) {
3533+
dev_err(ice_pf_to_dev(vsi->back), "Trying to use more Tx queues (%u), than were allocated (%u)!\n",
3534+
vsi->num_txq, vsi->alloc_txq);
3535+
return -EINVAL;
3536+
}
3537+
35163538
vsi->num_rxq = offset + qcount_rx;
3539+
if (vsi->num_rxq > vsi->alloc_rxq) {
3540+
dev_err(ice_pf_to_dev(vsi->back), "Trying to use more Rx queues (%u), than were allocated (%u)!\n",
3541+
vsi->num_rxq, vsi->alloc_rxq);
3542+
return -EINVAL;
3543+
}
35173544

35183545
/* Setup queue TC[0].qmap for given VSI context */
35193546
ctxt->info.tc_mapping[0] = cpu_to_le16(qmap);
@@ -3531,6 +3558,8 @@ ice_vsi_setup_q_map_mqprio(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt,
35313558
dev_dbg(ice_pf_to_dev(vsi->back), "vsi->num_rxq = %d\n", vsi->num_rxq);
35323559
dev_dbg(ice_pf_to_dev(vsi->back), "all_numtc %u, all_enatc: 0x%04x, tc_cfg.numtc %u\n",
35333560
vsi->all_numtc, vsi->all_enatc, vsi->tc_cfg.numtc);
3561+
3562+
return 0;
35343563
}
35353564

35363565
/**
@@ -3580,9 +3609,12 @@ int ice_vsi_cfg_tc(struct ice_vsi *vsi, u8 ena_tc)
35803609

35813610
if (vsi->type == ICE_VSI_PF &&
35823611
test_bit(ICE_FLAG_TC_MQPRIO, pf->flags))
3583-
ice_vsi_setup_q_map_mqprio(vsi, ctx, ena_tc);
3612+
ret = ice_vsi_setup_q_map_mqprio(vsi, ctx, ena_tc);
35843613
else
3585-
ice_vsi_setup_q_map(vsi, ctx);
3614+
ret = ice_vsi_setup_q_map(vsi, ctx);
3615+
3616+
if (ret)
3617+
goto out;
35863618

35873619
/* must to indicate which section of VSI context are being modified */
35883620
ctx->info.valid_sections = cpu_to_le16(ICE_AQ_VSI_PROP_RXQ_MAP_VALID);

0 commit comments

Comments
 (0)