SMP race handling is broken in key_user_lookup() in security/keys/key.c
(if CONFIG_KEYS is set to 'y'). This came up on our Samba servers, but is
not restricted to samba, though samba is probably the only software which
is likely to trigger this repeatedly (and it did happen allready four
times here in University of Helsinki, CS department).
However, it only takes two setreuid() calls at the same instant, so this
may be responsible for some other mysterious random crashes.
This is the same bug which was previously raported to LKML here (found by
google):
http://www.ussg.iu.edu/hypermail/linux/kernel/0502.2/0521.html
Here is a small test program, which can be used to trigger the bug and
crash the machine where it is run. It might take a few seconds:
#include<unistd.h>
#include<stdio.h>
int main() {
int i;
fork();
while(1) {
for(i=0;i<60000;i++) { setreuid(i,0); }
putchar('.'); fflush(stdout);
};
}
The (rather obvious) problem is that key_user_lookup() does not properly
re-initialize the user lookup if there was a race.
This patch applies to vanilla 2.6.11.7 and latest fedora kernel
2.6.11-1.14_FC3. When applied, the test program runs just fine (and does
nothing useful).
Please Cc: any replies to me, since I am not a regular kernel hacker and
do not subscribe LKML.
Signed-Off-By: Jani Jaakkola <[email protected]>
--- linux-2.6.11/security/keys/key.c.orig Wed Mar 2 09:38:25 2005
+++ linux-2.6.11/security/keys/key.c Tue Apr 12 18:19:50 2005
@@ -56,10 +56,12 @@
struct key_user *key_user_lookup(uid_t uid)
{
struct key_user *candidate = NULL, *user;
- struct rb_node *parent = NULL;
- struct rb_node **p = &key_user_tree.rb_node;
+ struct rb_node *parent;
+ struct rb_node **p;
try_again:
+ parent = NULL;
+ p = &key_user_tree.rb_node;
spin_lock(&key_user_lock);
/* search the tree for a user record with a matching UID */
tis 2005-04-12 klockan 21:58 +0300 skrev Jani Jaakkola:
> SMP race handling is broken in key_user_lookup() in security/keys/key.c
> (if CONFIG_KEYS is set to 'y'). This came up on our Samba servers, but is
> not restricted to samba, though samba is probably the only software which
> is likely to trigger this repeatedly (and it did happen allready four
> times here in University of Helsinki, CS department).
>
> However, it only takes two setreuid() calls at the same instant, so this
> may be responsible for some other mysterious random crashes.
>
> This is the same bug which was previously raported to LKML here (found by
> google):
> http://www.ussg.iu.edu/hypermail/linux/kernel/0502.2/0521.html
>
> Here is a small test program, which can be used to trigger the bug and
> crash the machine where it is run. It might take a few seconds:
>
> #include<unistd.h>
> #include<stdio.h>
> int main() {
> int i;
> fork();
> while(1) {
> for(i=0;i<60000;i++) { setreuid(i,0); }
> putchar('.'); fflush(stdout);
> };
> }
>
> The (rather obvious) problem is that key_user_lookup() does not properly
> re-initialize the user lookup if there was a race.
>
> This patch applies to vanilla 2.6.11.7 and latest fedora kernel
> 2.6.11-1.14_FC3. When applied, the test program runs just fine (and does
> nothing useful).
A fix went into mainline for this two months ago (post 2.6.11), but I
probably should have sent it into -stable aswell.
For your own sake always use the latest kernel when looking at
problems/fixes, things move fast around here :)
Jani Jaakkola <[email protected]> wrote:
>
> SMP race handling is broken in key_user_lookup() in security/keys/key.c
This was fixed post-2.6.11. Can you confirm that 2.6.12-rc2 works OK?
This is the patch we used. It should go into -stable if it's not already
there.
From: Alexander Nyberg <[email protected]>
I looked at some of the oops reports against keyrings, I think the problem
is that the search isn't restarted after dropping the key_user_lock, *p
will still be NULL when we get back to try_again and look through the tree.
It looks like the intention was that the search start over from scratch.
Signed-off-by: Alexander Nyberg <[email protected]>
Cc: David Howells <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
25-akpm/security/keys/key.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletion(-)
diff -puN security/keys/key.c~race-against-parent-deletion-in-key_user_lookup security/keys/key.c
--- 25/security/keys/key.c~race-against-parent-deletion-in-key_user_lookup 2005-03-10 00:38:38.000000000 -0800
+++ 25-akpm/security/keys/key.c 2005-03-10 00:38:38.000000000 -0800
@@ -57,9 +57,10 @@ struct key_user *key_user_lookup(uid_t u
{
struct key_user *candidate = NULL, *user;
struct rb_node *parent = NULL;
- struct rb_node **p = &key_user_tree.rb_node;
+ struct rb_node **p;
try_again:
+ p = &key_user_tree.rb_node;
spin_lock(&key_user_lock);
/* search the tree for a user record with a matching UID */
_
Jani Jaakkola <[email protected]> wrote:
> SMP race handling is broken in key_user_lookup() in security/keys/key.c
> (if CONFIG_KEYS is set to 'y').
A patch very much like the one you proposed is already resident in the latest
-rc kernels. Thanks anyway:-)
David
* Andrew Morton ([email protected]) wrote:
> Jani Jaakkola <[email protected]> wrote:
> >
> > SMP race handling is broken in key_user_lookup() in security/keys/key.c
>
> This was fixed post-2.6.11. Can you confirm that 2.6.12-rc2 works OK?
>
> This is the patch we used. It should go into -stable if it's not already
> there.
No, it's not in -stable, queued up, thanks.
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net