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;
MIME-Version: 1.0
References: <b1575d02-fd88-65e6-2f16-70a96985087e@schaufler-ca.com>
 <CAHC9VhSk1_fWooexHK0+5cKum55qTvj3uv5sRE=aJ4mv2GHJZg@mail.gmail.com>
 <CAGXu5jKYmoaR6bxW82ogN4iOeKi8AtEwkvVvCXd_gn3CCwFU2Q@mail.gmail.com>
 <fa325d38-db66-0ae7-0a6c-75934a9b0653@huawei.com> <CAHC9VhSpnSpYLLOcFUFLM=UU8tUTLAzJYb2NgdbN-nLPrh=55g@mail.gmail.com>
 <20190417145711.GI32622@redhat.com> <CAHC9VhSzo4DsMRknOA-4KKYJEir0wW74+quLheTjRw94OmhscA@mail.gmail.com>
 <20190417162723.GK32622@redhat.com> <CAHC9VhT_nk1jmrTj3W=F+1gP1vU3iZsZ=UGOCwdiBLbUsOc9=w@mail.gmail.com>
 <0ca3f4cf-5c64-2fc0-1885-9dbcca2f4b47@schaufler-ca.com> <5CB7E5D4.2060703@huawei.com>
 <CAHC9VhQkrR+m9_dYLg4VG-26TdUnf8XFjbmuVQ70YCOp6DhN_g@mail.gmail.com>
 <5CB933C4.7000300@huawei.com> <CAHC9VhTdJ6uUOjyxrk=+iOkKe=VN+N9DMPtV=OchAY34x95g=w@mail.gmail.com>
 <5CB9DC75.7010600@huawei.com> <CAHC9VhQqxL+LsK8YTD6PpTHaZsZ-PxAq3atdZKQcZLQZQH7t4g@mail.gmail.com>
 <5CBACC8F.8010409@huawei.com> <CAHC9VhSZBh8B+1CPM=PdLdbSFq1ba1ffuOJTgnzE5oBLXUEDxQ@mail.gmail.com>
 <5CBE8FC4.8060400@huawei.com>
In-Reply-To: <5CBE8FC4.8060400@huawei.com>
From:   Paul Moore <paul@paul-moore.com>
Date:   Tue, 23 Apr 2019 16:18:49 -0400
Message-ID: <CAHC9VhRitKcax43NL5JThKqsysyakmarYiC4qTQTkka5SNNHuw@mail.gmail.com>
Subject: Re: kernel BUG at kernel/cred.c:434!
To:     Yang Yingliang <yangyingliang@huawei.com>
Cc:     Casey Schaufler <casey@schaufler-ca.com>,
        Oleg Nesterov <oleg@redhat.com>, john.johansen@canonical.com,
        "chengjian (D)" <cj.chengjian@huawei.com>,
        Kees Cook <keescook@chromium.org>, NeilBrown <neilb@suse.com>,
        Anna Schumaker <Anna.Schumaker@netapp.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        "Xiexiuqi (Xie XiuQi)" <xiexiuqi@huawei.com>,
        Li Bin <huawei.libin@huawei.com>,
        Jason Yan <yanaijie@huawei.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Linux Security Module list 
        <linux-security-module@vger.kernel.org>,
        SELinux <selinux@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Apr 23, 2019 at 12:08 AM Yang Yingliang
<yangyingliang@huawei.com> wrote:
> On 2019/4/23 3:48, Paul Moore wrote:
> > On Sat, Apr 20, 2019 at 3:39 AM Yang Yingliang <yangyingliang@huawei.com> wrote:
> >> I'm not sure you got my point.
> > I went back and looked at your previous emails again to try and
> > understand what you are talking about, and I'm a little confused by
> > some of the output ...
> >
> >> --- 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);
> > Okay, with this patch applied we should the task/cred info when
> > do_acct_process is called.  Got it.
> >
> >> Messages:
> >> [   56.643298] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real
> >> cred:ffff88841ae450c0 cred:ffff88841ae450c0    //They are same.
> > Okay, it looks like do_acct_process() was called and f_cred,
> > real_cred, and cred are all the same.
>
> This is a original message, without patch applied.

The patch I am referring to is your pr_info patch (above).  I'm not
talking about any other patches at the moment; I just want to
understand the example dmesg output you copied into your email.

With that in mind, the message above seems to indicate that
do_acct_process() has been invoked with f_cred, real_cred, and cred
all pointing to the same credentials struct, yes?

> >> [   56.646609] Process accounting resumed
> > It looks like do_acct_process() has called check_free_space() now.  So
> > far so good.

...

> >> [   56.649943] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real
> >> cred:ffff88841c96c300 cred:ffff88841ae450c0
> > Wait a minute ... why are we seeing this again?  Looking at the task
> > pointer and the timestamp, this is the same task exiting and trying to
> > write to the accounting file, yes?  This output is particularly
> > curious since it appears that real_cred has changed; where is this
> > happening?
>
> This is the message when the BUG_ON was triggered without applying any
> fix patch.

The only place in the code that generates this message is the bit of
code that you patches in using the pr_info() patch (above), yes?  If
so, that would seem to indicate that the same task is calling
do_acct_process() twice, yes?  I may be fundamentally misunderstanding
something about process accounting, but I though do_acct_process()
would only be called once for a given task - while it was exiting.
Yes, no?

> If we apply this patch "proc: prevent changes to overridden
> credentials", program
> runs like this:

I'd like to focus on understanding the dmesg output you shared first,
because it doesn't seem to make sense to me.

-- 
paul moore
www.paul-moore.com