Commit fc21f083 authored by Edward Cree's avatar Edward Cree Committed by Paolo Abeni

sfc: handle error pointers returned by rhashtable_lookup_get_insert_fast()

Several places in TC offload code assumed that the return from
 rhashtable_lookup_get_insert_fast() was always either NULL or a valid
 pointer to an existing entry, but in fact that function can return an
 error pointer.  In that case, perform the usual cleanup of the newly
 created entry, then pass up the error, rather than attempting to take a
 reference on the old entry.

Fixes: d902e1a7 ("sfc: bare bones TC offload on EF100")
Reported-by: default avatarDan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: default avatarEdward Cree <ecree.xilinx@gmail.com>
Link: https://lore.kernel.org/r/20230919183949.59392-1-edward.cree@amd.comSigned-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent 1703b2e0
...@@ -136,6 +136,8 @@ static struct efx_tc_mac_pedit_action *efx_tc_flower_get_mac(struct efx_nic *efx ...@@ -136,6 +136,8 @@ static struct efx_tc_mac_pedit_action *efx_tc_flower_get_mac(struct efx_nic *efx
if (old) { if (old) {
/* don't need our new entry */ /* don't need our new entry */
kfree(ped); kfree(ped);
if (IS_ERR(old)) /* oh dear, it's actually an error */
return ERR_CAST(old);
if (!refcount_inc_not_zero(&old->ref)) if (!refcount_inc_not_zero(&old->ref))
return ERR_PTR(-EAGAIN); return ERR_PTR(-EAGAIN);
/* existing entry found, ref taken */ /* existing entry found, ref taken */
...@@ -602,6 +604,8 @@ static int efx_tc_flower_record_encap_match(struct efx_nic *efx, ...@@ -602,6 +604,8 @@ static int efx_tc_flower_record_encap_match(struct efx_nic *efx,
kfree(encap); kfree(encap);
if (pseudo) /* don't need our new pseudo either */ if (pseudo) /* don't need our new pseudo either */
efx_tc_flower_release_encap_match(efx, pseudo); efx_tc_flower_release_encap_match(efx, pseudo);
if (IS_ERR(old)) /* oh dear, it's actually an error */
return PTR_ERR(old);
/* check old and new em_types are compatible */ /* check old and new em_types are compatible */
switch (old->type) { switch (old->type) {
case EFX_TC_EM_DIRECT: case EFX_TC_EM_DIRECT:
...@@ -700,6 +704,8 @@ static struct efx_tc_recirc_id *efx_tc_get_recirc_id(struct efx_nic *efx, ...@@ -700,6 +704,8 @@ static struct efx_tc_recirc_id *efx_tc_get_recirc_id(struct efx_nic *efx,
if (old) { if (old) {
/* don't need our new entry */ /* don't need our new entry */
kfree(rid); kfree(rid);
if (IS_ERR(old)) /* oh dear, it's actually an error */
return ERR_CAST(old);
if (!refcount_inc_not_zero(&old->ref)) if (!refcount_inc_not_zero(&old->ref))
return ERR_PTR(-EAGAIN); return ERR_PTR(-EAGAIN);
/* existing entry found */ /* existing entry found */
...@@ -1482,7 +1488,10 @@ static int efx_tc_flower_replace_foreign(struct efx_nic *efx, ...@@ -1482,7 +1488,10 @@ static int efx_tc_flower_replace_foreign(struct efx_nic *efx,
old = rhashtable_lookup_get_insert_fast(&efx->tc->match_action_ht, old = rhashtable_lookup_get_insert_fast(&efx->tc->match_action_ht,
&rule->linkage, &rule->linkage,
efx_tc_match_action_ht_params); efx_tc_match_action_ht_params);
if (old) { if (IS_ERR(old)) {
rc = PTR_ERR(old);
goto release;
} else if (old) {
netif_dbg(efx, drv, efx->net_dev, netif_dbg(efx, drv, efx->net_dev,
"Ignoring already-offloaded rule (cookie %lx)\n", "Ignoring already-offloaded rule (cookie %lx)\n",
tc->cookie); tc->cookie);
...@@ -1697,7 +1706,10 @@ static int efx_tc_flower_replace_lhs(struct efx_nic *efx, ...@@ -1697,7 +1706,10 @@ static int efx_tc_flower_replace_lhs(struct efx_nic *efx,
old = rhashtable_lookup_get_insert_fast(&efx->tc->lhs_rule_ht, old = rhashtable_lookup_get_insert_fast(&efx->tc->lhs_rule_ht,
&rule->linkage, &rule->linkage,
efx_tc_lhs_rule_ht_params); efx_tc_lhs_rule_ht_params);
if (old) { if (IS_ERR(old)) {
rc = PTR_ERR(old);
goto release;
} else if (old) {
netif_dbg(efx, drv, efx->net_dev, netif_dbg(efx, drv, efx->net_dev,
"Already offloaded rule (cookie %lx)\n", tc->cookie); "Already offloaded rule (cookie %lx)\n", tc->cookie);
rc = -EEXIST; rc = -EEXIST;
...@@ -1858,7 +1870,10 @@ static int efx_tc_flower_replace(struct efx_nic *efx, ...@@ -1858,7 +1870,10 @@ static int efx_tc_flower_replace(struct efx_nic *efx,
old = rhashtable_lookup_get_insert_fast(&efx->tc->match_action_ht, old = rhashtable_lookup_get_insert_fast(&efx->tc->match_action_ht,
&rule->linkage, &rule->linkage,
efx_tc_match_action_ht_params); efx_tc_match_action_ht_params);
if (old) { if (IS_ERR(old)) {
rc = PTR_ERR(old);
goto release;
} else if (old) {
netif_dbg(efx, drv, efx->net_dev, netif_dbg(efx, drv, efx->net_dev,
"Already offloaded rule (cookie %lx)\n", tc->cookie); "Already offloaded rule (cookie %lx)\n", tc->cookie);
NL_SET_ERR_MSG_MOD(extack, "Rule already offloaded"); NL_SET_ERR_MSG_MOD(extack, "Rule already offloaded");
......
...@@ -298,7 +298,10 @@ static int efx_tc_ct_replace(struct efx_tc_ct_zone *ct_zone, ...@@ -298,7 +298,10 @@ static int efx_tc_ct_replace(struct efx_tc_ct_zone *ct_zone,
old = rhashtable_lookup_get_insert_fast(&efx->tc->ct_ht, old = rhashtable_lookup_get_insert_fast(&efx->tc->ct_ht,
&conn->linkage, &conn->linkage,
efx_tc_ct_ht_params); efx_tc_ct_ht_params);
if (old) { if (IS_ERR(old)) {
rc = PTR_ERR(old);
goto release;
} else if (old) {
netif_dbg(efx, drv, efx->net_dev, netif_dbg(efx, drv, efx->net_dev,
"Already offloaded conntrack (cookie %lx)\n", tc->cookie); "Already offloaded conntrack (cookie %lx)\n", tc->cookie);
rc = -EEXIST; rc = -EEXIST;
...@@ -482,6 +485,8 @@ struct efx_tc_ct_zone *efx_tc_ct_register_zone(struct efx_nic *efx, u16 zone, ...@@ -482,6 +485,8 @@ struct efx_tc_ct_zone *efx_tc_ct_register_zone(struct efx_nic *efx, u16 zone,
if (old) { if (old) {
/* don't need our new entry */ /* don't need our new entry */
kfree(ct_zone); kfree(ct_zone);
if (IS_ERR(old)) /* oh dear, it's actually an error */
return ERR_CAST(old);
if (!refcount_inc_not_zero(&old->ref)) if (!refcount_inc_not_zero(&old->ref))
return ERR_PTR(-EAGAIN); return ERR_PTR(-EAGAIN);
/* existing entry found */ /* existing entry found */
......
...@@ -236,6 +236,8 @@ struct efx_tc_counter_index *efx_tc_flower_get_counter_index( ...@@ -236,6 +236,8 @@ struct efx_tc_counter_index *efx_tc_flower_get_counter_index(
if (old) { if (old) {
/* don't need our new entry */ /* don't need our new entry */
kfree(ctr); kfree(ctr);
if (IS_ERR(old)) /* oh dear, it's actually an error */
return ERR_CAST(old);
if (!refcount_inc_not_zero(&old->ref)) if (!refcount_inc_not_zero(&old->ref))
return ERR_PTR(-EAGAIN); return ERR_PTR(-EAGAIN);
/* existing entry found */ /* existing entry found */
......
...@@ -132,6 +132,8 @@ static int efx_bind_neigh(struct efx_nic *efx, ...@@ -132,6 +132,8 @@ static int efx_bind_neigh(struct efx_nic *efx,
/* don't need our new entry */ /* don't need our new entry */
put_net_track(neigh->net, &neigh->ns_tracker); put_net_track(neigh->net, &neigh->ns_tracker);
kfree(neigh); kfree(neigh);
if (IS_ERR(old)) /* oh dear, it's actually an error */
return PTR_ERR(old);
if (!refcount_inc_not_zero(&old->ref)) if (!refcount_inc_not_zero(&old->ref))
return -EAGAIN; return -EAGAIN;
/* existing entry found, ref taken */ /* existing entry found, ref taken */
...@@ -640,6 +642,8 @@ struct efx_tc_encap_action *efx_tc_flower_create_encap_md( ...@@ -640,6 +642,8 @@ struct efx_tc_encap_action *efx_tc_flower_create_encap_md(
if (old) { if (old) {
/* don't need our new entry */ /* don't need our new entry */
kfree(encap); kfree(encap);
if (IS_ERR(old)) /* oh dear, it's actually an error */
return ERR_CAST(old);
if (!refcount_inc_not_zero(&old->ref)) if (!refcount_inc_not_zero(&old->ref))
return ERR_PTR(-EAGAIN); return ERR_PTR(-EAGAIN);
/* existing entry found, ref taken */ /* existing entry found, ref taken */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment