linux-kernel - Re: kernel BUG at kernel/cred.c:434!

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAHC9VhRitKcax43NL5JThKqsysyakmarYiC4qTQTkka5SNNHuw@mail.gmail.com>
Date:   Tue, 23 Apr 2019 16:18:49 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Yang Yingliang <yangyingliang@...wei.com>
Cc:     Casey Schaufler <casey@...aufler-ca.com>,
        Oleg Nesterov <oleg@...hat.com>, john.johansen@...onical.com,
        "chengjian (D)" <cj.chengjian@...wei.com>,
        Kees Cook <keescook@...omium.org>, NeilBrown <neilb@...e.com>,
        Anna Schumaker <Anna.Schumaker@...app.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        "Xiexiuqi (Xie XiuQi)" <xiexiuqi@...wei.com>,
        Li Bin <huawei.libin@...wei.com>,
        Jason Yan <yanaijie@...wei.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Linux Security Module list 
        <linux-security-module@...r.kernel.org>,
        SELinux <selinux@...r.kernel.org>
Subject: Re: kernel BUG at kernel/cred.c:434!

On Tue, Apr 23, 2019 at 12:08 AM Yang Yingliang
<yangyingliang@...wei.com> wrote:
> On 2019/4/23 3:48, Paul Moore wrote:
> > On Sat, Apr 20, 2019 at 3:39 AM Yang Yingliang <yangyingliang@...wei.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