All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jianbo Liu <jianbol@nvidia.com>
To: <netdev@vger.kernel.org>
Cc: <pablo@netfilter.org>, Jianbo Liu <jianbol@nvidia.com>,
	Roi Dayan <roid@nvidia.com>
Subject: [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in tcf_block_unbind
Date: Thu, 8 Apr 2021 07:47:18 +0000	[thread overview]
Message-ID: <20210408074718.14331-1-jianbol@nvidia.com> (raw)

When device is removed, indirect block is unregisterd. As
bo->unlocked_driver_cb is not initialized, the following UBSAN is
triggered.

UBSAN: invalid-load in net/sched/cls_api.c:1496:10
load of value 6 is not a valid value for type '_Bool'

This patch fixes the warning by calling device's indr block bind
callback, and unlocked_driver_cb is assigned with correct value.

Fixes: 0fdcf78d5973 ("net: use flow_indr_dev_setup_offload()")
Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
---
 include/net/flow_offload.h | 11 ++++++-----
 net/core/flow_offload.c    | 13 +++++++------
 net/sched/cls_api.c        | 11 ++++++++---
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index dc5c1e69cd9f..8cdc60833890 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -459,6 +459,11 @@ typedef int flow_setup_cb_t(enum tc_setup_type type, void *type_data,
 
 struct flow_block_cb;
 
+typedef int flow_indr_block_bind_cb_t(struct net_device *dev, struct Qdisc *sch, void *cb_priv,
+				      enum tc_setup_type type, void *type_data,
+				      void *data,
+				      void (*cleanup)(struct flow_block_cb *block_cb));
+
 struct flow_block_indr {
 	struct list_head		list;
 	struct net_device		*dev;
@@ -466,6 +471,7 @@ struct flow_block_indr {
 	enum flow_block_binder_type	binder_type;
 	void				*data;
 	void				*cb_priv;
+	flow_indr_block_bind_cb_t	*setup_cb;
 	void				(*cleanup)(struct flow_block_cb *block_cb);
 };
 
@@ -562,11 +568,6 @@ static inline void flow_block_init(struct flow_block *flow_block)
 	INIT_LIST_HEAD(&flow_block->cb_list);
 }
 
-typedef int flow_indr_block_bind_cb_t(struct net_device *dev, struct Qdisc *sch, void *cb_priv,
-				      enum tc_setup_type type, void *type_data,
-				      void *data,
-				      void (*cleanup)(struct flow_block_cb *block_cb));
-
 int flow_indr_dev_register(flow_indr_block_bind_cb_t *cb, void *cb_priv);
 void flow_indr_dev_unregister(flow_indr_block_bind_cb_t *cb, void *cb_priv,
 			      void (*release)(void *cb_priv));
diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index 715b67f6c62f..85a3d8530952 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -373,7 +373,8 @@ int flow_indr_dev_register(flow_indr_block_bind_cb_t *cb, void *cb_priv)
 }
 EXPORT_SYMBOL(flow_indr_dev_register);
 
-static void __flow_block_indr_cleanup(void (*release)(void *cb_priv),
+static void __flow_block_indr_cleanup(struct flow_indr_dev *indr_dev,
+				      void (*release)(void *cb_priv),
 				      void *cb_priv,
 				      struct list_head *cleanup_list)
 {
@@ -381,8 +382,10 @@ static void __flow_block_indr_cleanup(void (*release)(void *cb_priv),
 
 	list_for_each_entry_safe(this, next, &flow_block_indr_list, indr.list) {
 		if (this->release == release &&
-		    this->indr.cb_priv == cb_priv)
+		    this->indr.cb_priv == cb_priv) {
+			this->indr.setup_cb = indr_dev->cb;
 			list_move(&this->indr.list, cleanup_list);
+		}
 	}
 }
 
@@ -390,10 +393,8 @@ static void flow_block_indr_notify(struct list_head *cleanup_list)
 {
 	struct flow_block_cb *this, *next;
 
-	list_for_each_entry_safe(this, next, cleanup_list, indr.list) {
-		list_del(&this->indr.list);
+	list_for_each_entry_safe(this, next, cleanup_list, indr.list)
 		this->indr.cleanup(this);
-	}
 }
 
 void flow_indr_dev_unregister(flow_indr_block_bind_cb_t *cb, void *cb_priv,
@@ -418,7 +419,7 @@ void flow_indr_dev_unregister(flow_indr_block_bind_cb_t *cb, void *cb_priv,
 		return;
 	}
 
-	__flow_block_indr_cleanup(release, cb_priv, &cleanup_list);
+	__flow_block_indr_cleanup(this, release, cb_priv, &cleanup_list);
 	mutex_unlock(&flow_indr_block_lock);
 
 	flow_block_indr_notify(&cleanup_list);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d3db70865d66..b213206da728 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -646,7 +646,7 @@ static void tc_block_indr_cleanup(struct flow_block_cb *block_cb)
 	struct net_device *dev = block_cb->indr.dev;
 	struct Qdisc *sch = block_cb->indr.sch;
 	struct netlink_ext_ack extack = {};
-	struct flow_block_offload bo;
+	struct flow_block_offload bo = {};
 
 	tcf_block_offload_init(&bo, dev, sch, FLOW_BLOCK_UNBIND,
 			       block_cb->indr.binder_type,
@@ -654,8 +654,13 @@ static void tc_block_indr_cleanup(struct flow_block_cb *block_cb)
 			       &extack);
 	rtnl_lock();
 	down_write(&block->cb_lock);
-	list_del(&block_cb->driver_list);
-	list_move(&block_cb->list, &bo.cb_list);
+	if (!block_cb->indr.setup_cb ||
+	    block_cb->indr.setup_cb(dev, sch, block_cb->indr.cb_priv,
+				    TC_SETUP_BLOCK, &bo, block, NULL)) {
+		list_del(&block_cb->indr.list);
+		list_del(&block_cb->driver_list);
+		list_move(&block_cb->list, &bo.cb_list);
+	}
 	tcf_block_unbind(block, &bo);
 	up_write(&block->cb_lock);
 	rtnl_unlock();
-- 
2.26.2


             reply	other threads:[~2021-04-08  7:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-08  7:47 Jianbo Liu [this message]
2021-04-08 21:16 ` [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in tcf_block_unbind Jakub Kicinski
2021-04-09  6:25   ` Jianbo Liu
2021-04-09 16:01     ` Jakub Kicinski
2021-04-12  1:21       ` Jianbo Liu
2021-04-20  8:11       ` Jianbo Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210408074718.14331-1-jianbol@nvidia.com \
    --to=jianbol@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=roid@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.