All of lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] net/mlx5e: Handle FIB events to update tunnel endpoint device
@ 2021-02-10 11:08 Dan Carpenter
  2021-02-10 11:55 ` Vlad Buslov
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2021-02-10 11:08 UTC (permalink / raw)
  To: vladbu; +Cc: linux-rdma

Hello Vlad Buslov,

The patch 8914add2c9e5: "net/mlx5e: Handle FIB events to update
tunnel endpoint device" from Jan 25, 2021, leads to the following
static checker warning:

	drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c:1639 mlx5e_tc_tun_init()
	error: passing non negative 1 to ERR_PTR

drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
  1622  struct mlx5e_tc_tun_encap *mlx5e_tc_tun_init(struct mlx5e_priv *priv)
  1623  {
  1624          struct mlx5e_tc_tun_encap *encap;
  1625          int err;
  1626  
  1627          encap = kvzalloc(sizeof(*encap), GFP_KERNEL);
  1628          if (!encap)
  1629                  return ERR_PTR(-ENOMEM);
  1630  
  1631          encap->priv = priv;
  1632          encap->fib_nb.notifier_call = mlx5e_tc_tun_fib_event;
  1633          spin_lock_init(&encap->route_lock);
  1634          hash_init(encap->route_tbl);
  1635          err = register_fib_notifier(dev_net(priv->netdev), &encap->fib_nb,
  1636                                      NULL, NULL);

register_fib_notifier() calls fib_net_dump() which eventually calls
fib6_walk_continue() which can return 1 if "walk is incomplete (i.e.
suspended)".

  1637          if (err) {
  1638                  kvfree(encap);
  1639                  return ERR_PTR(err);

If this returns 1 it will eventually lead to an Oops.

  1640          }
  1641  
  1642          return encap;
  1643  }

regards,
dan carpenter

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

* Re: [bug report] net/mlx5e: Handle FIB events to update tunnel endpoint device
  2021-02-10 11:08 [bug report] net/mlx5e: Handle FIB events to update tunnel endpoint device Dan Carpenter
@ 2021-02-10 11:55 ` Vlad Buslov
  2021-02-10 13:21   ` Ido Schimmel
  0 siblings, 1 reply; 7+ messages in thread
From: Vlad Buslov @ 2021-02-10 11:55 UTC (permalink / raw)
  To: Dan Carpenter, jiri, idosch; +Cc: linux-rdma

On Wed 10 Feb 2021 at 13:08, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hello Vlad Buslov,
>
> The patch 8914add2c9e5: "net/mlx5e: Handle FIB events to update
> tunnel endpoint device" from Jan 25, 2021, leads to the following
> static checker warning:
>
> 	drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c:1639 mlx5e_tc_tun_init()
> 	error: passing non negative 1 to ERR_PTR
>
> drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
>   1622  struct mlx5e_tc_tun_encap *mlx5e_tc_tun_init(struct mlx5e_priv *priv)
>   1623  {
>   1624          struct mlx5e_tc_tun_encap *encap;
>   1625          int err;
>   1626  
>   1627          encap = kvzalloc(sizeof(*encap), GFP_KERNEL);
>   1628          if (!encap)
>   1629                  return ERR_PTR(-ENOMEM);
>   1630  
>   1631          encap->priv = priv;
>   1632          encap->fib_nb.notifier_call = mlx5e_tc_tun_fib_event;
>   1633          spin_lock_init(&encap->route_lock);
>   1634          hash_init(encap->route_tbl);
>   1635          err = register_fib_notifier(dev_net(priv->netdev), &encap->fib_nb,
>   1636                                      NULL, NULL);
>
> register_fib_notifier() calls fib_net_dump() which eventually calls
> fib6_walk_continue() which can return 1 if "walk is incomplete (i.e.
> suspended)".
>
>   1637          if (err) {
>   1638                  kvfree(encap);
>   1639                  return ERR_PTR(err);
>
> If this returns 1 it will eventually lead to an Oops.

Hi Dan,

Thanks for the bug report!

This looks a bit strange to me because none of the other users of this
API handle positive error code in any special way (including reference
netdevsim implementation). Maybe API itself should be fixed? Jiri, Ido,
what do you think?

Regards,
Vlad

>
>   1640          }
>   1641  
>   1642          return encap;
>   1643  }
>
> regards,
> dan carpenter


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

* Re: [bug report] net/mlx5e: Handle FIB events to update tunnel endpoint device
  2021-02-10 11:55 ` Vlad Buslov
@ 2021-02-10 13:21   ` Ido Schimmel
  2021-02-10 13:50     ` Vlad Buslov
  2021-02-10 14:00     ` Dan Carpenter
  0 siblings, 2 replies; 7+ messages in thread
From: Ido Schimmel @ 2021-02-10 13:21 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: Dan Carpenter, jiri, linux-rdma

On Wed, Feb 10, 2021 at 01:55:23PM +0200, Vlad Buslov wrote:
> On Wed 10 Feb 2021 at 13:08, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > Hello Vlad Buslov,
> >
> > The patch 8914add2c9e5: "net/mlx5e: Handle FIB events to update
> > tunnel endpoint device" from Jan 25, 2021, leads to the following
> > static checker warning:
> >
> > 	drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c:1639 mlx5e_tc_tun_init()
> > 	error: passing non negative 1 to ERR_PTR
> >
> > drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
> >   1622  struct mlx5e_tc_tun_encap *mlx5e_tc_tun_init(struct mlx5e_priv *priv)
> >   1623  {
> >   1624          struct mlx5e_tc_tun_encap *encap;
> >   1625          int err;
> >   1626  
> >   1627          encap = kvzalloc(sizeof(*encap), GFP_KERNEL);
> >   1628          if (!encap)
> >   1629                  return ERR_PTR(-ENOMEM);
> >   1630  
> >   1631          encap->priv = priv;
> >   1632          encap->fib_nb.notifier_call = mlx5e_tc_tun_fib_event;
> >   1633          spin_lock_init(&encap->route_lock);
> >   1634          hash_init(encap->route_tbl);
> >   1635          err = register_fib_notifier(dev_net(priv->netdev), &encap->fib_nb,
> >   1636                                      NULL, NULL);
> >
> > register_fib_notifier() calls fib_net_dump() which eventually calls
> > fib6_walk_continue() which can return 1 if "walk is incomplete (i.e.
> > suspended)".
> >
> >   1637          if (err) {
> >   1638                  kvfree(encap);
> >   1639                  return ERR_PTR(err);
> >
> > If this returns 1 it will eventually lead to an Oops.
> 
> Hi Dan,
> 
> Thanks for the bug report!
> 
> This looks a bit strange to me because none of the other users of this
> API handle positive error code in any special way (including reference
> netdevsim implementation). Maybe API itself should be fixed? Jiri, Ido,
> what do you think?

The other functions that call register_fib_notifier() return an int, but
mlx5e_tc_tun_init() returns a pointer. I think that's why it was
flagged: "error: passing non negative 1 to ERR_PTR".

fib6_walk_continue() cannot return a positive value when called from
register_fib_notifier(), but ignoring it means that you will keep
getting the same static analysis error. What about:

diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index f43e27555725..ef9d022e693f 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -499,7 +499,7 @@ int fib6_tables_dump(struct net *net, struct notifier_block *nb,
 
                hlist_for_each_entry_rcu(tb, head, tb6_hlist) {
                        err = fib6_table_dump(net, tb, w);
-                       if (err < 0)
+                       if (err)
                                goto out;
                }
        }
@@ -507,7 +507,8 @@ int fib6_tables_dump(struct net *net, struct notifier_block *nb,
 out:
        kfree(w);
 
-       return err;
+       /* The tree traversal function should never return a positive value. */
+       return err > 0 ? -EINVAL : err;
 }
 
 static int fib6_dump_node(struct fib6_walker *w)

> 
> Regards,
> Vlad
> 
> >
> >   1640          }
> >   1641  
> >   1642          return encap;
> >   1643  }
> >
> > regards,
> > dan carpenter
> 

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

* Re: [bug report] net/mlx5e: Handle FIB events to update tunnel endpoint device
  2021-02-10 13:21   ` Ido Schimmel
@ 2021-02-10 13:50     ` Vlad Buslov
  2021-02-10 14:01       ` Ido Schimmel
  2021-02-10 14:03       ` Dan Carpenter
  2021-02-10 14:00     ` Dan Carpenter
  1 sibling, 2 replies; 7+ messages in thread
From: Vlad Buslov @ 2021-02-10 13:50 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: Dan Carpenter, jiri, linux-rdma


On Wed 10 Feb 2021 at 15:21, Ido Schimmel <idosch@idosch.org> wrote:
> On Wed, Feb 10, 2021 at 01:55:23PM +0200, Vlad Buslov wrote:
>> On Wed 10 Feb 2021 at 13:08, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>> > Hello Vlad Buslov,
>> >
>> > The patch 8914add2c9e5: "net/mlx5e: Handle FIB events to update
>> > tunnel endpoint device" from Jan 25, 2021, leads to the following
>> > static checker warning:
>> >
>> > 	drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c:1639 mlx5e_tc_tun_init()
>> > 	error: passing non negative 1 to ERR_PTR
>> >
>> > drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
>> >   1622  struct mlx5e_tc_tun_encap *mlx5e_tc_tun_init(struct mlx5e_priv *priv)
>> >   1623  {
>> >   1624          struct mlx5e_tc_tun_encap *encap;
>> >   1625          int err;
>> >   1626  
>> >   1627          encap = kvzalloc(sizeof(*encap), GFP_KERNEL);
>> >   1628          if (!encap)
>> >   1629                  return ERR_PTR(-ENOMEM);
>> >   1630  
>> >   1631          encap->priv = priv;
>> >   1632          encap->fib_nb.notifier_call = mlx5e_tc_tun_fib_event;
>> >   1633          spin_lock_init(&encap->route_lock);
>> >   1634          hash_init(encap->route_tbl);
>> >   1635          err = register_fib_notifier(dev_net(priv->netdev), &encap->fib_nb,
>> >   1636                                      NULL, NULL);
>> >
>> > register_fib_notifier() calls fib_net_dump() which eventually calls
>> > fib6_walk_continue() which can return 1 if "walk is incomplete (i.e.
>> > suspended)".
>> >
>> >   1637          if (err) {
>> >   1638                  kvfree(encap);
>> >   1639                  return ERR_PTR(err);
>> >
>> > If this returns 1 it will eventually lead to an Oops.
>> 
>> Hi Dan,
>> 
>> Thanks for the bug report!
>> 
>> This looks a bit strange to me because none of the other users of this
>> API handle positive error code in any special way (including reference
>> netdevsim implementation). Maybe API itself should be fixed? Jiri, Ido,
>> what do you think?
>
> The other functions that call register_fib_notifier() return an int, but
> mlx5e_tc_tun_init() returns a pointer. I think that's why it was
> flagged: "error: passing non negative 1 to ERR_PTR".

Actually, some of them do. I mentioned netdevsim specifically because
nsim_fib_create() returns pointer and also casts return value of
register_fib_notifier() with ERR_PTR.

>
> fib6_walk_continue() cannot return a positive value when called from
> register_fib_notifier(), but ignoring it means that you will keep
> getting the same static analysis error. What about:
>
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index f43e27555725..ef9d022e693f 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -499,7 +499,7 @@ int fib6_tables_dump(struct net *net, struct notifier_block *nb,
>  
>                 hlist_for_each_entry_rcu(tb, head, tb6_hlist) {
>                         err = fib6_table_dump(net, tb, w);
> -                       if (err < 0)
> +                       if (err)
>                                 goto out;
>                 }
>         }
> @@ -507,7 +507,8 @@ int fib6_tables_dump(struct net *net, struct notifier_block *nb,
>  out:
>         kfree(w);
>  
> -       return err;
> +       /* The tree traversal function should never return a positive value. */
> +       return err > 0 ? -EINVAL : err;
>  }
>  
>  static int fib6_dump_node(struct fib6_walker *w)
>

Makes sense. You want me to send the fix or you will do it?

>> 
>> Regards,
>> Vlad
>> 
>> >
>> >   1640          }
>> >   1641  
>> >   1642          return encap;
>> >   1643  }
>> >
>> > regards,
>> > dan carpenter
>> 


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

* Re: [bug report] net/mlx5e: Handle FIB events to update tunnel endpoint device
  2021-02-10 13:21   ` Ido Schimmel
  2021-02-10 13:50     ` Vlad Buslov
@ 2021-02-10 14:00     ` Dan Carpenter
  1 sibling, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2021-02-10 14:00 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: Vlad Buslov, jiri, linux-rdma

On Wed, Feb 10, 2021 at 03:21:37PM +0200, Ido Schimmel wrote:
> On Wed, Feb 10, 2021 at 01:55:23PM +0200, Vlad Buslov wrote:
> > On Wed 10 Feb 2021 at 13:08, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > Hello Vlad Buslov,
> > >
> > > The patch 8914add2c9e5: "net/mlx5e: Handle FIB events to update
> > > tunnel endpoint device" from Jan 25, 2021, leads to the following
> > > static checker warning:
> > >
> > > 	drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c:1639 mlx5e_tc_tun_init()
> > > 	error: passing non negative 1 to ERR_PTR
> > >
> > > drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
> > >   1622  struct mlx5e_tc_tun_encap *mlx5e_tc_tun_init(struct mlx5e_priv *priv)
> > >   1623  {
> > >   1624          struct mlx5e_tc_tun_encap *encap;
> > >   1625          int err;
> > >   1626  
> > >   1627          encap = kvzalloc(sizeof(*encap), GFP_KERNEL);
> > >   1628          if (!encap)
> > >   1629                  return ERR_PTR(-ENOMEM);
> > >   1630  
> > >   1631          encap->priv = priv;
> > >   1632          encap->fib_nb.notifier_call = mlx5e_tc_tun_fib_event;
> > >   1633          spin_lock_init(&encap->route_lock);
> > >   1634          hash_init(encap->route_tbl);
> > >   1635          err = register_fib_notifier(dev_net(priv->netdev), &encap->fib_nb,
> > >   1636                                      NULL, NULL);
> > >
> > > register_fib_notifier() calls fib_net_dump() which eventually calls
> > > fib6_walk_continue() which can return 1 if "walk is incomplete (i.e.
> > > suspended)".
> > >
> > >   1637          if (err) {
> > >   1638                  kvfree(encap);
> > >   1639                  return ERR_PTR(err);
> > >
> > > If this returns 1 it will eventually lead to an Oops.
> > 
> > Hi Dan,
> > 
> > Thanks for the bug report!
> > 
> > This looks a bit strange to me because none of the other users of this
> > API handle positive error code in any special way (including reference
> > netdevsim implementation). Maybe API itself should be fixed? Jiri, Ido,
> > what do you think?
> 
> The other functions that call register_fib_notifier() return an int, but
> mlx5e_tc_tun_init() returns a pointer. I think that's why it was
> flagged: "error: passing non negative 1 to ERR_PTR".
> 
> fib6_walk_continue() cannot return a positive value when called from
> register_fib_notifier()

Ideally Smatch would be able to figure out this rule automatically.

I see now that fib6_tables_dump() can't return positive because it
returns either zero or the return from the "w->func = fib6_node_dump;"
and fib6_node_dump() doesn't return positive.  

Handling it properly is something I've thought about but I haven't
figured out how.  What I will do is hard code this into Smatch by adding
a line into the smatch_data/db/fixup_kernel.sh file:

delete from return_states where function = 'fib6_tables_dump' and return = '1';

regards,
dan carpenter


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

* Re: [bug report] net/mlx5e: Handle FIB events to update tunnel endpoint device
  2021-02-10 13:50     ` Vlad Buslov
@ 2021-02-10 14:01       ` Ido Schimmel
  2021-02-10 14:03       ` Dan Carpenter
  1 sibling, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2021-02-10 14:01 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: Dan Carpenter, jiri, linux-rdma

On Wed, Feb 10, 2021 at 03:50:03PM +0200, Vlad Buslov wrote:
> 
> On Wed 10 Feb 2021 at 15:21, Ido Schimmel <idosch@idosch.org> wrote:
> > On Wed, Feb 10, 2021 at 01:55:23PM +0200, Vlad Buslov wrote:
> >> On Wed 10 Feb 2021 at 13:08, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >> > Hello Vlad Buslov,
> >> >
> >> > The patch 8914add2c9e5: "net/mlx5e: Handle FIB events to update
> >> > tunnel endpoint device" from Jan 25, 2021, leads to the following
> >> > static checker warning:
> >> >
> >> > 	drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c:1639 mlx5e_tc_tun_init()
> >> > 	error: passing non negative 1 to ERR_PTR
> >> >
> >> > drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
> >> >   1622  struct mlx5e_tc_tun_encap *mlx5e_tc_tun_init(struct mlx5e_priv *priv)
> >> >   1623  {
> >> >   1624          struct mlx5e_tc_tun_encap *encap;
> >> >   1625          int err;
> >> >   1626  
> >> >   1627          encap = kvzalloc(sizeof(*encap), GFP_KERNEL);
> >> >   1628          if (!encap)
> >> >   1629                  return ERR_PTR(-ENOMEM);
> >> >   1630  
> >> >   1631          encap->priv = priv;
> >> >   1632          encap->fib_nb.notifier_call = mlx5e_tc_tun_fib_event;
> >> >   1633          spin_lock_init(&encap->route_lock);
> >> >   1634          hash_init(encap->route_tbl);
> >> >   1635          err = register_fib_notifier(dev_net(priv->netdev), &encap->fib_nb,
> >> >   1636                                      NULL, NULL);
> >> >
> >> > register_fib_notifier() calls fib_net_dump() which eventually calls
> >> > fib6_walk_continue() which can return 1 if "walk is incomplete (i.e.
> >> > suspended)".
> >> >
> >> >   1637          if (err) {
> >> >   1638                  kvfree(encap);
> >> >   1639                  return ERR_PTR(err);
> >> >
> >> > If this returns 1 it will eventually lead to an Oops.
> >> 
> >> Hi Dan,
> >> 
> >> Thanks for the bug report!
> >> 
> >> This looks a bit strange to me because none of the other users of this
> >> API handle positive error code in any special way (including reference
> >> netdevsim implementation). Maybe API itself should be fixed? Jiri, Ido,
> >> what do you think?
> >
> > The other functions that call register_fib_notifier() return an int, but
> > mlx5e_tc_tun_init() returns a pointer. I think that's why it was
> > flagged: "error: passing non negative 1 to ERR_PTR".
> 
> Actually, some of them do. I mentioned netdevsim specifically because
> nsim_fib_create() returns pointer and also casts return value of
> register_fib_notifier() with ERR_PTR.
> 
> >
> > fib6_walk_continue() cannot return a positive value when called from
> > register_fib_notifier(), but ignoring it means that you will keep
> > getting the same static analysis error. What about:
> >
> > diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> > index f43e27555725..ef9d022e693f 100644
> > --- a/net/ipv6/ip6_fib.c
> > +++ b/net/ipv6/ip6_fib.c
> > @@ -499,7 +499,7 @@ int fib6_tables_dump(struct net *net, struct notifier_block *nb,
> >  
> >                 hlist_for_each_entry_rcu(tb, head, tb6_hlist) {
> >                         err = fib6_table_dump(net, tb, w);
> > -                       if (err < 0)
> > +                       if (err)
> >                                 goto out;
> >                 }
> >         }
> > @@ -507,7 +507,8 @@ int fib6_tables_dump(struct net *net, struct notifier_block *nb,
> >  out:
> >         kfree(w);
> >  
> > -       return err;
> > +       /* The tree traversal function should never return a positive value. */
> > +       return err > 0 ? -EINVAL : err;
> >  }
> >  
> >  static int fib6_dump_node(struct fib6_walker *w)
> >
> 
> Makes sense. You want me to send the fix or you will do it?

Can you send to linux-internal and then I will apply it to our
regression and let you know tomorrow morning?

> 
> >> 
> >> Regards,
> >> Vlad
> >> 
> >> >
> >> >   1640          }
> >> >   1641  
> >> >   1642          return encap;
> >> >   1643  }
> >> >
> >> > regards,
> >> > dan carpenter
> >> 
> 

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

* Re: [bug report] net/mlx5e: Handle FIB events to update tunnel endpoint device
  2021-02-10 13:50     ` Vlad Buslov
  2021-02-10 14:01       ` Ido Schimmel
@ 2021-02-10 14:03       ` Dan Carpenter
  1 sibling, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2021-02-10 14:03 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: Ido Schimmel, jiri, linux-rdma

On Wed, Feb 10, 2021 at 03:50:03PM +0200, Vlad Buslov wrote:
> 
> On Wed 10 Feb 2021 at 15:21, Ido Schimmel <idosch@idosch.org> wrote:
> > On Wed, Feb 10, 2021 at 01:55:23PM +0200, Vlad Buslov wrote:
> >> On Wed 10 Feb 2021 at 13:08, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >> > Hello Vlad Buslov,
> >> >
> >> > The patch 8914add2c9e5: "net/mlx5e: Handle FIB events to update
> >> > tunnel endpoint device" from Jan 25, 2021, leads to the following
> >> > static checker warning:
> >> >
> >> > 	drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c:1639 mlx5e_tc_tun_init()
> >> > 	error: passing non negative 1 to ERR_PTR
> >> >
> >> > drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
> >> >   1622  struct mlx5e_tc_tun_encap *mlx5e_tc_tun_init(struct mlx5e_priv *priv)
> >> >   1623  {
> >> >   1624          struct mlx5e_tc_tun_encap *encap;
> >> >   1625          int err;
> >> >   1626  
> >> >   1627          encap = kvzalloc(sizeof(*encap), GFP_KERNEL);
> >> >   1628          if (!encap)
> >> >   1629                  return ERR_PTR(-ENOMEM);
> >> >   1630  
> >> >   1631          encap->priv = priv;
> >> >   1632          encap->fib_nb.notifier_call = mlx5e_tc_tun_fib_event;
> >> >   1633          spin_lock_init(&encap->route_lock);
> >> >   1634          hash_init(encap->route_tbl);
> >> >   1635          err = register_fib_notifier(dev_net(priv->netdev), &encap->fib_nb,
> >> >   1636                                      NULL, NULL);
> >> >
> >> > register_fib_notifier() calls fib_net_dump() which eventually calls
> >> > fib6_walk_continue() which can return 1 if "walk is incomplete (i.e.
> >> > suspended)".
> >> >
> >> >   1637          if (err) {
> >> >   1638                  kvfree(encap);
> >> >   1639                  return ERR_PTR(err);
> >> >
> >> > If this returns 1 it will eventually lead to an Oops.
> >> 
> >> Hi Dan,
> >> 
> >> Thanks for the bug report!
> >> 
> >> This looks a bit strange to me because none of the other users of this
> >> API handle positive error code in any special way (including reference
> >> netdevsim implementation). Maybe API itself should be fixed? Jiri, Ido,
> >> what do you think?
> >
> > The other functions that call register_fib_notifier() return an int, but
> > mlx5e_tc_tun_init() returns a pointer. I think that's why it was
> > flagged: "error: passing non negative 1 to ERR_PTR".
> 
> Actually, some of them do. I mentioned netdevsim specifically because
> nsim_fib_create() returns pointer and also casts return value of
> register_fib_notifier() with ERR_PTR.
> 

Huh.  Smatch prints a warning for nsim_fib_create() but I never
forwarded the warning to anyone apparently.

regards,
dan carpenter


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

end of thread, other threads:[~2021-02-10 14:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-10 11:08 [bug report] net/mlx5e: Handle FIB events to update tunnel endpoint device Dan Carpenter
2021-02-10 11:55 ` Vlad Buslov
2021-02-10 13:21   ` Ido Schimmel
2021-02-10 13:50     ` Vlad Buslov
2021-02-10 14:01       ` Ido Schimmel
2021-02-10 14:03       ` Dan Carpenter
2021-02-10 14:00     ` Dan Carpenter

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.