public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: Deadlock on the mm->mmap_sem
@ 2001-09-17 21:50 Manfred Spraul
  2001-09-17 23:39 ` Linus Torvalds
       [not found] ` <200109172339.f8HNd5W13244@penguin.transmeta.com>
  0 siblings, 2 replies; 50+ messages in thread
From: Manfred Spraul @ 2001-09-17 21:50 UTC (permalink / raw)
  To: "Ulrich Weigand"; +Cc: linux-kernel

> What happens is that proc_pid_read_maps grabs the mmap_sem as a
> reader, and *while it holds the lock*, does a copy_to_user.  This can
> of course page-fault, and the handler will also grab the mmap_sem
> (if it is the same task).

Ok, that's a bug.
You must not call copy_to_user with the mmap semaphore acquired - linux
semaphores are not recursive.

> Any ideas how to fix this?  Should proc_pid_read_maps just drop the
> lock before copy_to_user?

Yes, and preferable switch to multiline copies - a full page temporary
buffer is allocated, transfering data on a line-by-line base is way too
much overhead (and the current volatile_task is an ugly hack).

--
    Manfred






^ permalink raw reply	[flat|nested] 50+ messages in thread
[parent not found: <masp0008@stud.uni-sb.de>]
* Re: Deadlock on the mm->mmap_sem
@ 2001-09-18 13:22 Ulrich Weigand
  0 siblings, 0 replies; 50+ messages in thread
From: Ulrich Weigand @ 2001-09-18 13:22 UTC (permalink / raw)
  To: Manfred Spraul; +Cc: Andrea Arcangeli, Linus Torvalds, dhowells, linux-kernel

Manfred Spraul wrote:

>+   if (retval > count) BUG();
>+   if (copy_to_user(buf, kbuf, retval)) {
>+        retval = -EFAULT;
>+   } else {
>+        *ppos = (lineno << MAPS_LINE_SHIFT) + loff;
>    }
>    up_read(&mm->mmap_sem);

The copy_to_user is still done with the lock held ...  I guess you just
forgot to move the up_read() up before the copy_to_user(), right?


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 50+ messages in thread
* Deadlock on the mm->mmap_sem
@ 2001-09-17 20:57 Ulrich Weigand
  0 siblings, 0 replies; 50+ messages in thread
From: Ulrich Weigand @ 2001-09-17 20:57 UTC (permalink / raw)
  To: linux-kernel

Hello,

we're experiencing deadlocks on the mm->mmap_sem which appear to be
caused by proc_pid_read_maps (on S/390, but I believe this is arch-
independent).

What happens is that proc_pid_read_maps grabs the mmap_sem as a reader,
and *while it holds the lock*, does a copy_to_user.  This can of course
page-fault, and the handler will also grab the mmap_sem (if it is the
same task).

Now, normally this just works because both are readers.  However, on SMP
it might just so happen that another thread sharing the mm wants to grab
the lock as a writer after proc_pid_read_maps grabbed it as reader, but
before the page fault handler grabs it.

In that situation, that second thread blocks (because there's already a
writer), and then the first thread blocks in the page fault handler
(because a writer is pending).  Instant deadlock ...

B.t.w. S/390 uses the generic spinlock based rwsem code, if this is of
relevance.

Any ideas how to fix this?  Should proc_pid_read_maps just drop the lock
before copy_to_user?


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 50+ messages in thread

end of thread, other threads:[~2001-09-22 21:06 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2001-09-17 21:50 Deadlock on the mm->mmap_sem Manfred Spraul
2001-09-17 23:39 ` Linus Torvalds
     [not found] ` <200109172339.f8HNd5W13244@penguin.transmeta.com>
2001-09-18  0:01   ` Andrea Arcangeli
2001-09-18  7:31     ` Manfred Spraul
2001-09-18  7:55       ` Andrea Arcangeli
2001-09-18  8:18         ` David Howells
2001-09-18  9:32         ` David Howells
2001-09-18  9:37         ` Manfred Spraul
2001-09-18  9:49         ` Arjan van de Ven
2001-09-18 12:53         ` Manfred Spraul
2001-09-18 14:13           ` David Howells
2001-09-18 14:49             ` Alan Cox
2001-09-18 15:26               ` David Howells
2001-09-18 15:46                 ` Alan Cox
2001-09-18 15:11             ` David Howells
2001-09-18 16:49             ` Linus Torvalds
2001-09-19  9:51               ` David Howells
2001-09-19 12:49                 ` Andrea Arcangeli
2001-09-19 14:08               ` Manfred Spraul
2001-09-19 14:51               ` David Howells
2001-09-19 15:18                 ` Manfred Spraul
2001-09-19 14:53               ` David Howells
2001-09-19 18:03                 ` Andrea Arcangeli
2001-09-19 18:16                   ` Benjamin LaHaise
2001-09-19 18:27                     ` David Howells
2001-09-19 18:48                       ` Andrea Arcangeli
2001-09-19 18:45                     ` Andrea Arcangeli
2001-09-19 21:14                       ` Benjamin LaHaise
2001-09-19 22:07                         ` Andrea Arcangeli
2001-09-19 18:19                   ` Manfred Spraul
2001-09-20  2:07                     ` Andrea Arcangeli
2001-09-20  4:37                       ` Andrea Arcangeli
2001-09-20  7:05                       ` David Howells
2001-09-20  7:19                         ` Andrea Arcangeli
2001-09-20  8:01                           ` David Howells
2001-09-20  8:09                             ` Andrea Arcangeli
2001-09-19 18:26                   ` David Howells
2001-09-19 18:47                     ` Andrea Arcangeli
2001-09-19 23:25                       ` David Howells
2001-09-19 23:34                         ` Andrea Arcangeli
2001-09-19 23:46                           ` Andrea Arcangeli
2001-09-19 23:24                 ` [PATCH] attempt #2 (Re: Deadlock on the mm->mmap_sem) David Howells
2001-09-19 14:58               ` Deadlock on the mm->mmap_sem David Howells
     [not found] <masp0008@stud.uni-sb.de>
2001-09-20 10:57 ` Studierende der Universitaet des Saarlandes
2001-09-20 12:40   ` David Howells
2001-09-20 18:24   ` Andrea Arcangeli
2001-09-20 21:43     ` Manfred Spraul
2001-09-22 21:06     ` Manfred Spraul
  -- strict thread matches above, loose matches on Subject: below --
2001-09-18 13:22 Ulrich Weigand
2001-09-17 20:57 Ulrich Weigand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox