2021-07-16 10:43:38

by Xiyu Yang

[permalink] [raw]
Subject: [PATCH] drm/i915/gvt: Convert from atomic_t to refcount_t on intel_vgpu_ppgtt_spt->refcount

refcount_t type and corresponding API can protect refcounters from
accidental underflow and overflow and further use-after-free situations

Signed-off-by: Xiyu Yang <[email protected]>
Signed-off-by: Xin Tan <[email protected]>
---
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


2021-07-21 02:24:17

by Zhenyu Wang

[permalink] [raw]
Subject: Re: [PATCH] drm/i915/gvt: Convert from atomic_t to refcount_t on intel_vgpu_ppgtt_spt->refcount

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?

> Signed-off-by: Xiyu Yang <[email protected]>
> Signed-off-by: Xin Tan <[email protected]>
> ---
> 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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev


Attachments:
(No filename) (2.61 kB)
signature.asc (201.00 B)
Download all attachments

2021-07-21 09:47:30

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/i915/gvt: Convert from atomic_t to refcount_t on intel_vgpu_ppgtt_spt->refcount

On Wed, Jul 21, 2021 at 4:21 AM Zhenyu Wang <[email protected]> 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 <[email protected]>
> > Signed-off-by: Xin Tan <[email protected]>
> > ---
> > 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
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev



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