> Hi Elena,
>
> On Mon, Mar 06, 2017 at 04:21:00PM +0200, Elena Reshetova wrote:
> > refcount_t type and corresponding API should be
> > used instead of atomic_t when the variable is used as
> > a reference counter. This allows to avoid accidental
> > refcounter overflows that might lead to use-after-free
> > situations.
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@xxxxxxxxx>
> > Signed-off-by: Hans Liljestrand <ishkamiel@xxxxxxxxx>
> > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> > Signed-off-by: David Windsor <dwindsor@xxxxxxxxx>
> > ---
> > drivers/media/v4l2-core/videobuf2-memops.c | 6 +++---
> > include/media/videobuf2-memops.h | 3 ++-
> > 2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/videobuf2-memops.c b/drivers/media/v4l2-
> core/videobuf2-memops.c
> > index 1cd322e..4bb8424 100644
> > --- a/drivers/media/v4l2-core/videobuf2-memops.c
> > +++ b/drivers/media/v4l2-core/videobuf2-memops.c
> > @@ -96,10 +96,10 @@ static void vb2_common_vm_open(struct
> vm_area_struct *vma)
> > struct vb2_vmarea_handler *h = vma->vm_private_data;
> >
> > pr_debug("%s: %p, refcount: %d, vma: %08lx-%08lx\n",
> > - __func__, h, atomic_read(h->refcount), vma->vm_start,
> > + __func__, h, refcount_read(h->refcount), vma->vm_start,
> > vma->vm_end);
> >
> > - atomic_inc(h->refcount);
> > + refcount_inc(h->refcount);
> > }
> >
> > /**
> > @@ -114,7 +114,7 @@ static void vb2_common_vm_close(struct
> vm_area_struct *vma)
> > struct vb2_vmarea_handler *h = vma->vm_private_data;
> >
> > pr_debug("%s: %p, refcount: %d, vma: %08lx-%08lx\n",
> > - __func__, h, atomic_read(h->refcount), vma->vm_start,
> > + __func__, h, refcount_read(h->refcount), vma->vm_start,
> > vma->vm_end);
> >
> > h->put(h->arg);
> > diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-
> memops.h
> > index 36565c7a..a6ed091 100644
> > --- a/include/media/videobuf2-memops.h
> > +++ b/include/media/videobuf2-memops.h
> > @@ -16,6 +16,7 @@
> >
> > #include <media/videobuf2-v4l2.h>
> > #include <linux/mm.h>
> > +#include <linux/refcount.h>
> >
> > /**
> > * struct vb2_vmarea_handler - common vma refcount tracking handler
> > @@ -25,7 +26,7 @@
> > * @arg: argument for @put callback
> > */
> > struct vb2_vmarea_handler {
> > - atomic_t *refcount;
> > + refcount_t *refcount;
>
> This is a pointer to refcount, not refcount itself. The refcount is part of
> a memory type specific struct, the types that you change in the following
> three patches. I guess it would still compile and work as separate patches
> but you'd sure get warnings at least.
Actually it doesn't compile without this patch if I remember it correctly back in past when I was initially splitting the patches per variable.
>
> How about merging this and the three following patches that change the memop
> refcount types?
Sounds good!
Best Regards,
Elena.
>
> > void (*put)(void *arg);
> > void *arg;
> > };
>
> --
> Kind regards,
>
> Sakari Ailus
> e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
