From 90d77fbdd2c5066279f2c7ddeee0980811ba4923 Mon Sep 17 00:00:00 2001 From: Anand Avati Date: Mon, 4 Mar 2013 13:22:25 -0800 Subject: gfapi: dict_unref() xattr_req in fop finish instead of dict_destroy() The current way of calling dict_destroy() at the end of an API fop assumes that xattr_req is not stored/ref'd by any translators in the stack. However when translators like DHT store xattr_req in dht_local_t with a dict_ref() and perform dict_unref() in the unwind path, things get subject to a race. The race is between the woken up thread (by syncop_wake) i.e, the gfapi invoking thread and the thread where the FOP was unwound. As the C stack of STACK_UNWIND unwinds back, dht_local_unref() gets invoked within the DHT_STACK_UNWIND macro. This thread attempts dict_unref, which would be "safe" if it wins the race against the gfapi invoking thread. However if the gfapi invoking thread wins the race, it will perform dict_destroy() first and therefore make dict_unref() within dht_local_unref perform a double free. This is the embarrassing on-screen bug which showed up in a roomful of people during the gluster dev summit demo of qemu/libgfapi integration. Change-Id: I284c93de87cdc128d5801f42c84aa87f753090d4 BUG: 839950 Signed-off-by: Anand Avati Reviewed-on: http://review.gluster.org/4644 Reviewed-by: Jeff Darcy Tested-by: Gluster Build System --- api/src/glfs-fops.c | 8 ++++---- api/src/glfs-resolve.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'api') diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c index be26dc12..84d36485 100644 --- a/api/src/glfs-fops.c +++ b/api/src/glfs-fops.c @@ -268,7 +268,7 @@ out: loc_wipe (&loc); if (xattr_req) - dict_destroy (xattr_req); + dict_unref (xattr_req); if (ret && glfd) { glfs_fd_destroy (glfd); @@ -953,7 +953,7 @@ out: loc_wipe (&loc); if (xattr_req) - dict_destroy (xattr_req); + dict_unref (xattr_req); return ret; } @@ -1059,7 +1059,7 @@ out: loc_wipe (&loc); if (xattr_req) - dict_destroy (xattr_req); + dict_unref (xattr_req); return ret; } @@ -1130,7 +1130,7 @@ out: loc_wipe (&loc); if (xattr_req) - dict_destroy (xattr_req); + dict_unref (xattr_req); return ret; } diff --git a/api/src/glfs-resolve.c b/api/src/glfs-resolve.c index f7754d20..a9b93818 100644 --- a/api/src/glfs-resolve.c +++ b/api/src/glfs-resolve.c @@ -201,7 +201,7 @@ glfs_resolve_component (struct glfs *fs, xlator_t *subvol, inode_t *parent, *iatt = ciatt; out: if (xattr_req) - dict_destroy (xattr_req); + dict_unref (xattr_req); loc_wipe (&loc); -- cgit