Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in tcf_block_unbind
@ 2021-04-08  7:47 Jianbo Liu
  2021-04-08 21:16 ` Jakub Kicinski
  0 siblings, 1 reply; 5+ messages in thread
From: Jianbo Liu @ 2021-04-08  7:47 UTC (permalink / raw)
  To: netdev; +Cc: pablo, Jianbo Liu, Roi Dayan

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in tcf_block_unbind
  2021-04-08  7:47 [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in tcf_block_unbind Jianbo Liu
@ 2021-04-08 21:16 ` Jakub Kicinski
  2021-04-09  6:25   ` Jianbo Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2021-04-08 21:16 UTC (permalink / raw)
  To: Jianbo Liu; +Cc: netdev, pablo, Roi Dayan

On Thu, 8 Apr 2021 07:47:18 +0000 Jianbo Liu wrote:
> 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>

It's been a while since I looked at this code but I don't understand
what you're doing here.

The init in tc_block_indr_cleanup() makes sense. What's the change to
setup_cb achieving? Thanks.

> 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();


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in tcf_block_unbind
  2021-04-08 21:16 ` Jakub Kicinski
@ 2021-04-09  6:25   ` Jianbo Liu
  2021-04-09 16:01     ` Jakub Kicinski
  0 siblings, 1 reply; 5+ messages in thread
From: Jianbo Liu @ 2021-04-09  6:25 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, pablo, Roi Dayan

The 04/08/2021 14:16, Jakub Kicinski wrote:
> On Thu, 8 Apr 2021 07:47:18 +0000 Jianbo Liu wrote:
> > 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>
> 
> It's been a while since I looked at this code but I don't understand
> what you're doing here.

To fix the UBSAN warning in tcf_block_unbind. It's easily triggered when
netdev is removed before tunnel netdev.

> 
> The init in tc_block_indr_cleanup() makes sense. What's the change to
> setup_cb achieving? Thanks.

But unlocked_driver_cb of flow_block_offload is not initialized in init.
Calling setup_cb is to get the correct value from driver.

> 
> > 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();
> 

-- 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in tcf_block_unbind
  2021-04-09  6:25   ` Jianbo Liu
@ 2021-04-09 16:01     ` Jakub Kicinski
  2021-04-12  1:21       ` Jianbo Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2021-04-09 16:01 UTC (permalink / raw)
  To: Jianbo Liu; +Cc: netdev, pablo, Roi Dayan

On Fri, 9 Apr 2021 06:25:56 +0000 Jianbo Liu wrote:
> The 04/08/2021 14:16, Jakub Kicinski wrote:
> > On Thu, 8 Apr 2021 07:47:18 +0000 Jianbo Liu wrote:  
> > > 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>  
> > 
> > It's been a while since I looked at this code but I don't understand
> > what you're doing here.  
> 
> To fix the UBSAN warning in tcf_block_unbind. It's easily triggered when
> netdev is removed before tunnel netdev.
> 
> > The init in tc_block_indr_cleanup() makes sense. What's the change to
> > setup_cb achieving? Thanks.  
> 
> But unlocked_driver_cb of flow_block_offload is not initialized in init.
> Calling setup_cb is to get the correct value from driver.

I'm trying to understand what became of this code :/ Was there no call
with FLOW_BLOCK_UNBIND to the driver when driver was unregistering
before your change?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in tcf_block_unbind
  2021-04-09 16:01     ` Jakub Kicinski
@ 2021-04-12  1:21       ` Jianbo Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Jianbo Liu @ 2021-04-12  1:21 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, pablo, Roi Dayan

The 04/09/2021 09:01, Jakub Kicinski wrote:
> On Fri, 9 Apr 2021 06:25:56 +0000 Jianbo Liu wrote:
> > The 04/08/2021 14:16, Jakub Kicinski wrote:
> > > On Thu, 8 Apr 2021 07:47:18 +0000 Jianbo Liu wrote:  
> > > > 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>  
> > > 
> > > It's been a while since I looked at this code but I don't understand
> > > what you're doing here.  
> > 
> > To fix the UBSAN warning in tcf_block_unbind. It's easily triggered when
> > netdev is removed before tunnel netdev.
> > 
> > > The init in tc_block_indr_cleanup() makes sense. What's the change to
> > > setup_cb achieving? Thanks.  
> > 
> > But unlocked_driver_cb of flow_block_offload is not initialized in init.
> > Calling setup_cb is to get the correct value from driver.
> 
> I'm trying to understand what became of this code :/ Was there no call
> with FLOW_BLOCK_UNBIND to the driver when driver was unregistering
> before your change?

Yes, I think so.
> 

-- 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-08  7:47 [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in tcf_block_unbind Jianbo Liu
2021-04-08 21:16 ` Jakub Kicinski
2021-04-09  6:25   ` Jianbo Liu
2021-04-09 16:01     ` Jakub Kicinski
2021-04-12  1:21       ` Jianbo Liu

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git