Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1585445 > unrolled thread

[PATCH 2/4] fs, afs: convert afs_vlocation.usage from atomic_t to refcount_t

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/4] fs, afs: convert afs_vlocation.usage from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-02-21 16:50 +0100
    Re: [PATCH 2/4] fs, afs: convert afs_vlocation.usage from atomic_t to refcount_t David Howells <dhowells@redhat.com> - 2017-02-22 17:30 +0100

#1585445 — [PATCH 2/4] fs, afs: convert afs_vlocation.usage from atomic_t to refcount_t

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>
---
 fs/afs/internal.h  |  4 ++--
 fs/afs/proc.c      |  2 +-
 fs/afs/vlocation.c | 16 ++++++++--------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index e77fd4d..50cd1a6 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -246,7 +246,7 @@ struct afs_cache_vhash {
  * AFS volume location record
  */
 struct afs_vlocation {
-	atomic_t		usage;
+	refcount_t		usage;
 	time_t			time_of_death;	/* time at which put reduced usage to 0 */
 	struct list_head	link;		/* link in cell volume location list */
 	struct list_head	grave;		/* link in master graveyard list */
@@ -641,7 +641,7 @@ extern int afs_vl_get_entry_by_id(struct in_addr *, struct key *,
 /*
  * vlocation.c
  */
-#define afs_get_vlocation(V) do { atomic_inc(&(V)->usage); } while(0)
+#define afs_get_vlocation(V) do { refcount_inc(&(V)->usage); } while(0)
 
 extern int __init afs_vlocation_update_init(void);
 extern struct afs_vlocation *afs_vlocation_lookup(struct afs_cell *,
diff --git a/fs/afs/proc.c b/fs/afs/proc.c
index 7eec16d..dc195ed 100644
--- a/fs/afs/proc.c
+++ b/fs/afs/proc.c
@@ -461,7 +461,7 @@ static int afs_proc_cell_volumes_show(struct seq_file *m, void *v)
 
 	/* display one cell per line on subsequent lines */
 	seq_printf(m, "%3d %s %08x %08x %08x %s\n",
-		   atomic_read(&vlocation->usage),
+		   refcount_read(&vlocation->usage),
 		   afs_vlocation_states[vlocation->state],
 		   vlocation->vldb.vid[0],
 		   vlocation->vldb.vid[1],
diff --git a/fs/afs/vlocation.c b/fs/afs/vlocation.c
index d7d8dd8..da27a00 100644
--- a/fs/afs/vlocation.c
+++ b/fs/afs/vlocation.c
@@ -176,7 +176,7 @@ static struct afs_vlocation *afs_vlocation_alloc(struct afs_cell *cell,
 	if (vl) {
 		vl->cell = cell;
 		vl->state = AFS_VL_NEW;
-		atomic_set(&vl->usage, 1);
+		refcount_set(&vl->usage, 1);
 		INIT_LIST_HEAD(&vl->link);
 		INIT_LIST_HEAD(&vl->grave);
 		INIT_LIST_HEAD(&vl->update);
@@ -432,7 +432,7 @@ struct afs_vlocation *afs_vlocation_lookup(struct afs_cell *cell,
 found_in_memory:
 	/* found in memory */
 	_debug("found in memory");
-	atomic_inc(&vl->usage);
+	refcount_inc(&vl->usage);
 	spin_unlock(&cell->vl_lock);
 	if (!list_empty(&vl->grave)) {
 		spin_lock(&afs_vlocation_graveyard_lock);
@@ -495,15 +495,15 @@ void afs_put_vlocation(struct afs_vlocation *vl)
 
 	_enter("%s", vl->vldb.name);
 
-	ASSERTCMP(atomic_read(&vl->usage), >, 0);
+	ASSERTCMP(refcount_read(&vl->usage), >, 0);
 
-	if (likely(!atomic_dec_and_test(&vl->usage))) {
+	if (likely(!refcount_dec_and_test(&vl->usage))) {
 		_leave("");
 		return;
 	}
 
 	spin_lock(&afs_vlocation_graveyard_lock);
-	if (atomic_read(&vl->usage) == 0) {
+	if (refcount_read(&vl->usage) == 0) {
 		_debug("buried");
 		list_move_tail(&vl->grave, &afs_vlocation_graveyard);
 		vl->time_of_death = get_seconds();
@@ -566,7 +566,7 @@ static void afs_vlocation_reaper(struct work_struct *work)
 		}
 
 		spin_lock(&vl->cell->vl_lock);
-		if (atomic_read(&vl->usage) > 0) {
+		if (refcount_read(&vl->usage) > 0) {
 			_debug("no reap");
 			list_del_init(&vl->grave);
 		} else {
@@ -641,7 +641,7 @@ static void afs_vlocation_updater(struct work_struct *work)
 
 		vl = list_entry(afs_vlocation_updates.next,
 				struct afs_vlocation, update);
-		if (atomic_read(&vl->usage) > 0)
+		if (refcount_read(&vl->usage) > 0)
 			break;
 		list_del_init(&vl->update);
 	}
@@ -656,7 +656,7 @@ static void afs_vlocation_updater(struct work_struct *work)
 	}
 
 	list_del_init(&vl->update);
-	atomic_inc(&vl->usage);
+	refcount_inc(&vl->usage);
 	spin_unlock(&afs_vlocation_updates_lock);
 
 	/* we can now perform the update */
-- 
2.7.4

[toc] | [next] | [standalone]

#1586297

Elena Reshetova <elena.reshetova@intel.com> 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.

Please hold this patch also.  An assertion failure may happen here too because
afs_vlocation_lookup() may resurrect records with a refcount of 0 from the
cache.

David

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web