From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752847AbdATSKC (ORCPT ); Fri, 20 Jan 2017 13:10:02 -0500 Received: from out03.mta.xmission.com ([166.70.13.233]:44178 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752013AbdATSKB (ORCPT ); Fri, 20 Jan 2017 13:10:01 -0500 From: ebiederm@xmission.com (Eric W. Biederman) To: Nikolay Borisov Cc: Dmitry Vyukov , avagin , serge@hallyn.com, Kees Cook , Al Viro , LKML , syzkaller References: Date: Sat, 21 Jan 2017 07:05:49 +1300 In-Reply-To: (Nikolay Borisov's message of "Fri, 20 Jan 2017 15:26:45 +0200") Message-ID: <877f5py5c2.fsf@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1cUddO-0001Aw-Bu;;;mid=<877f5py5c2.fsf@xmission.com>;;;hst=in02.mta.xmission.com;;;ip=101.100.131.98;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19BKh58EpMShjRiC7yttVgxvLn42ggJhh8= X-SA-Exim-Connect-IP: 101.100.131.98 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa07 1397; Body=1 Fuz1=1 Fuz2=1] * 0.4 FVGT_m_MULTI_ODD Contains multiple odd letter combinations X-Spam-DCC: XMission; sa07 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Nikolay Borisov X-Spam-Relay-Country: X-Spam-Timing: total 410 ms - load_scoreonly_sql: 0.04 (0.0%), signal_user_changed: 3.1 (0.8%), b_tie_ro: 2.2 (0.5%), parse: 1.05 (0.3%), extract_message_metadata: 4.3 (1.0%), get_uri_detail_list: 2.2 (0.5%), tests_pri_-1000: 3.7 (0.9%), tests_pri_-950: 1.16 (0.3%), tests_pri_-900: 1.01 (0.2%), tests_pri_-400: 23 (5.6%), check_bayes: 22 (5.4%), b_tokenize: 7 (1.8%), b_tok_get_all: 7 (1.8%), b_comp_prob: 2.2 (0.5%), b_tok_touch_all: 3.1 (0.8%), b_finish: 0.64 (0.2%), tests_pri_0: 357 (87.1%), check_dkim_signature: 0.48 (0.1%), check_dkim_adsp: 3.0 (0.7%), tests_pri_500: 6 (1.4%), rewrite_mail: 0.00 (0.0%) Subject: Re: namespace: deadlock in dec_pid_namespaces X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Nikolay Borisov writes: > On 20.01.2017 15:07, Dmitry Vyukov wrote: >> Hello, >> >> I've got the following deadlock report while running syzkaller fuzzer >> on eec0d3d065bfcdf9cd5f56dd2a36b94d12d32297 of linux-next (on odroid >> device if it matters): I am puzzled I thought we had fixed this with: add7c65ca426 ("pid: fix lockdep deadlock warning due to ucount_lock") But apparently not. We just moved it from hardirq to softirq context. Bah. Thank you very much for the report. Nikolay can you make your change use spinlock_irq? And have put_ucounts do spin_lock_irqsave? That way we just don't care where we call this. I a tired of being clever. Eric > From 86565326b382b41cb988a83791eff1c4d600040b Mon Sep 17 00:00:00 2001 > From: Nikolay Borisov > Date: Fri, 20 Jan 2017 15:21:35 +0200 > Subject: [PATCH] userns: Make ucounts lock softirq-safe > > The ucounts_lock is being used to protect various ucounts lifecycle > management functionalities. However, those services can also be invoked > when a pidns is being freed in an RCU callback (e.g. softirq context). > This can lead to deadlocks. Fix it by making the spinlock disable softirq > > Signed-off-by: Nikolay Borisov > --- > kernel/ucount.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/kernel/ucount.c b/kernel/ucount.c > index b4aaee935b3e..23a44ea894cd 100644 > --- a/kernel/ucount.c > +++ b/kernel/ucount.c > @@ -132,10 +132,10 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid) > struct hlist_head *hashent = ucounts_hashentry(ns, uid); > struct ucounts *ucounts, *new; > > - spin_lock(&ucounts_lock); > + spin_lock_bh(&ucounts_lock); > ucounts = find_ucounts(ns, uid, hashent); > if (!ucounts) { > - spin_unlock(&ucounts_lock); > + spin_unlock_bh(&ucounts_lock); > > new = kzalloc(sizeof(*new), GFP_KERNEL); > if (!new) > @@ -145,7 +145,7 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid) > new->uid = uid; > atomic_set(&new->count, 0); > > - spin_lock(&ucounts_lock); > + spin_lock_bh(&ucounts_lock); > ucounts = find_ucounts(ns, uid, hashent); > if (ucounts) { > kfree(new); > @@ -156,16 +156,16 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid) > } > if (!atomic_add_unless(&ucounts->count, 1, INT_MAX)) > ucounts = NULL; > - spin_unlock(&ucounts_lock); > + spin_unlock_bh(&ucounts_lock); > return ucounts; > } > > static void put_ucounts(struct ucounts *ucounts) > { > if (atomic_dec_and_test(&ucounts->count)) { > - spin_lock(&ucounts_lock); > + spin_lock_bh(&ucounts_lock); > hlist_del_init(&ucounts->node); > - spin_unlock(&ucounts_lock); > + spin_unlock_bh(&ucounts_lock); > > kfree(ucounts); > }