diff mbox

[29/29] drivers, xen: convert grant_map.users from atomic_t to refcount_t

Message ID 1488810076-3754-30-git-send-email-elena.reshetova@intel.com (mailing list archive)
State Superseded
Headers show

Commit Message

Reshetova, Elena March 6, 2017, 2:21 p.m. UTC
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 <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 drivers/xen/gntdev.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Boris Ostrovsky March 6, 2017, 4:58 p.m. UTC | #1
On 03/06/2017 09:21 AM, 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@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>
> ---
>  drivers/xen/gntdev.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>



--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reshetova, Elena March 8, 2017, 1:49 p.m. UTC | #2
> On 03/06/2017 09:21 AM, 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@intel.com>

> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>

> > Signed-off-by: Kees Cook <keescook@chromium.org>

> > Signed-off-by: David Windsor <dwindsor@gmail.com>

> > ---

> >  drivers/xen/gntdev.c | 11 ++++++-----

> >  1 file changed, 6 insertions(+), 5 deletions(-)

> 

> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>


Is there a tree that can take this change? Turns out it is better to propagate changes via separate trees and only leftovers can be taken via Greg's tree.  

Best Regards,
Elena.
Boris Ostrovsky March 8, 2017, 5:45 p.m. UTC | #3
On 03/08/2017 08:49 AM, Reshetova, Elena wrote:
>> On 03/06/2017 09:21 AM, 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@intel.com>
>>> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>> Signed-off-by: David Windsor <dwindsor@gmail.com>
>>> ---
>>>  drivers/xen/gntdev.c | 11 ++++++-----
>>>  1 file changed, 6 insertions(+), 5 deletions(-)
>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Is there a tree that can take this change? Turns out it is better to propagate changes via separate trees and only leftovers can be taken via Greg's tree.  
>

Sure, we can take it via Xen tree for rc3.

-boris
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reshetova, Elena March 9, 2017, 7:19 a.m. UTC | #4
> On 03/08/2017 08:49 AM, Reshetova, Elena wrote:

> >> On 03/06/2017 09:21 AM, 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@intel.com>

> >>> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>

> >>> Signed-off-by: Kees Cook <keescook@chromium.org>

> >>> Signed-off-by: David Windsor <dwindsor@gmail.com>

> >>> ---

> >>>  drivers/xen/gntdev.c | 11 ++++++-----

> >>>  1 file changed, 6 insertions(+), 5 deletions(-)

> >> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

> > Is there a tree that can take this change? Turns out it is better to propagate

> changes via separate trees and only leftovers can be taken via Greg's tree.

> >

> 

> Sure, we can take it via Xen tree for rc3.


Thank you very much!

Best Regards,
Elena.

> 

> -boris
diff mbox

Patch

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 2ef2b61..b183cb2 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -35,6 +35,7 @@ 
 #include <linux/spinlock.h>
 #include <linux/slab.h>
 #include <linux/highmem.h>
+#include <linux/refcount.h>
 
 #include <xen/xen.h>
 #include <xen/grant_table.h>
@@ -85,7 +86,7 @@  struct grant_map {
 	int index;
 	int count;
 	int flags;
-	atomic_t users;
+	refcount_t users;
 	struct unmap_notify notify;
 	struct ioctl_gntdev_grant_ref *grants;
 	struct gnttab_map_grant_ref   *map_ops;
@@ -165,7 +166,7 @@  static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count)
 
 	add->index = 0;
 	add->count = count;
-	atomic_set(&add->users, 1);
+	refcount_set(&add->users, 1);
 
 	return add;
 
@@ -211,7 +212,7 @@  static void gntdev_put_map(struct gntdev_priv *priv, struct grant_map *map)
 	if (!map)
 		return;
 
-	if (!atomic_dec_and_test(&map->users))
+	if (!refcount_dec_and_test(&map->users))
 		return;
 
 	atomic_sub(map->count, &pages_mapped);
@@ -399,7 +400,7 @@  static void gntdev_vma_open(struct vm_area_struct *vma)
 	struct grant_map *map = vma->vm_private_data;
 
 	pr_debug("gntdev_vma_open %p\n", vma);
-	atomic_inc(&map->users);
+	refcount_inc(&map->users);
 }
 
 static void gntdev_vma_close(struct vm_area_struct *vma)
@@ -1003,7 +1004,7 @@  static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma)
 		goto unlock_out;
 	}
 
-	atomic_inc(&map->users);
+	refcount_inc(&map->users);
 
 	vma->vm_ops = &gntdev_vmops;