From mboxrd@z Thu Jan  1 00:00:00 1970
From: Fabio M. Di Nitto <fdinitto@redhat.com>
Date: Thu, 08 Sep 2011 08:42:25 +0200
Subject: [Cluster-devel] [PATCH] rgmanager: Fix segfault caused by unref
 race [RHEL6]
In-Reply-To: <1315423361-31947-1-git-send-email-lhh@redhat.com>
References: <1315423361-31947-1-git-send-email-lhh@redhat.com>
Message-ID: <4E6863D1.4060107@redhat.com>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

ACK

On 9/7/2011 9:22 PM, Lon Hohberger wrote:
> Since we only ever open/close the connection in one
> place, there is no need to call ref/unref in the
> separate thread, since we wait for that thread to exit
> before calling DBus *close() and *unref() functions.
> 
> It is possible for notification(s) to be sent during
> shutdown after shutting down the DBus connection.
> Since notifications are best-effort and having the
> DBus thread shut down is a normal condition, we should
> not log errors when this condition occurs.
> 
> The rgm_dbus_notify() function was not locking around
> checks to the private db variable, which was incorrect.
> 
> Resolves: rhbz#697446
> 
> Signed-off-by: Lon Hohberger <lhh@redhat.com>
> Tested-by: Lon Hohberger <lhh@redhat.com>
> ---
>  rgmanager/src/daemons/update-dbus.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/rgmanager/src/daemons/update-dbus.c b/rgmanager/src/daemons/update-dbus.c
> index bff1644..b45113c 100644
> --- a/rgmanager/src/daemons/update-dbus.c
> +++ b/rgmanager/src/daemons/update-dbus.c
> @@ -127,13 +127,11 @@ static void *
>  _dbus_auto_flush(void *arg)
>  {
>  	/* DBus connection functions are thread safe */
> -	dbus_connection_ref(db);
>  	while (dbus_connection_read_write(db, 500)) {
>  		if (!th)
>  			break;	
>  	}
>  
> -	dbus_connection_unref(db);
>  	th = 0;
>  	return NULL;
>  }
> @@ -147,7 +145,7 @@ _rgm_dbus_notify(const char *svcname,
>  		 const char *svclast)
>  {
>  	DBusMessage *msg = NULL;
> -	int ret = -1;
> +	int ret = 0;
>  
>  	pthread_mutex_lock(&mu);
>  
> @@ -155,6 +153,9 @@ _rgm_dbus_notify(const char *svcname,
>  		goto out_unlock;
>  	}
>  
> +	/* Notifications are enabled */
> +	ret = -1;
> +
>  	/* Check to ensure the connection is still valid. If it
>  	 * isn't, clean up and shut down the dbus connection.
>  	 *
> @@ -215,12 +216,17 @@ rgm_dbus_update(char *key, uint64_t view, void *data, uint32_t size)
>  
>  	if (!rgm_dbus_notify)
>  		goto out_free;
> -	if (!db)
> -		goto out_free;
>  	if (view == 1)
>  		goto out_free;
>  	if (size != (sizeof(*st)))
>  		goto out_free;
> +	
> +	pthread_mutex_lock(&mu);
> +	if (!db) {
> +		pthread_mutex_unlock(&mu);
> +		goto out_free;
> +	}
> +	pthread_mutex_unlock(&mu);
>  
>  	st = (rg_state_t *)data;
>  	swab_rg_state_t(st);