linux-kernel - Re: [PATCH 03/30] ocfs2: Remove mount/unmount votes

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]

Date:	Wed, 23 Jan 2008 14:05:09 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Mark Fasheh <mark.fasheh@...cle.com>
Cc:	linux-kernel@...r.kernel.org, ocfs2-devel@....oracle.com,
	mark.fasheh@...cle.com
Subject: Re: [PATCH 03/30] ocfs2: Remove mount/unmount votes

> On Thu, 17 Jan 2008 14:35:29 -0800 Mark Fasheh <mark.fasheh@...cle.com> wrote:
> The node maps that are set/unset by these votes are no longer relevant, thus
> we can remove the mount and umount votes. Since those are the last two
> remaining votes, we can also remove the entire vote infrastructure.
> 
> The vote thread has been renamed to the downconvert thread, and the small
> amount of functionality related to managing it has been moved into
> fs/ocfs2/dlmglue.c. All references to votes have been removed or updated.
> 

Locking looks fishy.

>
>...
>
> +static void ocfs2_downconvert_thread_do_work(struct ocfs2_super *osb)
> +{
> +	unsigned long processed;
> +	struct ocfs2_lock_res *lockres;
> +
> +	mlog_entry_void();
> +
> +	spin_lock(&osb->dc_task_lock);
> +	/* grab this early so we know to try again if a state change and
> +	 * wake happens part-way through our work  */
> +	osb->dc_work_sequence = osb->dc_wake_sequence;
> +
> +	processed = osb->blocked_lock_count;
> +	while (processed) {
> +		BUG_ON(list_empty(&osb->blocked_lock_list));
> +
> +		lockres = list_entry(osb->blocked_lock_list.next,
> +				     struct ocfs2_lock_res, l_blocked_list);
> +		list_del_init(&lockres->l_blocked_list);
> +		osb->blocked_lock_count--;
> +		spin_unlock(&osb->dc_task_lock);
> +
> +		BUG_ON(!processed);
> +		processed--;
> +
> +		ocfs2_process_blocked_lock(osb, lockres);
> +
> +		spin_lock(&osb->dc_task_lock);
> +	}
> +	spin_unlock(&osb->dc_task_lock);

Once the lock has been dropped there is (apparently) nothing to prevent
alteration of the list and of ->blocked_lock_count.  If this happens,
either items will be missed or we go BUG.

> +	mlog_exit_void();
> +}
> +
> +static int ocfs2_downconvert_thread_lists_empty(struct ocfs2_super *osb)
> +{
> +	int empty = 0;
> +
> +	spin_lock(&osb->dc_task_lock);
> +	if (list_empty(&osb->blocked_lock_list))
> +		empty = 1;
> +
> +	spin_unlock(&osb->dc_task_lock);
> +	return empty;
> +}

This function appears to be returning a value which is unreliable once the
lock was dropped.

> +static int ocfs2_downconvert_thread_should_wake(struct ocfs2_super *osb)
> +{
> +	int should_wake = 0;
> +
> +	spin_lock(&osb->dc_task_lock);
> +	if (osb->dc_work_sequence != osb->dc_wake_sequence)
> +		should_wake = 1;
> +	spin_unlock(&osb->dc_task_lock);
> +
> +	return should_wake;
> +}

Ditto.

> +int ocfs2_downconvert_thread(void *arg)
> +{
> +	int status = 0;
> +	struct ocfs2_super *osb = arg;
> +
> +	/* only quit once we've been asked to stop and there is no more
> +	 * work available */
> +	while (!(kthread_should_stop() &&
> +		 ocfs2_downconvert_thread_lists_empty(osb))) {

Extra whitespace

> +		wait_event_interruptible(osb->dc_event,
> +					 ocfs2_downconvert_thread_should_wake(osb) ||
> +					 kthread_should_stop());
> +
> +		mlog(0, "downconvert_thread: awoken\n");
> +
> +		ocfs2_downconvert_thread_do_work(osb);
> +	}
> +
> +	osb->dc_task = NULL;
> +	return status;
> +}
> +

--
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/