mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Manfred Spraul <manfred@colorfullife.com>
To: LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Davidlohr Bueso <dave@stgolabs.net>
Cc: 1vier1@web.de, dvyukov@google.com, mingo@kernel.org,
	peterz@infradead.org, fabf@skynet.be, kernel@kyup.com,
	syzkaller@googlegroups.com,
	Manfred Spraul <manfred@colorfullife.com>
Subject: [PATCH] ipc/sem.c: fix semop()/semop() locking failure
Date: Sun, 18 Dec 2016 19:38:45 +0100	[thread overview]
Message-ID: <1482086325-3189-1-git-send-email-manfred@colorfullife.com> (raw)
In-Reply-To: <aa108199-2208-0ccf-e346-9d02fd2e90bc@colorfullife.com>

Based on the syzcaller test case from dvyukov:
https://gist.githubusercontent.com/dvyukov/d0e5efefe4d7d6daed829f5c3ca26a40/raw/08d0a261fe3c987bed04fbf267e08ba04bd533ea/gistfile1.txt

The slow (i.e.: failure to acquire) syscall exit from semtimedop()
incorrectly assumed that the the same lock is acquired as it was
at the initial syscall entry.

This is wrong:
- thread A: single semop semop(), sleeps
- thread B: multi semop semop(), sleeps
- thread A: woken up by signal/timeout

With this sequence, the initial sem_lock() call locks the
per-semaphore spinlock, the call at the syscall return locks
the global spinlock.

The fix is trivial: Use the return value from sem_lock.

Reported-by: dvyukov@google.com
Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
Fixes: 370b262c896e ("ipc/sem: avoid idr tree lookup for interrupted semop")
Cc: dave@stgolabs.net
---
 ipc/sem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipc/sem.c b/ipc/sem.c
index e08b948..3ec5742 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1977,7 +1977,7 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
 		}
 
 		rcu_read_lock();
-		sem_lock(sma, sops, nsops);
+		locknum = sem_lock(sma, sops, nsops);
 
 		if (!ipc_valid_object(&sma->sem_perm))
 			goto out_unlock_free;
-- 
2.7.4

  reply	other threads:[~2016-12-18 18:38 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-16  9:33 ipc: BUG: sem_unlock unlocks non-locked lock Dmitry Vyukov
2016-12-18 16:28 ` Davidlohr Bueso
2016-12-18 16:29   ` Davidlohr Bueso
2016-12-18 18:32     ` Manfred Spraul
2016-12-18 18:38       ` Manfred Spraul [this message]
2016-12-19  3:45         ` [PATCH] ipc/sem.c: fix semop()/semop() locking failure Davidlohr Bueso
2017-01-07  4:45           ` Mike Galbraith
2016-12-20  6:34       ` [PATCH v2] ipc/sem.c: fix incorrect sem_lock pairing Manfred Spraul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1482086325-3189-1-git-send-email-manfred@colorfullife.com \
    --to=manfred@colorfullife.com \
    --cc=1vier1@web.de \
    --cc=akpm@linux-foundation.org \
    --cc=dave@stgolabs.net \
    --cc=dvyukov@google.com \
    --cc=fabf@skynet.be \
    --cc=kernel@kyup.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=syzkaller@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®