Received: by 2002:a25:4158:0:0:0:0:0 with SMTP id o85csp214876yba; Sat, 20 Apr 2019 00:40:16 -0700 (PDT) X-Google-Smtp-Source: APXvYqw1xV4uvevhHgA4BUSynSDGFdR1s+RoGpAtyqLrUh3ODTT9zrcUtckQqrvlcORbo6N09nXv X-Received: by 2002:a65:44c6:: with SMTP id g6mr7900202pgs.157.1555746016706; Sat, 20 Apr 2019 00:40:16 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1555746016; cv=none; d=google.com; s=arc-20160816; b=0iSHWyNcknsNKKiVLgW1KF0hmQ+aVBzOhtHn2wPEEu88+xHTHvJQQBVvO33ilRFrBt wBNpjcuC23HJMZGz9ZXlFWIDKOLs+k3YSvEQsxfoyJKR0TM0eHdqQe3PKVGkwmHuMYIV qHxkQxA60RehuifVt4atRVkcjePVXJGo+1IDEeNA6WMrJvt0TdFXz8XgLf1bvIhmKdxK FzJLCrmbBih2W+N5+MlYzHlThs38u6KyfLqotv5sxkdNVwx3NUInKWj+m6n3UwZrdjMH rUvx2mUpbuCG0brMY9lSTaWZxvBaGK3CDJmaB8sZZljyIlBoFEwyPA/AuNY8BA+ewGY8 uY2A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:in-reply-to :mime-version:user-agent:date:message-id:from:cc:references:to :subject; bh=aiDQBS690WNAhaUCODOeCE1HtQBN0m6uGxj1T1LZ8wo=; b=Vnx2yznnavAyvS6JYTFCu0HpEatE7jioLSWAYrqn7NFb4jtCTwFKYrOFEztdD8tzQO 6WOHwKvAur/2jngYlUHAtM3UoRZq62/ZvEkiKhNF5bmZMhITw2uH1SPJA1vaifO5UKr8 /HEiFtgOUoJsy9XkwABYwOWLMUJDHigZDkrBcfXTickUHuRXaKx3qKnHM1dOkee2EmGU rt1GKRkPIOcGPMWlULsSKWH6MXEeZjLvuKWlu0bn4T3cobbDr/TscvaqdqbNEiMQE5Ey VvugL3RSJWiLx+pWE5LJU9LhoPVR7/+VXdrll+8+Vglc87ORMSErBQICMdxdIbYmas16 /UVA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id cn16si7748369plb.174.2019.04.20.00.40.01; Sat, 20 Apr 2019 00:40:16 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727779AbfDTHjK (ORCPT + 99 others); Sat, 20 Apr 2019 03:39:10 -0400 Received: from szxga06-in.huawei.com ([45.249.212.32]:34656 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725863AbfDTHjK (ORCPT ); Sat, 20 Apr 2019 03:39:10 -0400 Received: from DGGEMS403-HUB.china.huawei.com (unknown [172.30.72.59]) by Forcepoint Email with ESMTP id D92EC885BE359E474BB9; Sat, 20 Apr 2019 15:39:05 +0800 (CST) Received: from [127.0.0.1] (10.177.19.219) by DGGEMS403-HUB.china.huawei.com (10.3.19.203) with Microsoft SMTP Server id 14.3.408.0; Sat, 20 Apr 2019 15:38:57 +0800 Subject: Re: kernel BUG at kernel/cred.c:434! To: Paul Moore References: <20190415150520.GA13257@redhat.com> <20190417145711.GI32622@redhat.com> <20190417162723.GK32622@redhat.com> <0ca3f4cf-5c64-2fc0-1885-9dbcca2f4b47@schaufler-ca.com> <5CB7E5D4.2060703@huawei.com> <5CB933C4.7000300@huawei.com> <5CB9DC75.7010600@huawei.com> CC: Casey Schaufler , Oleg Nesterov , , "chengjian (D)" , Kees Cook , NeilBrown , Anna Schumaker , "linux-kernel@vger.kernel.org" , Al Viro , "Xiexiuqi (Xie XiuQi)" , Li Bin , "Jason Yan" , Peter Zijlstra , Ingo Molnar , Linux Security Module list , SELinux From: Yang Yingliang Message-ID: <5CBACC8F.8010409@huawei.com> Date: Sat, 20 Apr 2019 15:38:55 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.19.219] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2019/4/20 0:13, Paul Moore wrote: > On Fri, Apr 19, 2019 at 10:34 AM Yang Yingliang > wrote: >> On 2019/4/19 21:24, Paul Moore wrote: >>> On Thu, Apr 18, 2019 at 10:42 PM Yang Yingliang >>> wrote: >>>> On 2019/4/19 10:04, Paul Moore wrote: >>>>> On Wed, Apr 17, 2019 at 10:50 PM Yang Yingliang >>>>> wrote: >>>>>> On 2019/4/18 8:24, Casey Schaufler wrote: >>>>>>> On 4/17/2019 4:39 PM, Paul Moore wrote: >>>>>>>> Since it looks like all three LSMs which implement the setprocattr >>>>>>>> hook are vulnerable I'm open to the idea that proc_pid_attr_write() is >>>>>>>> a better choice for the cred != read_cred check, but I would want to >>>>>>>> make sure John and Casey are okay with that. >>>>>>>> >>>>>>>> John? >>>>>>>> >>>>>>>> Casey? >>>>>>> I'm fine with the change going into proc_pid_attr_write(). >>>>>> The cred != real_cred checking is not enough. >>>>>> >>>>>> Consider this situation, when doing override, cred, real_cred and >>>>>> new_cred are all same: >>>>>> >>>>>> after override_creds() cred == real_cred == new1_cred >>>>> I'm sorry, you've lost me. After override_creds() returns >>>>> current->cred and current->real_cred are not going to be the same, >>>>> yes? >>>> It's possible the new cred is equal to current->real_cred and >>>> current->cred, >>>> so after overrides_creds(), they have the same value. >>> Both task_struct.cred and task_struct.real_cred are pointer values, >>> assuming that one uses prepare_creds() to allocate/initialize a new >>> cred struct for use with override_creds() then the newly created cred >>> should never be equal to task_struct.real_cred. Am I missing >>> something, or are you thinking of something else? >> In do_acct_process(), file->f_cred may equal to current->real_cred, I >> confirm >> it by adding some debug message in do_acct_process() like this: > I would expect that; real_cred is the task's objective DAC > credentials, so using it for f_cred makes sense. > > What we are now talking about is the task's subjective credentials, > which can be overridden via override_creds(), and are what the LSMs > change via proc_pid_attr_write(). I'm not sure you got my point. I was saying cred != real_cred check is not quite right, because the cred can be overridden by a same pointer as my print messages showing. "cred != real_cred" means override_creds() is called, but "cred == real_cred" doesn't mean override_creds() is not called. When we use "cred != real_cred" check, we may lost the situation that cred is overridden by a same pointer. In this case, we will do override_creds() => commit_creds() => revert_creds(), this make cred != real_cred, when a new commit_creds() is called, it also will trigger a BUG_ON(). > >> --- a/kernel/acct.c >> +++ b/kernel/acct.c >> @@ -481,6 +481,7 @@ static void do_acct_process(struct bsd_acct_struct >> *acct) >> flim = current->signal->rlim[RLIMIT_FSIZE].rlim_cur; >> current->signal->rlim[RLIMIT_FSIZE].rlim_cur = RLIM_INFINITY; >> /* Perform file operations on behalf of whoever enabled >> accounting */ >> + pr_info("task:%px new cred:%px real cred:%px cred:%px\n", >> current, file->f_cred, current->real_cred, current->cred); >> orig_cred = override_creds(file->f_cred);