Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1626432098-27626-1-git-send-email-xiyuyang19@fudan.edu.cn> <20210721020009.GG13928@zhen-hp.sh.intel.com>
In-Reply-To: <20210721020009.GG13928@zhen-hp.sh.intel.com>
From:   Daniel Vetter <daniel@ffwll.ch>
Date:   Wed, 21 Jul 2021 11:40:40 +0200
Message-ID: <CAKMK7uHFarZMChcVNdxpP=1XKqc+WJCayELvpiAjmzO9YXk+hA@mail.gmail.com>
Subject: Re: [PATCH] drm/i915/gvt: Convert from atomic_t to refcount_t on intel_vgpu_ppgtt_spt->refcount
To:     Zhenyu Wang <zhenyuw@linux.intel.com>
Cc:     Xiyu Yang <xiyuyang19@fudan.edu.cn>,
        Zhi Wang <zhi.a.wang@intel.com>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        David Airlie <airlied@linux.ie>,
        intel-gvt-dev <intel-gvt-dev@lists.freedesktop.org>,
        intel-gfx <intel-gfx@lists.freedesktop.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Xin Tan <tanxin.ctf@gmail.com>, yuanxzhang@fudan.edu.cn
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jul 21, 2021 at 4:21 AM Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> On 2021.07.16 18:41:38 +0800, Xiyu Yang wrote:
> > refcount_t type and corresponding API can protect refcounters from
> > accidental underflow and overflow and further use-after-free situations
> >
>
> Thanks for the patch. Is there any specific problem you run with current code?
> Any shadow ppgtt error?

refcount_t is just part of the kernel hardening project, and
recommeded to be used anywhere it's possible. It doesn't fix bugs
itself, but makes it impossible to exploit at least some of them and
warns in other cases, so easier to catch them if they do exist.
-Daniel

> > Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> > Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/gtt.c | 11 ++++++-----
> >  drivers/gpu/drm/i915/gvt/gtt.h |  3 ++-
> >  2 files changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> > index cc2c05e18206..62f3daff5a36 100644
> > --- a/drivers/gpu/drm/i915/gvt/gtt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> > @@ -841,7 +841,7 @@ static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt(
> >       }
> >
> >       spt->vgpu = vgpu;
> > -     atomic_set(&spt->refcount, 1);
> > +     refcount_set(&spt->refcount, 1);
> >       INIT_LIST_HEAD(&spt->post_shadow_list);
> >
> >       /*
> > @@ -927,18 +927,19 @@ static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt_gfn(
> >
> >  static inline void ppgtt_get_spt(struct intel_vgpu_ppgtt_spt *spt)
> >  {
> > -     int v = atomic_read(&spt->refcount);
> > +     int v = refcount_read(&spt->refcount);
> >
> >       trace_spt_refcount(spt->vgpu->id, "inc", spt, v, (v + 1));
> > -     atomic_inc(&spt->refcount);
> > +     refcount_inc(&spt->refcount);
> >  }
> >
> >  static inline int ppgtt_put_spt(struct intel_vgpu_ppgtt_spt *spt)
> >  {
> > -     int v = atomic_read(&spt->refcount);
> > +     int v = refcount_read(&spt->refcount);
> >
> >       trace_spt_refcount(spt->vgpu->id, "dec", spt, v, (v - 1));
> > -     return atomic_dec_return(&spt->refcount);
> > +     refcount_dec(&spt->refcount);
> > +     return refcount_read(&spt->refcount);
> >  }
> >
> >  static int ppgtt_invalidate_spt(struct intel_vgpu_ppgtt_spt *spt);
> > diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
> > index 3bf45672ef98..944c2d0739df 100644
> > --- a/drivers/gpu/drm/i915/gvt/gtt.h
> > +++ b/drivers/gpu/drm/i915/gvt/gtt.h
> > @@ -38,6 +38,7 @@
> >  #include <linux/kref.h>
> >  #include <linux/mutex.h>
> >  #include <linux/radix-tree.h>
> > +#include <linux/refcount.h>
> >
> >  #include "gt/intel_gtt.h"
> >
> > @@ -243,7 +244,7 @@ struct intel_vgpu_oos_page {
> >
> >  /* Represent a vgpu shadow page table. */
> >  struct intel_vgpu_ppgtt_spt {
> > -     atomic_t refcount;
> > +     refcount_t refcount;
> >       struct intel_vgpu *vgpu;
> >
> >       struct {
> > --
> > 2.7.4
> >
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch