Re: [BUG] KASAN: slab-use-after-free Write in sk_skb_reason_drop

[Eulgyu Kim <eulgyukim@snu.ac.kr>]

Hello,

We have found that the bug still exists in v7.2.

> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index fed9dfdfc..1af529b04 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -289,8 +289,11 @@ static void tun_napi_disable(struct tun_file *tfile)
> 
>  static void tun_napi_del(struct tun_file *tfile)
>  {
> -       if (tfile->napi_enabled)
> +       if (tfile->napi_enabled) {
> +               mutex_lock(&tfile->napi_mutex);
>                 netif_napi_del(&tfile->napi);
> +               mutex_unlock(&tfile->napi_mutex);
> +       }
>  }
> 
>  static bool tun_napi_frags_enabled(const struct tun_file *tfile)

> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index fed9dfdfc..147c80c90 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -289,8 +289,12 @@ static void tun_napi_disable(struct tun_file *tfile)
> 
>  static void tun_napi_del(struct tun_file *tfile)
>  {
> -       if (tfile->napi_enabled)
> -               netif_napi_del(&tfile->napi);
> +       if (!tfile->napi_enabled)
> +               return;
> +
> +       mutex_lock(&tfile->napi_mutex);
> +       netif_napi_del(&tfile->napi);
> +       mutex_unlock(&tfile->napi_mutex);
>  }
> 
>  static bool tun_napi_frags_enabled(const struct tun_file *tfile)
> @@ -1783,6 +1787,12 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> 
>                 if (frags) {
>                         mutex_lock(&tfile->napi_mutex);
> +                       if (unlikely(tfile->detached ||
> +                                    rcu_access_pointer(tfile->tun) != tun)) {
> +                               err = -EBUSY;
> +                               mutex_unlock(&tfile->napi_mutex);
> +                               goto out;
> +                       }
>                         skb = tun_napi_alloc_frags(tfile, copylen, from);
>                         /* tun_napi_alloc_frags() enforces a layout for the skb.
>                          * If zerocopy is enabled, then this layout will be
> @@ -1981,6 +1991,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>                 mutex_unlock(&tfile->napi_mutex);
>         }
> 
> +out:
>         return err ?: total_len;
>  }

We have tested the both proposed patches, from Eric and Jiayuan,
and our reproducer did not trigger any issue.

It seems that Eric's patch is sufficient to address the reported use-after-free.
However, we are not sure whether the additional state check proposed by
Jiayuan is also necessary.

Thanks!

Best Regards,
Eulgyu Kim