[PATCH net-next] nfp: flower: fix cb_ident duplicate in indirect block register

[PATCH net-next] nfp: flower: fix cb_ident duplicate in indirect block register

[Jakub Kicinski]

From: John Hurley <john.hurley@netronome.com>

Previously the identifier used for indirect block callback registry and
for block rule cb registry (when done via indirect blocks) was the pointer
to the netdev we were interested in receiving updates on. This worked fine
if a single app existed that registered one callback per netdev of
interest. However, if multiple cards are in place and, in turn, multiple
apps, then each app may register the same callback with the same
identifier to both the netdev's indirect block cb list and to a block's cb
list. This can lead to EEXIST errors and/or incorrect cb deletions.

Prevent this conflict by using the app pointer as the identifier for
netdev indirect block cb registry, allowing each app to register a unique
callback per netdev. For block cb registry, the same app may register
multiple cbs to the same block if using TC shared blocks. Instead of the
app, use the pointer to the allocated cb_priv data as the identifier here.
This means that there can be a unique block callback for each app/netdev
combo.

Fixes: 3166dd07a9cb ("nfp: flower: offload tunnel decap rules via indirect TC blocks")
Reported-by: Edward Cree <ecree@solarflare.com>
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../ethernet/netronome/nfp/flower/offload.c   | 21 ++++++++++---------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 21499a5b3b6b..c642fd84eb02 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -718,7 +718,7 @@ nfp_flower_setup_indr_tc_block(struct net_device *netdev, struct nfp_app *app,
 
 		err = tcf_block_cb_register(f->block,
 					    nfp_flower_setup_indr_block_cb,
-					    netdev, cb_priv, f->extack);
+					    cb_priv, cb_priv, f->extack);
 		if (err) {
 			list_del(&cb_priv->list);
 			kfree(cb_priv);
@@ -726,13 +726,15 @@ nfp_flower_setup_indr_tc_block(struct net_device *netdev, struct nfp_app *app,
 
 		return err;
 	case TC_BLOCK_UNBIND:
-		tcf_block_cb_unregister(f->block,
-					nfp_flower_setup_indr_block_cb, netdev);
 		cb_priv = nfp_flower_indr_block_cb_priv_lookup(app, netdev);
-		if (cb_priv) {
-			list_del(&cb_priv->list);
-			kfree(cb_priv);
-		}
+		if (!cb_priv)
+			return -ENOENT;
+
+		tcf_block_cb_unregister(f->block,
+					nfp_flower_setup_indr_block_cb,
+					cb_priv);
+		list_del(&cb_priv->list);
+		kfree(cb_priv);
 
 		return 0;
 	default:
@@ -766,15 +768,14 @@ int nfp_flower_reg_indir_block_handler(struct nfp_app *app,
 	if (event == NETDEV_REGISTER) {
 		err = __tc_indr_block_cb_register(netdev, app,
 						  nfp_flower_indr_setup_tc_cb,
-						  netdev);
+						  app);
 		if (err)
 			nfp_flower_cmsg_warn(app,
 					     "Indirect block reg failed - %s\n",
 					     netdev->name);
 	} else if (event == NETDEV_UNREGISTER) {
 		__tc_indr_block_cb_unregister(netdev,
-					      nfp_flower_indr_setup_tc_cb,
-					      netdev);
+					      nfp_flower_indr_setup_tc_cb, app);
 	}
 
 	return NOTIFY_OK;
-- 
2.19.2

Re: [PATCH net-next] nfp: flower: fix cb_ident duplicate in indirect block register

[David Miller]

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 17 Dec 2018 19:18:39 -0800

> From: John Hurley <john.hurley@netronome.com>
> 
> Previously the identifier used for indirect block callback registry and
> for block rule cb registry (when done via indirect blocks) was the pointer
> to the netdev we were interested in receiving updates on. This worked fine
> if a single app existed that registered one callback per netdev of
> interest. However, if multiple cards are in place and, in turn, multiple
> apps, then each app may register the same callback with the same
> identifier to both the netdev's indirect block cb list and to a block's cb
> list. This can lead to EEXIST errors and/or incorrect cb deletions.
> 
> Prevent this conflict by using the app pointer as the identifier for
> netdev indirect block cb registry, allowing each app to register a unique
> callback per netdev. For block cb registry, the same app may register
> multiple cbs to the same block if using TC shared blocks. Instead of the
> app, use the pointer to the allocated cb_priv data as the identifier here.
> This means that there can be a unique block callback for each app/netdev
> combo.
> 
> Fixes: 3166dd07a9cb ("nfp: flower: offload tunnel decap rules via indirect TC blocks")
> Reported-by: Edward Cree <ecree@solarflare.com>
> Signed-off-by: John Hurley <john.hurley@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Applied.