| 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
| ||
|
Message-ID: <30552.1280828047@redhat.com>
Date: Tue, 03 Aug 2010 10:34:07 +0100
From: David Howells <dhowells@...hat.com>
To: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc: dhowells@...hat.com, paulmck@...ux.vnet.ibm.com, torvalds@...l.org,
akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org, Jiri Olsa <jolsa@...hat.com>
Subject: Re: [PATCH 2/2] CRED: Fix __task_cred()'s lockdep check and banner comment
Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp> wrote:
> I got below warning. Is this related to this patch?
>
> [ 140.173556] ===================================================
> [ 140.215379] [ INFO: suspicious rcu_dereference_check() usage. ]
> [ 140.216461] ---------------------------------------------------
> [ 140.217530] kernel/signal.c:660 invoked rcu_dereference_check() without protection!
Yes. The patch has uncovered a case of where we should be holding a lock, but
aren't.
Can you try the attached patch?
David
---
From: David Howells <dhowells@...hat.com>
Subject: [PATCH] CRED: Fix RCU warning due to previous patch fixing __task_cred()'s checks
A previous patch:
commit 8f92054e7ca1d3a3ae50fb42d2253ac8730d9b2a
Author: David Howells <dhowells@...hat.com>
Date: Thu Jul 29 12:45:55 2010 +0100
Subject: CRED: Fix __task_cred()'s lockdep check and banner comment
fixed the lockdep checks on __task_cred(). This has shown up a place in the
signalling code where a lock should be held - namely that
check_kill_permission() requires its callers to hold the RCU lock.
It's may be that it would be better to add RCU read lock calls in
group_send_sig_info() only, around the call to check_kill_permission(). On the
other hand, some of the callers are either holding the RCU read lock already,
or have disabled interrupts, in which case, it's just extra overhead to do it
in g_s_s_i().
Without this patch, the following warning can occur:
[ 140.173556] ===================================================
[ 140.215379] [ INFO: suspicious rcu_dereference_check() usage. ]
[ 140.216461] ---------------------------------------------------
[ 140.217530] kernel/signal.c:660 invoked rcu_dereference_check() without protection!
[ 140.218937]
[ 140.218938] other info that might help us debug this:
[ 140.218939]
[ 140.220508]
[ 140.220509] rcu_scheduler_active = 1, debug_locks = 1
[ 140.221991] 1 lock held by init/1:
[ 140.222668] #0: (tasklist_lock){.+.+..}, at: [<c104a0ac>] kill_something_info+0x7c/0x160
[ 140.224709]
[ 140.224711] stack backtrace:
[ 140.225661] Pid: 1, comm: init Not tainted 2.6.35 #1
[ 140.226576] Call Trace:
[ 140.227111] [<c103cca8>] ? printk+0x18/0x20
[ 140.227908] [<c1069884>] lockdep_rcu_dereference+0x94/0xb0
[ 140.228931] [<c104936a>] check_kill_permission+0x15a/0x170
[ 140.229932] [<c104a0ac>] ? kill_something_info+0x7c/0x160
[ 140.230921] [<c1049cca>] group_send_sig_info+0x1a/0x50
[ 140.231866] [<c1049d36>] __kill_pgrp_info+0x36/0x60
[ 140.232780] [<c104a0d0>] kill_something_info+0xa0/0x160
[ 140.233740] [<c10831c5>] ? __call_rcu+0xa5/0x110
[ 140.234596] [<c104b7ee>] sys_kill+0x5e/0x70
[ 140.235387] [<c10d1eee>] ? mntput_no_expire+0x1e/0xa0
[ 140.236329] [<c10bbd10>] ? __fput+0x170/0x220
[ 140.257756] [<c10bbdd9>] ? fput+0x19/0x20
[ 140.258529] [<c137ad94>] ? restore_all_notrace+0x0/0x18
[ 140.259496] [<c11bfb04>] ? trace_hardirqs_on_thunk+0xc/0x10
[ 140.260531] [<c137ad61>] syscall_call+0x7/0xb
[ 144.627841] nfsd: last server has exited, flushing export cache
[ 154.040420] Restarting system.
[ 154.041061] machine restart
Reported-by: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Signed-off-by: David Howells <dhowells@...hat.com>
---
kernel/exit.c | 2 ++
kernel/signal.c | 8 ++++++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/exit.c b/kernel/exit.c
index ceffc67..7858ebf 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -773,6 +773,7 @@ static void forget_original_parent(struct task_struct *father)
exit_ptrace(father);
+ rcu_read_lock();
write_lock_irq(&tasklist_lock);
reaper = find_new_reaper(father);
@@ -791,6 +792,7 @@ static void forget_original_parent(struct task_struct *father)
reparent_leader(father, p, &dead_children);
}
write_unlock_irq(&tasklist_lock);
+ rcu_read_unlock();
BUG_ON(!list_empty(&father->children));
diff --git a/kernel/signal.c b/kernel/signal.c
index 906ae5a..f771156 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -637,7 +637,7 @@ static inline bool si_fromuser(const struct siginfo *info)
/*
* Bad permissions for sending the signal
- * - the caller must hold at least the RCU read lock
+ * - the caller must hold the RCU read lock
*/
static int check_kill_permission(int sig, struct siginfo *info,
struct task_struct *t)
@@ -1127,7 +1127,7 @@ struct sighand_struct *lock_task_sighand(struct task_struct *tsk, unsigned long
/*
* send signal info to all the members of a group
- * - the caller must hold the RCU read lock at least
+ * - the caller must hold the RCU read lock
*/
int group_send_sig_info(int sig, struct siginfo *info, struct task_struct *p)
{
@@ -1151,11 +1151,13 @@ int __kill_pgrp_info(int sig, struct siginfo *info, struct pid *pgrp)
success = 0;
retval = -ESRCH;
+ rcu_read_lock();
do_each_pid_task(pgrp, PIDTYPE_PGID, p) {
int err = group_send_sig_info(sig, info, p);
success |= !err;
retval = err;
} while_each_pid_task(pgrp, PIDTYPE_PGID, p);
+ rcu_read_unlock();
return success ? 0 : retval;
}
@@ -1261,6 +1263,7 @@ static int kill_something_info(int sig, struct siginfo *info, pid_t pid)
int retval = 0, count = 0;
struct task_struct * p;
+ rcu_read_lock();
for_each_process(p) {
if (task_pid_vnr(p) > 1 &&
!same_thread_group(p, current)) {
@@ -1270,6 +1273,7 @@ static int kill_something_info(int sig, struct siginfo *info, pid_t pid)
retval = err;
}
}
+ rcu_read_unlock();
ret = count ? retval : -ESRCH;
}
read_unlock(&tasklist_lock);
--
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/
Powered by blists - more mailing lists