Hi Elena,

On Mon, Mar 06, 2017 at 04:21:00PM +0200, Elena Reshetova wrote:

quoted

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@intel.com>
Signed-off-by: Hans Liljestrand <redacted>
Signed-off-by: Kees Cook <redacted>
Signed-off-by: David Windsor <redacted>
---
 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

quoted

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)

quoted

 	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)

quoted

 	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

quoted

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.

quoted

 	void			(*put)(void *arg);
 	void			*arg;
 };

--
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk