2013-07-20 13:14:00

by Fengguang Wu

[permalink] [raw]
Subject: [ipc,shm] BUG: lock held when returning to user space!

Greetings,

I got the below dmesg and the first bad commit is

commit c5d0282a0405b0a81fa3390e4230e4cbb3ced7a2
Author: Davidlohr Bueso <[email protected]>
Date: Fri Jul 19 09:56:58 2013 +1000

ipc,shm: shorten critical region for shmat

Similar to other system calls, acquire the kern_ipc_perm lock after doing
the initial permission and security checks.

Signed-off-by: Davidlohr Bueso <[email protected]>
Tested-by: Sedat Dilek <[email protected]>
Cc: Rik van Riel <[email protected]>
Cc: Manfred Spraul <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>

[ 20.702156]
[ 20.702493] ================================================
[ 20.703511] [ BUG: lock held when returning to user space! ]
[ 20.704532] 3.11.0-rc1-next-20130719 #50 Not tainted
[ 20.705416] ------------------------------------------------
[ 20.706425] trinity-child0/174 is leaving the kernel with locks still held!
[ 20.707638] 1 lock held by trinity-child0/174:
[ 20.708475] #0: (rcu_read_lock){.+.+..}, at: [<ffffffff814a8491>] do_shmat+0xe1/0x500

git bisect start c1f631b9a68251007a6353041ae90f9f7dca771c d03792f9db9b892f494d3aa19d767ddf0365d1ff --
git bisect good 10a3f1f902465ae1320cc95a3284fd3697e05dd8 # 11:14 65+ binfmt_elf.c: use get_random_int() to fix entropy depleting
git bisect bad dac28788378838efb63e37a7eabd7729d97aba6b # 11:32 0- dcache: remove dentries from LRU before putting on dispose list
git bisect good 3140b2ed6dfe5c9e5eca371c77ca85dca05321d4 # 11:50 65+ ipc,shm: introduce shmctl_nolock
git bisect bad 48a91248649fa3327bd8a31c114ee9149a07f3a7 # 12:04 0- staging/lustre/ldlm: convert to shrinkers to count/scan API
git bisect good 98b78126a51aa5d3ee6d5dae5768e0d16deeeaa3 # 12:14 65+ ipc,shm: cleanup do_shmat pasta
git bisect bad 36ccfd799cad33e2edd5c14ac8776b33e63d195b # 12:14 0- ipc: rename ids->rw_mutex
git bisect bad c5d0282a0405b0a81fa3390e4230e4cbb3ced7a2 # 12:14 0- ipc,shm: shorten critical region for shmat
git bisect good 98b78126a51aa5d3ee6d5dae5768e0d16deeeaa3 # 15:34 195+ ipc,shm: cleanup do_shmat pasta
git bisect bad c1f631b9a68251007a6353041ae90f9f7dca771c # 15:34 0- Add linux-next specific files for 20130719
git bisect good 709b465ee655387c4ec056383fa27f16c64f48db # 18:21 195+ Revert "ipc,shm: shorten critical region for shmat"
git bisect good d471ce53b1fab60110e4e9f647a345cea31752de # 18:44 195+ Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rw/uml
git bisect bad c1f631b9a68251007a6353041ae90f9f7dca771c # 18:44 0- Add linux-next specific files for 20130719

Thanks,
Fengguang


Attachments:
(No filename) (2.66 kB)
dmesg-kvm-inn-6308-20130719220433-3.11.0-rc1-next-20130719-50 (102.17 kB)
bisect-c1f631b9a68251007a6353041ae90f9f7dca771c-x86_64-randconfig-x02-0719-BUG:-lock-held-when-returning-to-user-space-88052.log (18.65 kB)
config-3.11.0-rc1-next-20130719 (87.53 kB)
Download all attachments

2013-07-20 16:02:37

by Xiaotian Feng

[permalink] [raw]
Subject: Re: [ipc,shm] BUG: lock held when returning to user space!

On Sat, Jul 20, 2013 at 9:13 PM, Fengguang Wu <[email protected]> wrote:
> Greetings,
>
> I got the below dmesg and the first bad commit is
>
> commit c5d0282a0405b0a81fa3390e4230e4cbb3ced7a2
> Author: Davidlohr Bueso <[email protected]>
> Date: Fri Jul 19 09:56:58 2013 +1000
>
> ipc,shm: shorten critical region for shmat
>
> Similar to other system calls, acquire the kern_ipc_perm lock after doing
> the initial permission and security checks.
>
> Signed-off-by: Davidlohr Bueso <[email protected]>
> Tested-by: Sedat Dilek <[email protected]>
> Cc: Rik van Riel <[email protected]>
> Cc: Manfred Spraul <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
>
> [ 20.702156]
> [ 20.702493] ================================================
> [ 20.703511] [ BUG: lock held when returning to user space! ]
> [ 20.704532] 3.11.0-rc1-next-20130719 #50 Not tainted
> [ 20.705416] ------------------------------------------------
> [ 20.706425] trinity-child0/174 is leaving the kernel with locks still held!
> [ 20.707638] 1 lock held by trinity-child0/174:
> [ 20.708475] #0: (rcu_read_lock){.+.+..}, at: [<ffffffff814a8491>] do_shmat+0xe1/0x500
>

----
ns = current->nsproxy->ipc_ns;
- shp = shm_lock_check(ns, shmid);
+ rcu_read_lock();
+ shp = shm_obtain_object_check(ns, shmid);
if (IS_ERR(shp)) {
err = PTR_ERR(shp);
goto out;
----

If shm_obtain_object_check() failed, goto out will return with
rcu_read_lock() held. I think following patch should cure this.

diff --git a/ipc/shm.c b/ipc/shm.c
index 59f2194..cb2ceda 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -1093,7 +1093,7 @@ long do_shmat(int shmid, char __user *shmaddr,
int shmflg, ulong *raddr,
shp = shm_obtain_object_check(ns, shmid);
if (IS_ERR(shp)) {
err = PTR_ERR(shp);
- goto out;
+ goto out_unlock;
}

err = -EACCES;




> git bisect start c1f631b9a68251007a6353041ae90f9f7dca771c d03792f9db9b892f494d3aa19d767ddf0365d1ff --
> git bisect good 10a3f1f902465ae1320cc95a3284fd3697e05dd8 # 11:14 65+ binfmt_elf.c: use get_random_int() to fix entropy depleting
> git bisect bad dac28788378838efb63e37a7eabd7729d97aba6b # 11:32 0- dcache: remove dentries from LRU before putting on dispose list
> git bisect good 3140b2ed6dfe5c9e5eca371c77ca85dca05321d4 # 11:50 65+ ipc,shm: introduce shmctl_nolock
> git bisect bad 48a91248649fa3327bd8a31c114ee9149a07f3a7 # 12:04 0- staging/lustre/ldlm: convert to shrinkers to count/scan API
> git bisect good 98b78126a51aa5d3ee6d5dae5768e0d16deeeaa3 # 12:14 65+ ipc,shm: cleanup do_shmat pasta
> git bisect bad 36ccfd799cad33e2edd5c14ac8776b33e63d195b # 12:14 0- ipc: rename ids->rw_mutex
> git bisect bad c5d0282a0405b0a81fa3390e4230e4cbb3ced7a2 # 12:14 0- ipc,shm: shorten critical region for shmat
> git bisect good 98b78126a51aa5d3ee6d5dae5768e0d16deeeaa3 # 15:34 195+ ipc,shm: cleanup do_shmat pasta
> git bisect bad c1f631b9a68251007a6353041ae90f9f7dca771c # 15:34 0- Add linux-next specific files for 20130719
> git bisect good 709b465ee655387c4ec056383fa27f16c64f48db # 18:21 195+ Revert "ipc,shm: shorten critical region for shmat"
> git bisect good d471ce53b1fab60110e4e9f647a345cea31752de # 18:44 195+ Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rw/uml
> git bisect bad c1f631b9a68251007a6353041ae90f9f7dca771c # 18:44 0- Add linux-next specific files for 20130719
>
> Thanks,
> Fengguang

2013-07-20 16:46:49

by Davidlohr Bueso

[permalink] [raw]
Subject: Re: [ipc,shm] BUG: lock held when returning to user space!

On Sun, 2013-07-21 at 00:02 +0800, Xiaotian Feng wrote:
> On Sat, Jul 20, 2013 at 9:13 PM, Fengguang Wu <[email protected]> wrote:
> > Greetings,
> >
> > I got the below dmesg and the first bad commit is
> >
> > commit c5d0282a0405b0a81fa3390e4230e4cbb3ced7a2
> > Author: Davidlohr Bueso <[email protected]>
> > Date: Fri Jul 19 09:56:58 2013 +1000
> >
> > ipc,shm: shorten critical region for shmat
> >
> > Similar to other system calls, acquire the kern_ipc_perm lock after doing
> > the initial permission and security checks.
> >
> > Signed-off-by: Davidlohr Bueso <[email protected]>
> > Tested-by: Sedat Dilek <[email protected]>
> > Cc: Rik van Riel <[email protected]>
> > Cc: Manfred Spraul <[email protected]>
> > Signed-off-by: Andrew Morton <[email protected]>
> >
> > [ 20.702156]
> > [ 20.702493] ================================================
> > [ 20.703511] [ BUG: lock held when returning to user space! ]
> > [ 20.704532] 3.11.0-rc1-next-20130719 #50 Not tainted
> > [ 20.705416] ------------------------------------------------
> > [ 20.706425] trinity-child0/174 is leaving the kernel with locks still held!
> > [ 20.707638] 1 lock held by trinity-child0/174:
> > [ 20.708475] #0: (rcu_read_lock){.+.+..}, at: [<ffffffff814a8491>] do_shmat+0xe1/0x500
> >
>
> ----
> ns = current->nsproxy->ipc_ns;
> - shp = shm_lock_check(ns, shmid);
> + rcu_read_lock();
> + shp = shm_obtain_object_check(ns, shmid);
> if (IS_ERR(shp)) {
> err = PTR_ERR(shp);
> goto out;
> ----
>
> If shm_obtain_object_check() failed, goto out will return with
> rcu_read_lock() held. I think following patch should cure this.

Yep that should solve it, sorry about that. Sasha Levin sent out a fix
for it yesterday (offline).

Thanks,
Davidlohr

>
> diff --git a/ipc/shm.c b/ipc/shm.c
> index 59f2194..cb2ceda 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -1093,7 +1093,7 @@ long do_shmat(int shmid, char __user *shmaddr,
> int shmflg, ulong *raddr,
> shp = shm_obtain_object_check(ns, shmid);
> if (IS_ERR(shp)) {
> err = PTR_ERR(shp);
> - goto out;
> + goto out_unlock;
> }
>
> err = -EACCES;
>
>
>
>
> > git bisect start c1f631b9a68251007a6353041ae90f9f7dca771c d03792f9db9b892f494d3aa19d767ddf0365d1ff --
> > git bisect good 10a3f1f902465ae1320cc95a3284fd3697e05dd8 # 11:14 65+ binfmt_elf.c: use get_random_int() to fix entropy depleting
> > git bisect bad dac28788378838efb63e37a7eabd7729d97aba6b # 11:32 0- dcache: remove dentries from LRU before putting on dispose list
> > git bisect good 3140b2ed6dfe5c9e5eca371c77ca85dca05321d4 # 11:50 65+ ipc,shm: introduce shmctl_nolock
> > git bisect bad 48a91248649fa3327bd8a31c114ee9149a07f3a7 # 12:04 0- staging/lustre/ldlm: convert to shrinkers to count/scan API
> > git bisect good 98b78126a51aa5d3ee6d5dae5768e0d16deeeaa3 # 12:14 65+ ipc,shm: cleanup do_shmat pasta
> > git bisect bad 36ccfd799cad33e2edd5c14ac8776b33e63d195b # 12:14 0- ipc: rename ids->rw_mutex
> > git bisect bad c5d0282a0405b0a81fa3390e4230e4cbb3ced7a2 # 12:14 0- ipc,shm: shorten critical region for shmat
> > git bisect good 98b78126a51aa5d3ee6d5dae5768e0d16deeeaa3 # 15:34 195+ ipc,shm: cleanup do_shmat pasta
> > git bisect bad c1f631b9a68251007a6353041ae90f9f7dca771c # 15:34 0- Add linux-next specific files for 20130719
> > git bisect good 709b465ee655387c4ec056383fa27f16c64f48db # 18:21 195+ Revert "ipc,shm: shorten critical region for shmat"
> > git bisect good d471ce53b1fab60110e4e9f647a345cea31752de # 18:44 195+ Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rw/uml
> > git bisect bad c1f631b9a68251007a6353041ae90f9f7dca771c # 18:44 0- Add linux-next specific files for 20130719
> >
> > Thanks,
> > Fengguang

2013-07-27 13:35:01

by Fengguang Wu

[permalink] [raw]
Subject: Re: [ipc,shm] BUG: lock held when returning to user space!

On Sat, Jul 20, 2013 at 09:46:45AM -0700, Davidlohr Bueso wrote:
> On Sun, 2013-07-21 at 00:02 +0800, Xiaotian Feng wrote:
> > On Sat, Jul 20, 2013 at 9:13 PM, Fengguang Wu <[email protected]> wrote:
> > > Greetings,
> > >
> > > I got the below dmesg and the first bad commit is
> > >
> > > commit c5d0282a0405b0a81fa3390e4230e4cbb3ced7a2
> > > Author: Davidlohr Bueso <[email protected]>
> > > Date: Fri Jul 19 09:56:58 2013 +1000
> > >
> > > ipc,shm: shorten critical region for shmat
> > >
> > > Similar to other system calls, acquire the kern_ipc_perm lock after doing
> > > the initial permission and security checks.
> > >
> > > Signed-off-by: Davidlohr Bueso <[email protected]>
> > > Tested-by: Sedat Dilek <[email protected]>
> > > Cc: Rik van Riel <[email protected]>
> > > Cc: Manfred Spraul <[email protected]>
> > > Signed-off-by: Andrew Morton <[email protected]>
> > >
> > > [ 20.702156]
> > > [ 20.702493] ================================================
> > > [ 20.703511] [ BUG: lock held when returning to user space! ]
> > > [ 20.704532] 3.11.0-rc1-next-20130719 #50 Not tainted
> > > [ 20.705416] ------------------------------------------------
> > > [ 20.706425] trinity-child0/174 is leaving the kernel with locks still held!
> > > [ 20.707638] 1 lock held by trinity-child0/174:
> > > [ 20.708475] #0: (rcu_read_lock){.+.+..}, at: [<ffffffff814a8491>] do_shmat+0xe1/0x500
> > >
> >
> > ----
> > ns = current->nsproxy->ipc_ns;
> > - shp = shm_lock_check(ns, shmid);
> > + rcu_read_lock();
> > + shp = shm_obtain_object_check(ns, shmid);
> > if (IS_ERR(shp)) {
> > err = PTR_ERR(shp);
> > goto out;
> > ----
> >
> > If shm_obtain_object_check() failed, goto out will return with
> > rcu_read_lock() held. I think following patch should cure this.
>
> Yep that should solve it, sorry about that. Sasha Levin sent out a fix
> for it yesterday (offline).

What's the patch's status? The bug is still there in linux-next 20130726.

Thanks,
Fengguang

2013-08-02 04:02:55

by Davidlohr Bueso

[permalink] [raw]
Subject: Re: [ipc,shm] BUG: lock held when returning to user space!

On Sat, 2013-07-27 at 21:34 +0800, Fengguang Wu wrote:
> On Sat, Jul 20, 2013 at 09:46:45AM -0700, Davidlohr Bueso wrote:
> > On Sun, 2013-07-21 at 00:02 +0800, Xiaotian Feng wrote:
> > > On Sat, Jul 20, 2013 at 9:13 PM, Fengguang Wu <[email protected]> wrote:
> > > > Greetings,
> > > >
> > > > I got the below dmesg and the first bad commit is
> > > >
> > > > commit c5d0282a0405b0a81fa3390e4230e4cbb3ced7a2
> > > > Author: Davidlohr Bueso <[email protected]>
> > > > Date: Fri Jul 19 09:56:58 2013 +1000
> > > >
> > > > ipc,shm: shorten critical region for shmat
> > > >
> > > > Similar to other system calls, acquire the kern_ipc_perm lock after doing
> > > > the initial permission and security checks.
> > > >
> > > > Signed-off-by: Davidlohr Bueso <[email protected]>
> > > > Tested-by: Sedat Dilek <[email protected]>
> > > > Cc: Rik van Riel <[email protected]>
> > > > Cc: Manfred Spraul <[email protected]>
> > > > Signed-off-by: Andrew Morton <[email protected]>
> > > >
> > > > [ 20.702156]
> > > > [ 20.702493] ================================================
> > > > [ 20.703511] [ BUG: lock held when returning to user space! ]
> > > > [ 20.704532] 3.11.0-rc1-next-20130719 #50 Not tainted
> > > > [ 20.705416] ------------------------------------------------
> > > > [ 20.706425] trinity-child0/174 is leaving the kernel with locks still held!
> > > > [ 20.707638] 1 lock held by trinity-child0/174:
> > > > [ 20.708475] #0: (rcu_read_lock){.+.+..}, at: [<ffffffff814a8491>] do_shmat+0xe1/0x500
> > > >
> > >
> > > ----
> > > ns = current->nsproxy->ipc_ns;
> > > - shp = shm_lock_check(ns, shmid);
> > > + rcu_read_lock();
> > > + shp = shm_obtain_object_check(ns, shmid);
> > > if (IS_ERR(shp)) {
> > > err = PTR_ERR(shp);
> > > goto out;
> > > ----
> > >
> > > If shm_obtain_object_check() failed, goto out will return with
> > > rcu_read_lock() held. I think following patch should cure this.
> >
> > Yep that should solve it, sorry about that. Sasha Levin sent out a fix
> > for it yesterday (offline).
>
> What's the patch's status? The bug is still there in linux-next 20130726.

Andrew, unless you have an objection (or have already done so), could
you pickup Sasha's fix?

Thanks,
Davidlohr