netdev - Re: [PATCH] tun: support not enabling carrier in TUNSETIFF

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CANP3RGcvHH2WyQ7KtVoJ+LDLPH7kPx4DdxUTkq_8YciLQjBO3Q@mail.gmail.com>
Date:   Fri, 16 Sep 2022 16:54:17 -0700
From:   Maciej Żenczykowski <maze@...gle.com>
To:     Patrick Rohr <prohr@...gle.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Linux Network Development Mailing List 
        <netdev@...r.kernel.org>, Lorenzo Colitti <lorenzo@...gle.com>,
        Jason Wang <jasowang@...hat.com>
Subject: Re: [PATCH] tun: support not enabling carrier in TUNSETIFF

On Fri, Sep 16, 2022 at 4:46 PM Patrick Rohr <prohr@...gle.com> wrote:
>
> This change adds support for not enabling carrier during TUNSETIFF
> interface creation by specifying the IFF_NO_CARRIER flag.
>
> Our tests make heavy use of tun interfaces. In some scenarios, the test
> process creates the interface but another process brings it up after the
> interface is discovered via netlink notification. In that case, it is
> not possible to create a tun/tap interface with carrier off without it
> racing against the bring up. Immediately setting carrier off via
> TUNSETCARRIER is still too late.
>
> Since ifr_flags is only a short, the value for IFF_DETACH_QUEUE is
> reused for IFF_NO_CARRIER. IFF_DETACH_QUEUE has currently no meaning in
> TUNSETIFF.
>
> Signed-off-by: Patrick Rohr <prohr@...gle.com>
> Cc: Maciej Żenczykowski <maze@...gle.com>
> Cc: Lorenzo Colitti <lorenzo@...gle.com>
> Cc: Jason Wang <jasowang@...hat.com>
> ---
>  drivers/net/tun.c           | 15 ++++++++++++---
>  include/uapi/linux/if_tun.h |  2 ++
>  2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 259b2b84b2b3..502f56095650 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -2709,6 +2709,12 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>         struct net_device *dev;
>         int err;
>
> +       /* Do not save the IFF_NO_CARRIER flag as it uses the same value as
> +        * IFF_DETACH_QUEUE.
> +        */
> +       bool no_carrier = ifr->ifr_flags & IFF_NO_CARRIER;
> +       ifr->ifr_flags &= ~IFF_NO_CARRIER;
> +
>         if (tfile->detached)
>                 return -EINVAL;
>
> @@ -2828,7 +2834,10 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>                 rcu_assign_pointer(tfile->tun, tun);
>         }
>
> -       netif_carrier_on(tun->dev);
> +       if (no_carrier)
> +               netif_carrier_off(tun->dev);
> +       else
> +               netif_carrier_on(tun->dev);
>
>         /* Make sure persistent devices do not get stuck in
>          * xoff state.
> @@ -3056,8 +3065,8 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
>                  * This is needed because we never checked for invalid flags on
>                  * TUNSETIFF.
>                  */
> -               return put_user(IFF_TUN | IFF_TAP | TUN_FEATURES,
> -                               (unsigned int __user*)argp);
> +               return put_user(IFF_TUN | IFF_TAP | IFF_NO_CARRIER |
> +                               TUN_FEATURES, (unsigned int __user*)argp);
>         } else if (cmd == TUNSETQUEUE) {
>                 return tun_set_queue(file, &ifr);
>         } else if (cmd == SIOCGSKNS) {
> diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> index 2ec07de1d73b..12dde91957a5 100644
> --- a/include/uapi/linux/if_tun.h
> +++ b/include/uapi/linux/if_tun.h
> @@ -75,6 +75,8 @@
>  #define IFF_MULTI_QUEUE 0x0100
>  #define IFF_ATTACH_QUEUE 0x0200
>  #define IFF_DETACH_QUEUE 0x0400
> +/* Used in TUNSETIFF to bring up tun/tap without carrier */
> +#define IFF_NO_CARRIER IFF_DETACH_QUEUE
>  /* read-only flag */
>  #define IFF_PERSIST    0x0800
>  #define IFF_NOFILTER   0x1000
> --
> 2.37.3.968.ga6b4b080e4-goog

Reviewed-by: Maciej Żenczykowski <maze@...gle.com>