Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753252AbdLOG5E (ORCPT ); Fri, 15 Dec 2017 01:57:04 -0500 Received: from out01.mta.xmission.com ([166.70.13.231]:33956 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752029AbdLOG5B (ORCPT ); Fri, 15 Dec 2017 01:57:01 -0500 From: ebiederm@xmission.com (Eric W. Biederman) To: Cong Wang Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Al Viro , Andrew Morton , Linus Torvalds , stable@vger.kernel.org References: <20171214201757.5393-1-xiyou.wangcong@gmail.com> Date: Fri, 15 Dec 2017 00:56:40 -0600 In-Reply-To: <20171214201757.5393-1-xiyou.wangcong@gmail.com> (Cong Wang's message of "Thu, 14 Dec 2017 12:17:57 -0800") Message-ID: <87efnwcwd3.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=1ePjvY-0004lw-H3;;;mid=<87efnwcwd3.fsf@xmission.com>;;;hst=in02.mta.xmission.com;;;ip=75.170.127.89;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/9dkraB5lTe3JDJTcbH5Cyh692hR6kFWU= X-SA-Exim-Connect-IP: 75.170.127.89 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.7 XMSubLong Long Subject * 0.0 TVD_RCVD_IP Message was received from an IP address * 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 * [sa06 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject * 0.0 T_TooManySym_02 5+ unique symbols in subject X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Cong Wang X-Spam-Relay-Country: X-Spam-Timing: total 211 ms - load_scoreonly_sql: 0.04 (0.0%), signal_user_changed: 3.0 (1.4%), b_tie_ro: 2.1 (1.0%), parse: 1.02 (0.5%), extract_message_metadata: 20 (9.3%), get_uri_detail_list: 2.6 (1.2%), tests_pri_-1000: 8 (3.7%), tests_pri_-950: 1.16 (0.6%), tests_pri_-900: 0.98 (0.5%), tests_pri_-400: 21 (9.7%), check_bayes: 20 (9.3%), b_tokenize: 7 (3.1%), b_tok_get_all: 6 (2.9%), b_comp_prob: 2.0 (1.0%), b_tok_touch_all: 2.9 (1.4%), b_finish: 0.65 (0.3%), tests_pri_0: 149 (70.6%), check_dkim_signature: 0.45 (0.2%), check_dkim_adsp: 2.8 (1.3%), tests_pri_500: 4.7 (2.2%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH] exit: move exit_task_namespaces() after exit_task_work() 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 Content-Length: 1698 Lines: 53 Cong Wang writes: > syzbot reported we have a use-after-free when mqueue_evict_inode() > is called on __cleanup_mnt() path, where the ipc ns is already > freed by the previous exit_task_namespaces(). We can just move > it after after exit_task_work() to avoid this use-after-free. How does that possibly work. (I haven't seen this syzbot report). Looking at the code we have get_ns_from_inode. Which takes the mq_lock, sees if the pointer is NULL and takes a reference if it is non-NULL. Meanwhile put_ipc_ns calls mq_clear_sbinfo(ns) with the mq_lock held when the count drops to zero. Where is the race in that? The rest of mqueue_evict_inode uses the returned pointer and tests that the pointer is non-NULL before user it. So either szbot is giving you a bad report or there is a subtle race there I am not seeing. The change below is not at all the proper way to fix a subtle race. Eric > > Reported-by: syzbot > Cc: Ingo Molnar > Cc: Al Viro > Cc: Andrew Morton > Cc: Linus Torvalds > Cc: stable@vger.kernel.org > Signed-off-by: Cong Wang > --- > kernel/exit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/exit.c b/kernel/exit.c > index 6b4298a41167..909e43c45158 100644 > --- a/kernel/exit.c > +++ b/kernel/exit.c > @@ -861,8 +861,8 @@ void __noreturn do_exit(long code) > exit_fs(tsk); > if (group_dead) > disassociate_ctty(1); > - exit_task_namespaces(tsk); > exit_task_work(tsk); > + exit_task_namespaces(tsk); > exit_thread(tsk); > > /*