linux-kernel - Re: [PATCH -mm -next] ipc,sem: fix lockdep false positive

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 for Android: free password hash cracker in your pocket

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

Message-ID: <51534FF3.2010007@surriel.com>
Date:	Wed, 27 Mar 2013 16:00:51 -0400
From:	Rik van Riel <riel@...riel.com>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	Michel Lespinasse <walken@...gle.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	torvalds@...ux-foundation.org, davidlohr.bueso@...com,
	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	hhuang@...hat.com, jason.low2@...com, lwoodman@...hat.com,
	chegu_vinod@...com, Dave Jones <davej@...hat.com>,
	benisty.e@...il.com, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH -mm -next] ipc,sem: fix lockdep false positive

On 03/27/2013 04:42 AM, Peter Zijlstra wrote:
> On Tue, 2013-03-26 at 11:19 -0400, Rik van Riel wrote:
>>> Maybe something like:
>>>
>>> void sma_lock(struct sem_array *sma) /* global */
>>> {
>>>        int i;
>>>
>>>        sma->global_locked = 1;
>>>        smp_wmb(); /* can we merge with the LOCK ? */
>>>        spin_lock(&sma->global_lock);
>>>
>>>        /* wait for all local locks to go away */
>>>        for (i = 0; i < sma->sem_nsems; i++)
>>>                spin_unlock_wait(&sem->sem_base[i]->lock);
>>> }
>>>
>>> void sma_lock_one(struct sem_array *sma, int nr) /* local */
>>> {
>>>        smp_rmb(); /* pairs with wmb in sma_lock() */
>>>        if (unlikely(sma->global_locked)) { /* wait for global lock */
>>>                while (sma->global_locked)
>>>                        spin_unlock_wait(&sma->global_lock);
>>>        }
>>>        spin_lock(&sma->sem_base[nr]->lock);
>>> }
>
> I since realized there's an ordering problem with ->global_locked, we
> need to use spin_is_locked() or somesuch.
>
> Two competing sma_lock() operations will screw over the separate
> variable.

There may be another problem with your idea.

If there are two single locks coming in for the same semaphore,
the first one holds the lock, while the second one is spinning
on the lock.

The global lock is spinning on spin_unlock_wait, which may end
up finishing after the first single lock holder unlocks, right
before the second single lock holder grabs the lock.

At that point, you have both a process that thinks it holds the
global lock, and a process that thinks it holds a single lock.

To prevent against this, the single lock probably needs to test
whether the global lock is locked, after it acquires the local
lock.

If the global lock is locked, it needs to unlock its single lock,
and then do a spin_unlock_wait on the global lock, before trying
again from the start.

Would that work?

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