linux-kernel - Re: [PATCH 4/4] cpuset,mm: use rwlock to protect task->mempolicy and mems

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, 3 Mar 2010 15:50:04 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	miaox@...fujitsu.com
Cc:	David Rientjes <rientjes@...gle.com>,
	Lee Schermerhorn <lee.schermerhorn@...com>,
	Nick Piggin <npiggin@...e.de>, Paul Menage <menage@...gle.com>,
	Linux-Kernel <linux-kernel@...r.kernel.org>,
	Linux-MM <linux-mm@...ck.org>
Subject: Re: [PATCH 4/4] cpuset,mm: use rwlock to protect task->mempolicy
 and mems_allowed

On Wed, 03 Mar 2010 18:52:39 +0800
Miao Xie <miaox@...fujitsu.com> wrote:

> if MAX_NUMNODES > BITS_PER_LONG, loading/storing task->mems_allowed or mems_allowed in
> task->mempolicy are not atomic operations, and the kernel page allocator gets an empty
> mems_allowed when updating task->mems_allowed or mems_allowed in task->mempolicy. So we
> use a rwlock to protect them to fix this probelm.

Boy, that is one big ugly patch.  Is there no other way of doing this?

>
> ...
>
> --- a/include/linux/mempolicy.h
> +++ b/include/linux/mempolicy.h
> @@ -51,6 +51,7 @@ enum {
>   */
>  #define MPOL_F_SHARED  (1 << 0)	/* identify shared policies */
>  #define MPOL_F_LOCAL   (1 << 1)	/* preferred local allocation */
> +#define MPOL_F_TASK    (1 << 2)	/* identify tasks' policies */

What's this?  It wasn't mentioned in the changelog - I suspect it
should have been?

>
> ...
>
> +int cpuset_mems_allowed_intersects(struct task_struct *tsk1,
> +				   struct task_struct *tsk2)
>  {
> -	return nodes_intersects(tsk1->mems_allowed, tsk2->mems_allowed);
> +	unsigned long flags1, flags2;
> +	int retval;
> +
> +	read_mem_lock_irqsave(tsk1, flags1);
> +	read_mem_lock_irqsave(tsk2, flags2);
> +	retval = nodes_intersects(tsk1->mems_allowed, tsk2->mems_allowed);
> +	read_mem_unlock_irqrestore(tsk2, flags2);
> +	read_mem_unlock_irqrestore(tsk1, flags1);

I suspect this is deadlockable in sufficiently arcane circumstances:
one task takes the locks in a,b order, another task takes them in b,a
order and a third task gets in at the right time and does a
write_lock().  Probably that's not possible for some reason, dunno.  The usual
way of solving this is to always take the locks in
sorted-by-ascending-virtual-address order.


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