On Wed, Feb 10, 2021 at 03:50:03PM +0200, Vlad Buslov wrote:
>
> On Wed 10 Feb 2021 at 15:21, Ido Schimmel <idosch@xxxxxxxxxx> 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@xxxxxxxxxx> 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
