Skip to content

Commit f01032a

Browse files
alobakinkuba-moo
authored andcommitted
idpf: fix memory leaks and crashes while performing a soft reset
The second tagged commit introduced a UAF, as it removed restoring q_vector->vport pointers after reinitializating the structures. This is due to that all queue allocation functions are performed here with the new temporary vport structure and those functions rewrite the backpointers to the vport. Then, this new struct is freed and the pointers start leading to nowhere. But generally speaking, the current logic is very fragile. It claims to be more reliable when the system is low on memory, but in fact, it consumes two times more memory as at the moment of running this function, there are two vports allocated with their queues and vectors. Moreover, it claims to prevent the driver from running into "bad state", but in fact, any error during the rebuild leaves the old vport in the partially allocated state. Finally, if the interface is down when the function is called, it always allocates a new queue set, but when the user decides to enable the interface later on, vport_open() allocates them once again, IOW there's a clear memory leak here. Just don't allocate a new queue set when performing a reset, that solves crashes and memory leaks. Readd the old queue number and reopen the interface on rollback - that solves limbo states when the device is left disabled and/or without HW queues enabled. Fixes: 02cbfba ("idpf: add ethtool callbacks") Fixes: e4891e4 ("idpf: split &idpf_queue into 4 strictly-typed queue structures") Signed-off-by: Alexander Lobakin <[email protected]> Reviewed-by: Simon Horman <[email protected]> Tested-by: Krishneil Singh <[email protected]> Signed-off-by: Tony Nguyen <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent da03f5d commit f01032a

File tree

1 file changed

+15
-15
lines changed

1 file changed

+15
-15
lines changed

drivers/net/ethernet/intel/idpf/idpf_lib.c

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1335,9 +1335,8 @@ static void idpf_rx_init_buf_tail(struct idpf_vport *vport)
13351335
/**
13361336
* idpf_vport_open - Bring up a vport
13371337
* @vport: vport to bring up
1338-
* @alloc_res: allocate queue resources
13391338
*/
1340-
static int idpf_vport_open(struct idpf_vport *vport, bool alloc_res)
1339+
static int idpf_vport_open(struct idpf_vport *vport)
13411340
{
13421341
struct idpf_netdev_priv *np = netdev_priv(vport->netdev);
13431342
struct idpf_adapter *adapter = vport->adapter;
@@ -1350,11 +1349,9 @@ static int idpf_vport_open(struct idpf_vport *vport, bool alloc_res)
13501349
/* we do not allow interface up just yet */
13511350
netif_carrier_off(vport->netdev);
13521351

1353-
if (alloc_res) {
1354-
err = idpf_vport_queues_alloc(vport);
1355-
if (err)
1356-
return err;
1357-
}
1352+
err = idpf_vport_queues_alloc(vport);
1353+
if (err)
1354+
return err;
13581355

13591356
err = idpf_vport_intr_alloc(vport);
13601357
if (err) {
@@ -1539,7 +1536,7 @@ void idpf_init_task(struct work_struct *work)
15391536
np = netdev_priv(vport->netdev);
15401537
np->state = __IDPF_VPORT_DOWN;
15411538
if (test_and_clear_bit(IDPF_VPORT_UP_REQUESTED, vport_config->flags))
1542-
idpf_vport_open(vport, true);
1539+
idpf_vport_open(vport);
15431540

15441541
/* Spawn and return 'idpf_init_task' work queue until all the
15451542
* default vports are created
@@ -1898,9 +1895,6 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport,
18981895
goto free_vport;
18991896
}
19001897

1901-
err = idpf_vport_queues_alloc(new_vport);
1902-
if (err)
1903-
goto free_vport;
19041898
if (current_state <= __IDPF_VPORT_DOWN) {
19051899
idpf_send_delete_queues_msg(vport);
19061900
} else {
@@ -1932,17 +1926,23 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport,
19321926

19331927
err = idpf_set_real_num_queues(vport);
19341928
if (err)
1935-
goto err_reset;
1929+
goto err_open;
19361930

19371931
if (current_state == __IDPF_VPORT_UP)
1938-
err = idpf_vport_open(vport, false);
1932+
err = idpf_vport_open(vport);
19391933

19401934
kfree(new_vport);
19411935

19421936
return err;
19431937

19441938
err_reset:
1945-
idpf_vport_queues_rel(new_vport);
1939+
idpf_send_add_queues_msg(vport, vport->num_txq, vport->num_complq,
1940+
vport->num_rxq, vport->num_bufq);
1941+
1942+
err_open:
1943+
if (current_state == __IDPF_VPORT_UP)
1944+
idpf_vport_open(vport);
1945+
19461946
free_vport:
19471947
kfree(new_vport);
19481948

@@ -2171,7 +2171,7 @@ static int idpf_open(struct net_device *netdev)
21712171
idpf_vport_ctrl_lock(netdev);
21722172
vport = idpf_netdev_to_vport(netdev);
21732173

2174-
err = idpf_vport_open(vport, true);
2174+
err = idpf_vport_open(vport);
21752175

21762176
idpf_vport_ctrl_unlock(netdev);
21772177

0 commit comments

Comments
 (0)