linux-kernel - Re: [PATCH 1/1] EDAC: fix edac core deadlock when removing a device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20081218144342.4f033416.akpm@linux-foundation.org>
Date:	Thu, 18 Dec 2008 14:43:42 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Harry Ciao <qingtao.cao@...driver.com>
Cc:	linux-kernel@...r.kernel.org,
	bluesmoke-devel@...ts.sourceforge.net,
	Doug Thompson <norsk5@...oo.com>
Subject: Re: [PATCH 1/1] EDAC: fix edac core deadlock when removing a device

On Thu, 18 Dec 2008 13:09:18 +0800
Harry Ciao <qingtao.cao@...driver.com> wrote:

> To remove an edac device, all pending work must be complete. At that point, 
> it is safe to remove the edac_dev structure.
> 
> If the pending work is not properly cleared and proper care is not taken 
> when waiting for it's completion, the following stack trace result:
>
> ...
> 
> --- a/drivers/edac/edac_device.c
> +++ b/drivers/edac/edac_device.c
> @@ -394,6 +394,12 @@ static void edac_device_workq_function(struct work_struct *work_req)
>  
>  	mutex_lock(&device_ctls_mutex);
>  
> +	/* If we are being removed, bail out immediately */
> +	if (edac_dev->op_state == OP_OFFLINE) {
> +		mutex_unlock(&device_ctls_mutex);
> +		return;
> +	}
> +
>  	/* Only poll controllers that are running polled and have a check */
>  	if ((edac_dev->op_state == OP_RUNNING_POLL) &&
>  		(edac_dev->edac_check != NULL)) {
> @@ -585,14 +591,14 @@ struct edac_device_ctl_info *edac_device_del_device(struct device *dev)
>  	/* mark this instance as OFFLINE */
>  	edac_dev->op_state = OP_OFFLINE;
>  
> -	/* clear workq processing on this instance */
> -	edac_device_workq_teardown(edac_dev);
> -
>  	/* deregister from global list */
>  	del_edac_device_from_global_list(edac_dev);
>  
>  	mutex_unlock(&device_ctls_mutex);
>  
> +	/* clear workq processing on this instance */
> +	edac_device_workq_teardown(edac_dev);
> +
>  	/* Tear down the sysfs entries for this instance */
>  	edac_device_remove_sysfs(edac_dev);
>  

Is the change to edac_device_workq_function() necessary? 
edac_device_workq_teardown()'s call to cancel_delayed_work() will
ensure that edac_device_workq_function() isn't running.

Incidentally, edac_device_workq_teardown() is an open-coded
cancel_delayed_work_sync().

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/