linux-kernel - Re: [PATCH] uids: Prevent tear down race

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20091102125730.GB5495@linux.vnet.ibm.com>
Date:	Mon, 2 Nov 2009 18:27:30 +0530
From:	Dhaval Giani <dhaval@...ux.vnet.ibm.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	"Paul E. McKenney" <paulmck@...ibm.com>,
	Kay Sievers <kay.sievers@...y.org>, stable@...nel.org
Subject: Re: [PATCH] uids: Prevent tear down race

On Mon, Nov 02, 2009 at 12:09:40PM -0000, Thomas Gleixner wrote:
> Ingo triggered the following warning:
> 
> WARNING: at lib/debugobjects.c:255 debug_print_object+0x42/0x50()
> Hardware name: System Product Name
> ODEBUG: init active object type: timer_list
> Modules linked in:
> Pid: 2619, comm: dmesg Tainted: G        W  2.6.32-rc5-tip+ #5298
> Call Trace:
>  [<81035443>] warn_slowpath_common+0x6a/0x81
>  [<8120e483>] ? debug_print_object+0x42/0x50
>  [<81035498>] warn_slowpath_fmt+0x29/0x2c
>  [<8120e483>] debug_print_object+0x42/0x50
>  [<8120ec2a>] __debug_object_init+0x279/0x2d7
>  [<8120ecb3>] debug_object_init+0x13/0x18
>  [<810409d2>] init_timer_key+0x17/0x6f
>  [<81041526>] free_uid+0x50/0x6c
>  [<8104ed2d>] put_cred_rcu+0x61/0x72
>  [<81067fac>] rcu_do_batch+0x70/0x121
> 
> debugobjects warns about an enqueued timer being initialized. If
> CONFIG_USER_SCHED=y the user management code uses delayed work to
> remove the user from the hash table and tear down the sysfs objects.
> 
> free_uid is called from RCU and initializes/schedules delayed work if
> the usage count of the user_struct is 0. The init/schedule happens
> outside of the uidhash_lock protected region which allows a concurrent
> caller of find_user() to reference the about to be destroyed
> user_struct w/o preventing the work from being scheduled. If the next
> free_uid call happens before the work timer expired then the active
> timer is initialized and the work scheduled again.
> 
> The race was introduced in commit 5cb350ba (sched: group scheduling,
> sysfs tunables) and made more prominent by commit 3959214f (sched:
> delayed cleanup of user_struct)
> 
> Move the init/schedule_delayed_work inside of the uidhash_lock
> protected region to prevent the race.
> 
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Paul E. McKenney <paulmck@...ibm.com>
> Cc: Kay Sievers <kay.sievers@...y.org>
> Cc: Dhaval Giani <dhaval@...ux.vnet.ibm.com>

Acked-by: Dhaval Giani <dhaval@...ux.vnet.ibm.com>

> ---
>  kernel/user.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6/kernel/user.c
> ===================================================================
> --- linux-2.6.orig/kernel/user.c
> +++ linux-2.6/kernel/user.c
> @@ -330,9 +330,9 @@ done:
>   */
>  static void free_user(struct user_struct *up, unsigned long flags)
>  {
> -	spin_unlock_irqrestore(&uidhash_lock, flags);
>  	INIT_DELAYED_WORK(&up->work, cleanup_user_struct);
>  	schedule_delayed_work(&up->work, msecs_to_jiffies(1000));
> +	spin_unlock_irqrestore(&uidhash_lock, flags);
>  }
> 
>  #else	/* CONFIG_USER_SCHED && CONFIG_SYSFS */
> 

-- 
regards,
Dhaval
--
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/