netdev - Re: [PATCH net-next v2 1/2] net: create device lookup API with reference tracking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 16 Jun 2023 10:21:04 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, pabeni@...hat.com, 
	dsahern@...il.com
Subject: Re: [PATCH net-next v2 1/2] net: create device lookup API with
 reference tracking

On Mon, Jun 12, 2023 at 11:49 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> New users of dev_get_by_index() and dev_get_by_name() keep
> getting added and it would be nice to steer them towards
> the APIs with reference tracking.
>
> Add variants of those calls which allocate the reference
> tracker and use them in a couple of places.
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---


> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 392aaa373b66..e510a4162ef8 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3503,6 +3503,7 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
>                  struct fib6_config *cfg, gfp_t gfp_flags,
>                  struct netlink_ext_ack *extack)
>  {
> +       netdevice_tracker *dev_tracker = &fib6_nh->fib_nh_dev_tracker;
>         struct net_device *dev = NULL;
>         struct inet6_dev *idev = NULL;
>         int addr_type;
> @@ -3520,7 +3521,8 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
>
>         err = -ENODEV;
>         if (cfg->fc_ifindex) {
> -               dev = dev_get_by_index(net, cfg->fc_ifindex);
> +               dev = netdev_get_by_index(net, cfg->fc_ifindex,
> +                                         dev_tracker, gfp_flags);
>                 if (!dev)
>                         goto out;
>                 idev = in6_dev_get(dev);
> @@ -3554,11 +3556,11 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
>                 /* hold loopback dev/idev if we haven't done so. */
>                 if (dev != net->loopback_dev) {
>                         if (dev) {
> -                               dev_put(dev);
> +                               netdev_put(dev, dev_tracker);
>                                 in6_dev_put(idev);
>                         }
>                         dev = net->loopback_dev;
> -                       dev_hold(dev);
> +                       netdev_hold(dev, dev_tracker, gfp_flags);
>                         idev = in6_dev_get(dev);
>                         if (!idev) {
>                                 err = -ENODEV;
> @@ -3610,8 +3612,6 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
>         }
>
>         fib6_nh->fib_nh_dev = dev;
> -       netdev_tracker_alloc(dev, &fib6_nh->fib_nh_dev_tracker, gfp_flags);
> -
>         fib6_nh->fib_nh_oif = dev->ifindex;
>         err = 0;
>  out:
> @@ -3621,7 +3621,7 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
>         if (err) {
>                 lwtstate_put(fib6_nh->fib_nh_lws);
>                 fib6_nh->fib_nh_lws = NULL;
> -               dev_put(dev);
> +               netdev_put(dev, dev_tracker);
>         }
>
>         return err;
> --
> 2.40.1
>

Oops, ip6_validate_gw() is able to change dev under us (this is done
in ip6_route_check_nh())

Crazy, I know...