Skip to content

Commit 74f9d55

Browse files
committed
Merge branch '40GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue
Tony Nguyen says: ==================== virtchnl: fix fake 1-elem arrays Alexander Lobakin says: 6.5-rc1 started spitting warning splats when composing virtchnl messages, precisely on virtchnl_rss_key and virtchnl_lut: [ 84.167709] memcpy: detected field-spanning write (size 52) of single field "vrk->key" at drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1095 (size 1) [ 84.169915] WARNING: CPU: 3 PID: 11 at drivers/net/ethernet/intel/ iavf/iavf_virtchnl.c:1095 iavf_set_rss_key+0x123/0x140 [iavf] ... [ 84.191982] Call Trace: [ 84.192439] <TASK> [ 84.192900] ? __warn+0xc9/0x1a0 [ 84.193353] ? iavf_set_rss_key+0x123/0x140 [iavf] [ 84.193818] ? report_bug+0x12c/0x1b0 [ 84.194266] ? handle_bug+0x42/0x70 [ 84.194714] ? exc_invalid_op+0x1a/0x50 [ 84.195149] ? asm_exc_invalid_op+0x1a/0x20 [ 84.195592] ? iavf_set_rss_key+0x123/0x140 [iavf] [ 84.196033] iavf_watchdog_task+0xb0c/0xe00 [iavf] ... [ 84.225476] memcpy: detected field-spanning write (size 64) of single field "vrl->lut" at drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1127 (size 1) [ 84.227190] WARNING: CPU: 27 PID: 1044 at drivers/net/ethernet/intel/ iavf/iavf_virtchnl.c:1127 iavf_set_rss_lut+0x123/0x140 [iavf] ... [ 84.246601] Call Trace: [ 84.247228] <TASK> [ 84.247840] ? __warn+0xc9/0x1a0 [ 84.248263] ? iavf_set_rss_lut+0x123/0x140 [iavf] [ 84.248698] ? report_bug+0x12c/0x1b0 [ 84.249122] ? handle_bug+0x42/0x70 [ 84.249549] ? exc_invalid_op+0x1a/0x50 [ 84.249970] ? asm_exc_invalid_op+0x1a/0x20 [ 84.250390] ? iavf_set_rss_lut+0x123/0x140 [iavf] [ 84.250820] iavf_watchdog_task+0xb16/0xe00 [iavf] Gustavo already tried to fix those back in 2021[0][1]. Unfortunately, a VM can run a different kernel than the host, meaning that those structures are sorta ABI. However, it is possible to have proper flex arrays + struct_size() calculations and still send the very same messages with the same sizes. The common rule is: elem[1] -> elem[] size = struct_size() + <difference between the old and the new msg size> The "old" size in the current code is calculated 3 different ways for 10 virtchnl structures total. Each commit addresses one of the ways cumulatively instead of per-structure. I was planning to send it to -net initially, but given that virtchnl was renamed from i40evf and got some fat style cleanup commits in the past, it's not very straightforward to even pick appropriate SHAs, not speaking of automatic portability. I may send manual backports for a couple of the latest supported kernels later on if anyone needs it at all. [0] https://lore.kernel.org/all/20210525230912.GA175802@embeddedor [1] https://lore.kernel.org/all/20210525231851.GA176647@embeddedor * '40GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue: virtchnl: fix fake 1-elem arrays for structures allocated as `nents` virtchnl: fix fake 1-elem arrays in structures allocated as `nents + 1` virtchnl: fix fake 1-elem arrays in structs allocated as `nents + 1` - 1 ==================== Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
2 parents 04f2840 + b0654e6 commit 74f9d55

File tree

7 files changed

+124
-101
lines changed

7 files changed

+124
-101
lines changed

drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,7 @@ i40e_config_rdma_qvlist(struct i40e_vf *vf,
506506
struct virtchnl_rdma_qv_info *qv_info;
507507
u32 v_idx, i, reg_idx, reg;
508508
u32 next_q_idx, next_q_type;
509+
size_t size;
509510
u32 msix_vf;
510511
int ret = 0;
511512

@@ -521,9 +522,9 @@ i40e_config_rdma_qvlist(struct i40e_vf *vf,
521522
}
522523

523524
kfree(vf->qvlist_info);
524-
vf->qvlist_info = kzalloc(struct_size(vf->qvlist_info, qv_info,
525-
qvlist_info->num_vectors - 1),
526-
GFP_KERNEL);
525+
size = virtchnl_struct_size(vf->qvlist_info, qv_info,
526+
qvlist_info->num_vectors);
527+
vf->qvlist_info = kzalloc(size, GFP_KERNEL);
527528
if (!vf->qvlist_info) {
528529
ret = -ENOMEM;
529530
goto err_out;
@@ -2103,7 +2104,7 @@ static int i40e_vc_get_vf_resources_msg(struct i40e_vf *vf, u8 *msg)
21032104
goto err;
21042105
}
21052106

2106-
len = struct_size(vfres, vsi_res, num_vsis);
2107+
len = virtchnl_struct_size(vfres, vsi_res, num_vsis);
21072108
vfres = kzalloc(len, GFP_KERNEL);
21082109
if (!vfres) {
21092110
aq_ret = -ENOMEM;

drivers/net/ethernet/intel/iavf/iavf.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,9 @@ struct iavf_vsi {
9292
#define IAVF_MBPS_DIVISOR 125000 /* divisor to convert to Mbps */
9393
#define IAVF_MBPS_QUANTA 50
9494

95-
#define IAVF_VIRTCHNL_VF_RESOURCE_SIZE (sizeof(struct virtchnl_vf_resource) + \
96-
(IAVF_MAX_VF_VSI * \
97-
sizeof(struct virtchnl_vsi_resource)))
95+
#define IAVF_VIRTCHNL_VF_RESOURCE_SIZE \
96+
virtchnl_struct_size((struct virtchnl_vf_resource *)NULL, \
97+
vsi_res, IAVF_MAX_VF_VSI)
9898

9999
/* MAX_MSIX_Q_VECTORS of these are allocated,
100100
* but we only use one per queue-specific vector.

drivers/net/ethernet/intel/iavf/iavf_client.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,8 +469,8 @@ static int iavf_client_setup_qvlist(struct iavf_info *ldev,
469469
}
470470

471471
v_qvlist_info = (struct virtchnl_rdma_qvlist_info *)qvlist_info;
472-
msg_size = struct_size(v_qvlist_info, qv_info,
473-
v_qvlist_info->num_vectors - 1);
472+
msg_size = virtchnl_struct_size(v_qvlist_info, qv_info,
473+
v_qvlist_info->num_vectors);
474474

475475
adapter->client_pending |= BIT(VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP);
476476
err = iavf_aq_send_msg_to_pf(&adapter->hw,

drivers/net/ethernet/intel/iavf/iavf_client.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ struct iavf_qv_info {
5353

5454
struct iavf_qvlist_info {
5555
u32 num_vectors;
56-
struct iavf_qv_info qv_info[1];
56+
struct iavf_qv_info qv_info[];
5757
};
5858

5959
#define IAVF_CLIENT_MSIX_ALL 0xFFFFFFFF

drivers/net/ethernet/intel/iavf/iavf_virtchnl.c

Lines changed: 30 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,7 @@ int iavf_get_vf_config(struct iavf_adapter *adapter)
215215
u16 len;
216216
int err;
217217

218-
len = sizeof(struct virtchnl_vf_resource) +
219-
IAVF_MAX_VF_VSI * sizeof(struct virtchnl_vsi_resource);
218+
len = IAVF_VIRTCHNL_VF_RESOURCE_SIZE;
220219
event.buf_len = len;
221220
event.msg_buf = kzalloc(len, GFP_KERNEL);
222221
if (!event.msg_buf)
@@ -284,7 +283,7 @@ void iavf_configure_queues(struct iavf_adapter *adapter)
284283
return;
285284
}
286285
adapter->current_op = VIRTCHNL_OP_CONFIG_VSI_QUEUES;
287-
len = struct_size(vqci, qpair, pairs);
286+
len = virtchnl_struct_size(vqci, qpair, pairs);
288287
vqci = kzalloc(len, GFP_KERNEL);
289288
if (!vqci)
290289
return;
@@ -397,7 +396,7 @@ void iavf_map_queues(struct iavf_adapter *adapter)
397396

398397
q_vectors = adapter->num_msix_vectors - NONQ_VECS;
399398

400-
len = struct_size(vimi, vecmap, adapter->num_msix_vectors);
399+
len = virtchnl_struct_size(vimi, vecmap, adapter->num_msix_vectors);
401400
vimi = kzalloc(len, GFP_KERNEL);
402401
if (!vimi)
403402
return;
@@ -476,13 +475,11 @@ void iavf_add_ether_addrs(struct iavf_adapter *adapter)
476475
}
477476
adapter->current_op = VIRTCHNL_OP_ADD_ETH_ADDR;
478477

479-
len = struct_size(veal, list, count);
478+
len = virtchnl_struct_size(veal, list, count);
480479
if (len > IAVF_MAX_AQ_BUF_SIZE) {
481480
dev_warn(&adapter->pdev->dev, "Too many add MAC changes in one request\n");
482-
count = (IAVF_MAX_AQ_BUF_SIZE -
483-
sizeof(struct virtchnl_ether_addr_list)) /
484-
sizeof(struct virtchnl_ether_addr);
485-
len = struct_size(veal, list, count);
481+
while (len > IAVF_MAX_AQ_BUF_SIZE)
482+
len = virtchnl_struct_size(veal, list, --count);
486483
more = true;
487484
}
488485

@@ -547,13 +544,11 @@ void iavf_del_ether_addrs(struct iavf_adapter *adapter)
547544
}
548545
adapter->current_op = VIRTCHNL_OP_DEL_ETH_ADDR;
549546

550-
len = struct_size(veal, list, count);
547+
len = virtchnl_struct_size(veal, list, count);
551548
if (len > IAVF_MAX_AQ_BUF_SIZE) {
552549
dev_warn(&adapter->pdev->dev, "Too many delete MAC changes in one request\n");
553-
count = (IAVF_MAX_AQ_BUF_SIZE -
554-
sizeof(struct virtchnl_ether_addr_list)) /
555-
sizeof(struct virtchnl_ether_addr);
556-
len = struct_size(veal, list, count);
550+
while (len > IAVF_MAX_AQ_BUF_SIZE)
551+
len = virtchnl_struct_size(veal, list, --count);
557552
more = true;
558553
}
559554
veal = kzalloc(len, GFP_ATOMIC);
@@ -687,12 +682,12 @@ void iavf_add_vlans(struct iavf_adapter *adapter)
687682

688683
adapter->current_op = VIRTCHNL_OP_ADD_VLAN;
689684

690-
len = sizeof(*vvfl) + (count * sizeof(u16));
685+
len = virtchnl_struct_size(vvfl, vlan_id, count);
691686
if (len > IAVF_MAX_AQ_BUF_SIZE) {
692687
dev_warn(&adapter->pdev->dev, "Too many add VLAN changes in one request\n");
693-
count = (IAVF_MAX_AQ_BUF_SIZE - sizeof(*vvfl)) /
694-
sizeof(u16);
695-
len = sizeof(*vvfl) + (count * sizeof(u16));
688+
while (len > IAVF_MAX_AQ_BUF_SIZE)
689+
len = virtchnl_struct_size(vvfl, vlan_id,
690+
--count);
696691
more = true;
697692
}
698693
vvfl = kzalloc(len, GFP_ATOMIC);
@@ -732,15 +727,12 @@ void iavf_add_vlans(struct iavf_adapter *adapter)
732727
more = true;
733728
}
734729

735-
len = sizeof(*vvfl_v2) + ((count - 1) *
736-
sizeof(struct virtchnl_vlan_filter));
730+
len = virtchnl_struct_size(vvfl_v2, filters, count);
737731
if (len > IAVF_MAX_AQ_BUF_SIZE) {
738732
dev_warn(&adapter->pdev->dev, "Too many add VLAN changes in one request\n");
739-
count = (IAVF_MAX_AQ_BUF_SIZE - sizeof(*vvfl_v2)) /
740-
sizeof(struct virtchnl_vlan_filter);
741-
len = sizeof(*vvfl_v2) +
742-
((count - 1) *
743-
sizeof(struct virtchnl_vlan_filter));
733+
while (len > IAVF_MAX_AQ_BUF_SIZE)
734+
len = virtchnl_struct_size(vvfl_v2, filters,
735+
--count);
744736
more = true;
745737
}
746738

@@ -838,12 +830,12 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
838830

839831
adapter->current_op = VIRTCHNL_OP_DEL_VLAN;
840832

841-
len = sizeof(*vvfl) + (count * sizeof(u16));
833+
len = virtchnl_struct_size(vvfl, vlan_id, count);
842834
if (len > IAVF_MAX_AQ_BUF_SIZE) {
843835
dev_warn(&adapter->pdev->dev, "Too many delete VLAN changes in one request\n");
844-
count = (IAVF_MAX_AQ_BUF_SIZE - sizeof(*vvfl)) /
845-
sizeof(u16);
846-
len = sizeof(*vvfl) + (count * sizeof(u16));
836+
while (len > IAVF_MAX_AQ_BUF_SIZE)
837+
len = virtchnl_struct_size(vvfl, vlan_id,
838+
--count);
847839
more = true;
848840
}
849841
vvfl = kzalloc(len, GFP_ATOMIC);
@@ -884,16 +876,12 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
884876

885877
adapter->current_op = VIRTCHNL_OP_DEL_VLAN_V2;
886878

887-
len = sizeof(*vvfl_v2) +
888-
((count - 1) * sizeof(struct virtchnl_vlan_filter));
879+
len = virtchnl_struct_size(vvfl_v2, filters, count);
889880
if (len > IAVF_MAX_AQ_BUF_SIZE) {
890881
dev_warn(&adapter->pdev->dev, "Too many add VLAN changes in one request\n");
891-
count = (IAVF_MAX_AQ_BUF_SIZE -
892-
sizeof(*vvfl_v2)) /
893-
sizeof(struct virtchnl_vlan_filter);
894-
len = sizeof(*vvfl_v2) +
895-
((count - 1) *
896-
sizeof(struct virtchnl_vlan_filter));
882+
while (len > IAVF_MAX_AQ_BUF_SIZE)
883+
len = virtchnl_struct_size(vvfl_v2, filters,
884+
--count);
897885
more = true;
898886
}
899887

@@ -1085,8 +1073,7 @@ void iavf_set_rss_key(struct iavf_adapter *adapter)
10851073
adapter->current_op);
10861074
return;
10871075
}
1088-
len = sizeof(struct virtchnl_rss_key) +
1089-
(adapter->rss_key_size * sizeof(u8)) - 1;
1076+
len = virtchnl_struct_size(vrk, key, adapter->rss_key_size);
10901077
vrk = kzalloc(len, GFP_KERNEL);
10911078
if (!vrk)
10921079
return;
@@ -1117,8 +1104,7 @@ void iavf_set_rss_lut(struct iavf_adapter *adapter)
11171104
adapter->current_op);
11181105
return;
11191106
}
1120-
len = sizeof(struct virtchnl_rss_lut) +
1121-
(adapter->rss_lut_size * sizeof(u8)) - 1;
1107+
len = virtchnl_struct_size(vrl, lut, adapter->rss_lut_size);
11221108
vrl = kzalloc(len, GFP_KERNEL);
11231109
if (!vrl)
11241110
return;
@@ -1499,7 +1485,7 @@ void iavf_enable_channels(struct iavf_adapter *adapter)
14991485
return;
15001486
}
15011487

1502-
len = struct_size(vti, list, adapter->num_tc - 1);
1488+
len = virtchnl_struct_size(vti, list, adapter->num_tc);
15031489
vti = kzalloc(len, GFP_KERNEL);
15041490
if (!vti)
15051491
return;
@@ -2175,9 +2161,8 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
21752161
}
21762162
break;
21772163
case VIRTCHNL_OP_GET_VF_RESOURCES: {
2178-
u16 len = sizeof(struct virtchnl_vf_resource) +
2179-
IAVF_MAX_VF_VSI *
2180-
sizeof(struct virtchnl_vsi_resource);
2164+
u16 len = IAVF_VIRTCHNL_VF_RESOURCE_SIZE;
2165+
21812166
memcpy(adapter->vf_res, msg, min(msglen, len));
21822167
iavf_validate_num_queues(adapter);
21832168
iavf_vf_parse_hw_config(&adapter->hw, adapter->vf_res);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ static int ice_vc_get_vf_res_msg(struct ice_vf *vf, u8 *msg)
428428
goto err;
429429
}
430430

431-
len = sizeof(struct virtchnl_vf_resource);
431+
len = virtchnl_struct_size(vfres, vsi_res, 0);
432432

433433
vfres = kzalloc(len, GFP_KERNEL);
434434
if (!vfres) {

0 commit comments

Comments
 (0)