netdev - Re: [PATCH net] net: tun: stop NAPI when detaching queues

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]

Message-ID: <CA+DcSEhaiOySZduR+-1Ep3WYt_9cm0kYB25GbSDZLPsCVbuYgA@mail.gmail.com>
Date:   Fri, 24 Jun 2022 18:09:38 -0700
From:   Petar Penkov <peterpenkov96@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Network Development <netdev@...r.kernel.org>,
        pabeni@...hat.com, Mahesh Bandewar <maheshb@...gle.com>
Subject: Re: [PATCH net] net: tun: stop NAPI when detaching queues

Thanks for the fix (and for the syzbot one)!

Acked-by: Petar Penkov <ppenkov@...atrix.com>


On Wed, Jun 22, 2022 at 9:21 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> While looking at a syzbot report I noticed the NAPI only gets
> disabled before it's deleted. I think that user can detach
> the queue before destroying the device and the NAPI will never
> be stopped.
>
> Compile tested only.
>
> Fixes: 943170998b20 ("tun: enable NAPI for TUN/TAP driver")
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> CC: maheshb@...gle.com
> CC: peterpenkov96@...il.com
> ---
>  drivers/net/tun.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 7fd0288c3789..e2eb35887394 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -273,6 +273,12 @@ static void tun_napi_init(struct tun_struct *tun, struct tun_file *tfile,
>         }
>  }
>
> +static void tun_napi_enable(struct tun_file *tfile)
> +{
> +       if (tfile->napi_enabled)
> +               napi_enable(&tfile->napi);
> +}
> +
>  static void tun_napi_disable(struct tun_file *tfile)
>  {
>         if (tfile->napi_enabled)
> @@ -653,8 +659,10 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
>                 if (clean) {
>                         RCU_INIT_POINTER(tfile->tun, NULL);
>                         sock_put(&tfile->sk);
> -               } else
> +               } else {
>                         tun_disable_queue(tun, tfile);
> +                       tun_napi_disable(tfile);
> +               }
>
>                 synchronize_net();
>                 tun_flow_delete_by_queue(tun, tun->numqueues + 1);
> @@ -808,6 +816,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
>
>         if (tfile->detached) {
>                 tun_enable_queue(tfile);
> +               tun_napi_enable(tfile);
>         } else {
>                 sock_hold(&tfile->sk);
>                 tun_napi_init(tun, tfile, napi, napi_frags);
> --
> 2.36.1
>