Re: kernel BUG at kernel/cred.c:434!
On Tue, Apr 23, 2019 at 12:08 AM Yang Yingliang
wrote:
> On 2019/4/23 3:48, Paul Moore wrote:
> > On Sat, Apr 20, 2019 at 3:39 AM Yang Yingliang
> > 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:88841a9595c0 new cred:88841ae450c0 real
> >> cred:88841ae450c0 cred:88841ae450c0//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:88841a9595c0 new cred:88841ae450c0 real
> >> cred:88841c96c300 cred:88841ae450c0
> > 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
Re: kernel BUG at kernel/cred.c:434!
On 2019/4/23 3:48, Paul Moore wrote:
On Sat, Apr 20, 2019 at 3:39 AM Yang Yingliang 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:88841a9595c0 new cred:88841ae450c0 real
cred:88841ae450c0 cred:88841ae450c0//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.
[ 56.646609] Process accounting resumed
It looks like do_acct_process() has called check_free_space() now. So
far so good.
[ 56.649943] task:88841a9595c0 new cred:88841ae450c0 real
cred:88841c96c300 cred:88841ae450c0
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.
If we apply this patch "proc: prevent changes to overridden
credentials", program
runs like this:
1. As print message shows, before overriden, the pointer has the
following value:
real_cread=cred=0x88841ae450c0, f_cred=0x88841ae450c0
override_creds() is called in do_acct_process():
...
/* Perform file operations on behalf of whoever enabled accounting */
orig_cred = override_creds(file->f_cred);
...
2. After override_creds(), if (current_cred() != current_real_cred()) is
not work here,
we will call commit_creds() in security_setprocattr().
...
/* Prevent changes to overridden credentials. */
if (current_cred() != current_real_cred()) {
rcu_read_unlock();
return -EBUSY;
}
...
3. After commit_creds(), we have new cred and real_cred.
security_setprocattr()//commit_creds is called here
4. revert_creds() is called in in do_acct_process(), the cred
is reverted to the old value(0x88841ae450c0)
...
current->signal->rlim[RLIMIT_FSIZE].rlim_cur = flim;
revert_creds(orig_cred);
5. After reverting, cred and real_cred are not equal.
If it has a risk to trigger the BUG_ON, when doing another
commit_creds() ?
Re: kernel BUG at kernel/cred.c:434!
On Sat, Apr 20, 2019 at 3:39 AM Yang Yingliang 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:88841a9595c0 new cred:88841ae450c0 real
> cred:88841ae450c0 cred:88841ae450c0//They are same.
Okay, it looks like do_acct_process() was called and f_cred,
real_cred, and cred are all the same.
> [ 56.646609] Process accounting resumed
It looks like do_acct_process() has called check_free_space() now. So
far so good.
> [ 56.649943] task:88841a9595c0 new cred:88841ae450c0 real
> cred:88841c96c300 cred:88841ae450c0
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?
> [ 56.653565] [ cut here ]
> [ 56.655119] kernel BUG at kernel/cred.c:434!
> [ 56.656590] invalid opcode: [#1] SMP PTI
> [ 56.658033] CPU: 2 PID: 4169 Comm: syz-executor.15 Not tainted
> 5.1.0-rc4-00034-g869e3305f23d-dirty #143
> [ 56.661077] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> [ 56.664895] RIP: 0010:commit_creds+0x1eb/0x230
> [ 56.666344] Code: 43 1c 0f 85 08 ff ff ff e9 10 ff ff ff 8b 45 10 39
> 43 10 0f 85 18 ff ff ff 8b 43 20 39 45 20 0f 85 0c ff ff ff e9 14 ff ff
> ff <0f> 0b 48 c7 c7 d0 d2 49 82 e8 17 3b 3e 00 0f 0b 48 c7 c7 c0 d2 49
> [ 56.672410] RSP: 0018:c90003a17b20 EFLAGS: 00010287
> [ 56.674098] RAX: 88841a9595c0 RBX: 88841ae450c0 RCX:
>
> [ 56.676410] RDX: 0001 RSI: 0020 RDI:
> 88841c96ce40
> [ 56.678691] RBP: 0001 R08: 0080 R09:
>
> [ 56.680997] R10: 88841c9265a0 R11: 810d6940 R12:
> 88841a9595c0
> [ 56.681198] task:88841a9195c0 new cred:88841aeaa0c0 real
> cred:88841aeaa0c0 cred:88841aeaa0c0
> [ 56.683293] R13: 0040 R14: 88841c96ce40 R15:
> 0040
> [ 56.683296] FS: 7f5969a5c700() GS:88842fa8()
> knlGS:
> [ 56.683297] CS: 0010 DS: ES: CR0: 80050033
> [ 56.683299] CR2: 7f82742214f0 CR3: 00041cbc0005 CR4:
> 000206e0
> [ 56.683305] Call Trace:
> [ 56.683340] selinux_setprocattr+0x17b/0x480
> [ 56.686513] Process accounting resumed
> [ 56.688849] proc_pid_attr_write+0xc0/0xf0
> [ 56.688857] __kernel_write+0x4f/0xf0
> [ 56.688866] do_acct_process+0x538/0x750
> [ 56.703090] ? __schedule+0x290/0x960
> [ 56.704311] ? __queue_work+0x160/0x5c0
> [ 56.705571] acct_pin_kill+0x1e/0x70
> [ 56.706743] pin_kill+0x81/0x150
> [ 56.707813] ? finish_wait+0x80/0x80
> [ 56.708985] mnt_pin_kill+0x1e/0x30
> [ 56.710127] cleanup_mnt+0x6e/0x70
> [ 56.711247] task_work_run+0x8a/0xb0
> [ 56.712453] do_exit+0x2e0/0xc80
> [ 56.713525] do_group_exit+0x33/0xb0
> [ 56.714701] get_signal+0x143/0x810
> [ 56.715865] do_signal+0x36/0x610
> [ 56.716962] ? __x64_sys_futex+0x134/0x180
> [ 56.718307] ? _copy_to_user+0x22/0x30
> [ 56.719606] exit_to_usermode_loop+0x80/0xe0
> [ 56.721003] do_syscall_64+0x16c/0x180
> [ 56.722242] entry_SYSCALL_64_after_hwframe+0x44/0xa9
--
paul moore
www.paul-moore.com
Re: kernel BUG at kernel/cred.c:434!
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);
Re: kernel BUG at kernel/cred.c:434!
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? -- paul moore www.paul-moore.com
Re: kernel BUG at kernel/cred.c:434!
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().
> --- 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);
--
paul moore
www.paul-moore.com
Re: kernel BUG at kernel/cred.c:434!
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:
--- 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);
Messages:
[ 56.643298] task:88841a9595c0 new cred:88841ae450c0 real
cred:88841ae450c0 cred:88841ae450c0//They are same.
[ 56.646609] Process accounting resumed
[ 56.649943] task:88841a9595c0 new cred:88841ae450c0 real
cred:88841c96c300 cred:88841ae450c0
[ 56.653565] --------[ cut here ]--------
[ 56.655119] kernel BUG at kernel/cred.c:434!
[ 56.656590] invalid opcode: [#1] SMP PTI
[ 56.658033] CPU: 2 PID: 4169 Comm: syz-executor.15 Not tainted
5.1.0-rc4-00034-g869e3305f23d-dirty #143
[ 56.661077] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
[ 56.664895] RIP: 0010:commit_creds+0x1eb/0x230
[ 56.666344] Code: 43 1c 0f 85 08 ff ff ff e9 10 ff ff ff 8b 45 10 39
43 10 0f 85 18 ff ff ff 8b 43 20 39 45 20 0f 85 0c ff ff ff e9 14 ff ff
ff <0f> 0b 48 c7 c7 d0 d2 49 82 e8 17 3b 3e 00 0f 0b 48 c7 c7 c0 d2 49
[ 56.672410] RSP: 0018:c90003a17b20 EFLAGS: 00010287
[ 56.674098] RAX: 88841a9595c0 RBX: 88841ae450c0 RCX:
[ 56.676410] RDX: 0001 RSI: 0020 RDI:
88841c96ce40
[ 56.678691] RBP: 0001 R08: 0080 R09:
[ 56.680997] R10: 88841c9265a0 R11: 810d6940 R12:
88841a9595c0
[ 56.681198] task:88841a9195c0 new cred:88841aeaa0c0 real
cred:88841aeaa0c0 cred:88841aeaa0c0
[ 56.683293] R13: 0040 R14: 88841c96ce40 R15:
0040
[ 56.683296] FS: 7f5969a5c700() GS:88842fa8()
knlGS:
[ 56.683297] CS: 0010 DS: ES: CR0: 80050033
[ 56.683299] CR2: 7f82742214f0 CR3: 00041cbc0005 CR4:
000206e0
[ 56.683305] Call Trace:
[ 56.683340] selinux_setprocattr+0x17b/0x480
[ 56.686513] Process accounting resumed
[ 56.688849] proc_pid_attr_write+0xc0/0xf0
[ 56.688857] __kernel_write+0x4f/0xf0
[ 56.688866] do_acct_process+0x538/0x750
[ 56.703090] ? __schedule+0x290/0x960
[ 56.704311] ? __queue_work+0x160/0x5c0
[ 56.705571] acct_pin_kill+0x1e/0x70
[ 56.706743] pin_kill+0x81/0x150
[ 56.707813] ? finish_wait+0x80/0x80
[ 56.708985] mnt_pin_kill+0x1e/0x30
[ 56.710127] cleanup_mnt+0x6e/0x70
[ 56.711247] task_work_run+0x8a/0xb0
[ 56.712453] do_exit+0x2e0/0xc80
[ 56.713525] do_group_exit+0x33/0xb0
[ 56.714701] get_signal+0x143/0x810
[ 56.715865] do_signal+0x36/0x610
[ 56.716962] ? __x64_sys_futex+0x134/0x180
[ 56.718307] ? _copy_to_user+0x22/0x30
[ 56.719606] exit_to_usermode_loop+0x80/0xe0
[ 56.721003] do_syscall_64+0x16c/0x180
[ 56.722242] entry_SYSCALL_64_after_hwframe+0x44/0xa9
Re: kernel BUG at kernel/cred.c:434!
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. after prepare_creds() new2_cred after commit_creds() becasue the check is false, so cred == real_cred == new2_cred after revert_creds()cred == new1_cred, real_cred == new2_cred It will cause cred != real_cred finally.
Re: kernel BUG at kernel/cred.c:434!
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? > after prepare_creds() new2_cred > after commit_creds() becasue the check is false, so cred == > real_cred == new2_cred > after revert_creds()cred == new1_cred, real_cred == new2_cred > > It will cause cred != real_cred finally. -- paul moore www.paul-moore.com
Re: kernel BUG at kernel/cred.c:434!
On 4/17/19 12:42 PM, Casey Schaufler wrote: On 4/17/2019 9:27 AM, Oleg Nesterov wrote: On 04/17, Paul Moore wrote: On Wed, Apr 17, 2019 at 10:57 AM Oleg Nesterov wrote: On 04/17, Paul Moore wrote: I'm tempted to simply return an error in selinux_setprocattr() if the task's credentials are not the same as its real_cred; What about other modules? I have no idea what smack_setprocattr() is, but it too does prepare_creds/commit creds. it seems that the simplest workaround should simply add the additional cred == real_cred into proc_pid_attr_write(). Yes, that is simple, but I worry about what other LSMs might want to do. While I believe failing if the effective creds are not the same as the real_creds is okay for SELinux (possibly Smack too), I worry about what other LSMs may want to do. After all, proc_pid_attr_write() doesn't change the the creds itself, that is something the specific LSMs do. Yes, but if proc_pid_attr_write() is called with cred != real_cred then something is already wrong? In fact, I think that something is already wrong if it is not called by user-space directly. Too late to ask, but why is this /proc/self/attr/ magic not implemented via syscall(s) ? Shell scripts, for one thing. It's a straightforward and appropriate use of the /proc interface. System calls would require additional change to existing programs, whereas using the /proc interface allows a good deal to be done in the containing scripts. It is fairly awkward/fragile to use these interfaces from shell scripts due to limitations of the interface (e.g. no partial writes, thereby breaking if the shell splits up the data into multiple write() calls) and due to the fact that most of the attributes (except for current) are typically reset/cleared upon exec to prevent undue caller/callee influence. It works well enough for echo -n "somelabel" > /proc/self/attr/current but not much else, and even that doesn't work without the -n due to multiple write() calls. Just for the record, this functionality was originally implemented via separate system calls at least for SELinux (in its original kernel patch), then multiplexed through the single LSM security system call upon porting to LSM (before merging to mainline), but viro and others strongly favored using /proc as the interface for getting/setting any new process attributes. Understandable, but it does impose some limitations, including a dependency on having a writable proc mount in the namespace of any process that needs to set any of these attributes,
Re: kernel BUG at kernel/cred.c:434!
Hi, Casey On 2019/4/18 8:24, Casey Schaufler wrote: On 4/17/2019 4:39 PM, Paul Moore wrote: On Wed, Apr 17, 2019 at 12:27 PM Oleg Nesterov wrote: On 04/17, Paul Moore wrote: On Wed, Apr 17, 2019 at 10:57 AM Oleg Nesterov wrote: On 04/17, Paul Moore wrote: I'm tempted to simply return an error in selinux_setprocattr() if the task's credentials are not the same as its real_cred; What about other modules? I have no idea what smack_setprocattr() is, but it too does prepare_creds/commit creds. it seems that the simplest workaround should simply add the additional cred == real_cred into proc_pid_attr_write(). Yes, that is simple, but I worry about what other LSMs might want to do. While I believe failing if the effective creds are not the same as the real_creds is okay for SELinux (possibly Smack too), I worry about what other LSMs may want to do. After all, proc_pid_attr_write() doesn't change the the creds itself, that is something the specific LSMs do. Yes, but if proc_pid_attr_write() is called with cred != real_cred then something is already wrong? True, or at least I would think so. Looking at the current tree there are three LSMs which implement setprocattr hooks: SELinux, Smack, and AppArmor. I know Casey has already mentioned that he wasn't able to trigger the problem in Smack, but looking at smack_setprocattr() I see the similar commit_creds() usage so I would expect the same problem in Smack; what say you Casey? I say that my test program runs without ill effect. I call acct() with "/proc/self/attr/current", which succeeds and enables accounting just like it is supposed to. I then have the program open "/proc/self/attr/current" and read it, all of which goes swimmingly. When Smack frees a cred it usually does not free any memory of its own, so it is conceivable that I'm just getting lucky. Or, I may not have sufficient debug enabled. Looking at apparmor_setprocattr(), it appears that it too could end up calling commit_creds() via aa_set_current_hat(). 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 after prepare_creds() new2_cred after commit_creds() becasue the check is false, so cred == real_cred == new2_cred after revert_creds()cred == new1_cred, real_cred == new2_cred It will cause cred != real_cred finally. Regards, Yang
Re: kernel BUG at kernel/cred.c:434!
On 4/17/2019 4:39 PM, Paul Moore wrote: On Wed, Apr 17, 2019 at 12:27 PM Oleg Nesterov wrote: On 04/17, Paul Moore wrote: On Wed, Apr 17, 2019 at 10:57 AM Oleg Nesterov wrote: On 04/17, Paul Moore wrote: I'm tempted to simply return an error in selinux_setprocattr() if the task's credentials are not the same as its real_cred; What about other modules? I have no idea what smack_setprocattr() is, but it too does prepare_creds/commit creds. it seems that the simplest workaround should simply add the additional cred == real_cred into proc_pid_attr_write(). Yes, that is simple, but I worry about what other LSMs might want to do. While I believe failing if the effective creds are not the same as the real_creds is okay for SELinux (possibly Smack too), I worry about what other LSMs may want to do. After all, proc_pid_attr_write() doesn't change the the creds itself, that is something the specific LSMs do. Yes, but if proc_pid_attr_write() is called with cred != real_cred then something is already wrong? True, or at least I would think so. Looking at the current tree there are three LSMs which implement setprocattr hooks: SELinux, Smack, and AppArmor. I know Casey has already mentioned that he wasn't able to trigger the problem in Smack, but looking at smack_setprocattr() I see the similar commit_creds() usage so I would expect the same problem in Smack; what say you Casey? I say that my test program runs without ill effect. I call acct() with "/proc/self/attr/current", which succeeds and enables accounting just like it is supposed to. I then have the program open "/proc/self/attr/current" and read it, all of which goes swimmingly. When Smack frees a cred it usually does not free any memory of its own, so it is conceivable that I'm just getting lucky. Or, I may not have sufficient debug enabled. Looking at apparmor_setprocattr(), it appears that it too could end up calling commit_creds() via aa_set_current_hat(). 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().
Re: kernel BUG at kernel/cred.c:434!
On 4/17/19 4:39 PM, Paul Moore wrote: > On Wed, Apr 17, 2019 at 12:27 PM Oleg Nesterov wrote: >> On 04/17, Paul Moore wrote: >>> >>> On Wed, Apr 17, 2019 at 10:57 AM Oleg Nesterov wrote: On 04/17, Paul Moore wrote: > > I'm tempted to simply return an error in selinux_setprocattr() if > the task's credentials are not the same as its real_cred; What about other modules? I have no idea what smack_setprocattr() is, but it too does prepare_creds/commit creds. it seems that the simplest workaround should simply add the additional cred == real_cred into proc_pid_attr_write(). >>> >>> Yes, that is simple, but I worry about what other LSMs might want to >>> do. While I believe failing if the effective creds are not the same >>> as the real_creds is okay for SELinux (possibly Smack too), I worry >>> about what other LSMs may want to do. After all, >>> proc_pid_attr_write() doesn't change the the creds itself, that is >>> something the specific LSMs do. >> >> Yes, but if proc_pid_attr_write() is called with cred != real_cred then >> something is already wrong? > > True, or at least I would think so. > > Looking at the current tree there are three LSMs which implement > setprocattr hooks: SELinux, Smack, and AppArmor. I know Casey has > already mentioned that he wasn't able to trigger the problem in Smack, > but looking at smack_setprocattr() I see the similar commit_creds() > usage so I would expect the same problem in Smack; what say you Casey? > Looking at apparmor_setprocattr(), it appears that it too could end > up calling commit_creds() via aa_set_current_hat(). > > 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? heh yeah, seems I messed up our check, we actually have if (current_cred() != current_real_cred()) return -EBUSY; on the change_profile path and missed it on the change_hat one. It makes sense to lift the check up earlier
Re: kernel BUG at kernel/cred.c:434!
On Wed, Apr 17, 2019 at 12:27 PM Oleg Nesterov wrote: > On 04/17, Paul Moore wrote: > > > > On Wed, Apr 17, 2019 at 10:57 AM Oleg Nesterov wrote: > > > On 04/17, Paul Moore wrote: > > > > > > > > I'm tempted to simply return an error in selinux_setprocattr() if > > > > the task's credentials are not the same as its real_cred; > > > > > > What about other modules? I have no idea what smack_setprocattr() is, > > > but it too does prepare_creds/commit creds. > > > > > > it seems that the simplest workaround should simply add the additional > > > cred == real_cred into proc_pid_attr_write(). > > > > Yes, that is simple, but I worry about what other LSMs might want to > > do. While I believe failing if the effective creds are not the same > > as the real_creds is okay for SELinux (possibly Smack too), I worry > > about what other LSMs may want to do. After all, > > proc_pid_attr_write() doesn't change the the creds itself, that is > > something the specific LSMs do. > > Yes, but if proc_pid_attr_write() is called with cred != real_cred then > something is already wrong? True, or at least I would think so. Looking at the current tree there are three LSMs which implement setprocattr hooks: SELinux, Smack, and AppArmor. I know Casey has already mentioned that he wasn't able to trigger the problem in Smack, but looking at smack_setprocattr() I see the similar commit_creds() usage so I would expect the same problem in Smack; what say you Casey? Looking at apparmor_setprocattr(), it appears that it too could end up calling commit_creds() via aa_set_current_hat(). 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? -- paul moore www.paul-moore.com
Re: kernel BUG at kernel/cred.c:434!
On 4/17/2019 9:27 AM, Oleg Nesterov wrote: On 04/17, Paul Moore wrote: On Wed, Apr 17, 2019 at 10:57 AM Oleg Nesterov wrote: On 04/17, Paul Moore wrote: I'm tempted to simply return an error in selinux_setprocattr() if the task's credentials are not the same as its real_cred; What about other modules? I have no idea what smack_setprocattr() is, but it too does prepare_creds/commit creds. it seems that the simplest workaround should simply add the additional cred == real_cred into proc_pid_attr_write(). Yes, that is simple, but I worry about what other LSMs might want to do. While I believe failing if the effective creds are not the same as the real_creds is okay for SELinux (possibly Smack too), I worry about what other LSMs may want to do. After all, proc_pid_attr_write() doesn't change the the creds itself, that is something the specific LSMs do. Yes, but if proc_pid_attr_write() is called with cred != real_cred then something is already wrong? In fact, I think that something is already wrong if it is not called by user-space directly. Too late to ask, but why is this /proc/self/attr/ magic not implemented via syscall(s) ? Shell scripts, for one thing. It's a straightforward and appropriate use of the /proc interface. System calls would require additional change to existing programs, whereas using the /proc interface allows a good deal to be done in the containing scripts.
Re: kernel BUG at kernel/cred.c:434!
On 04/17, Paul Moore wrote: > > On Wed, Apr 17, 2019 at 10:57 AM Oleg Nesterov wrote: > > On 04/17, Paul Moore wrote: > > > > > > I'm tempted to simply return an error in selinux_setprocattr() if > > > the task's credentials are not the same as its real_cred; > > > > What about other modules? I have no idea what smack_setprocattr() is, > > but it too does prepare_creds/commit creds. > > > > it seems that the simplest workaround should simply add the additional > > cred == real_cred into proc_pid_attr_write(). > > Yes, that is simple, but I worry about what other LSMs might want to > do. While I believe failing if the effective creds are not the same > as the real_creds is okay for SELinux (possibly Smack too), I worry > about what other LSMs may want to do. After all, > proc_pid_attr_write() doesn't change the the creds itself, that is > something the specific LSMs do. Yes, but if proc_pid_attr_write() is called with cred != real_cred then something is already wrong? In fact, I think that something is already wrong if it is not called by user-space directly. Too late to ask, but why is this /proc/self/attr/ magic not implemented via syscall(s) ? But, Paul, this is up to you. I don't understand this all even remotely. Oleg.
Re: kernel BUG at kernel/cred.c:434!
On Wed, Apr 17, 2019 at 10:57 AM Oleg Nesterov wrote: > On 04/17, Paul Moore wrote: > > > > I'm tempted to simply return an error in selinux_setprocattr() if > > the task's credentials are not the same as its real_cred; > > What about other modules? I have no idea what smack_setprocattr() is, > but it too does prepare_creds/commit creds. > > it seems that the simplest workaround should simply add the additional > cred == real_cred into proc_pid_attr_write(). Yes, that is simple, but I worry about what other LSMs might want to do. While I believe failing if the effective creds are not the same as the real_creds is okay for SELinux (possibly Smack too), I worry about what other LSMs may want to do. After all, proc_pid_attr_write() doesn't change the the creds itself, that is something the specific LSMs do. -- paul moore www.paul-moore.com
Re: kernel BUG at kernel/cred.c:434!
On 4/17/2019 7:57 AM, Oleg Nesterov wrote: On 04/17, Paul Moore wrote: I'm tempted to simply return an error in selinux_setprocattr() if the task's credentials are not the same as its real_cred; What about other modules? I have no idea what smack_setprocattr() is, but it too does prepare_creds/commit creds. For what it's worth, my test for Smack does not reproduce the problem. it seems that the simplest workaround should simply add the additional cred == real_cred into proc_pid_attr_write(). Oleg.
Re: kernel BUG at kernel/cred.c:434!
On 04/17, Paul Moore wrote: > > I'm tempted to simply return an error in selinux_setprocattr() if > the task's credentials are not the same as its real_cred; What about other modules? I have no idea what smack_setprocattr() is, but it too does prepare_creds/commit creds. it seems that the simplest workaround should simply add the additional cred == real_cred into proc_pid_attr_write(). Oleg.
Re: kernel BUG at kernel/cred.c:434!
On Tue, Apr 16, 2019 at 10:46 AM chengjian (D) wrote:
> On 2019/4/16 11:40, Kees Cook wrote:
> > On Mon, Apr 15, 2019 at 11:20 AM Paul Moore wrote:
> >> On Mon, Apr 15, 2019 at 11:05 AM Oleg Nesterov wrote:
> >>> On 04/15, Paul Moore wrote:
> On Mon, Apr 15, 2019 at 9:43 AM Oleg Nesterov wrote:
> > Well, acct("/proc/self/attr/current") doesn't look like a good idea,
> > but I do
> > not know where should we put the additional check... And probably
> > "echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit
> > the
> > same problem, do_coredump() does override_creds() too.
> >
> > May be just add
> >
> > if (current->cred != current->real_cred)
> > return -EACCES;
> >
> > into proc_pid_attr_write(), I dunno.
> Is the problem that do_acct_process() is calling override_creds() and
> the returned/old credentials are being freed before do_acct_process()
> can reinstall the creds via revert_creds()? Presumably because the
> process accounting is causing the credentials to be replaced?
> >>> Afaics, the problem is that do_acct_process() does override_creds() and
> >>> then __kernel_write(). Which calls proc_pid_attr_write(), which in turn
> >>> calls
> >>> selinux_setprocattr(), which does another prepare_creds() +
> >>> commit_creds();
> >>> and commit_creds() hits
> >>>
> >>> BUG_ON(task->cred != old);
> >> Gotcha. In the process of looking at the backtrace I forgot about the
> >> BUG_ON() at the top of the oops message.
> >>
> >> I wonder what terrible things would happen if we changed the BUG_ON()
> >> in commit_creds to simple returning an error an error code to the
> >> caller. There is a warning/requirement in commit_creds() function
> >> header comment that it should always return 0.
> > Would callers be expected to call abort_creds() on failure? There are
> > a number of places where it'd need fixing up. And would likely be best
> > with a __must_check marking.
> >
> > It seems like avoiding the pathological case might be simpler?
>
> Yeah, Avoiding this pathological case is a better solution.
No arguments that this is particularly messed up scenario, I'm just
trying to arrive at a solution that isn't too ugly.
> From: Yang Yingliang
> Date: Sat, 13 Apr 2019 21:56:01 +0800
> Subject: [PATCH] fix cred bug_on
>
> Signed-off-by: Yang Yingliang
> ---
> kernel/acct.c| 3 ++-
> kernel/cred.c| 6 ++
> security/selinux/hooks.c | 2 ++
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/acct.c b/kernel/acct.c
> index addf7732fb56..f2065f899eee 100644
> --- a/kernel/acct.c
> +++ b/kernel/acct.c
> @@ -522,7 +522,8 @@ static void do_acct_process(struct bsd_acct_struct
> *acct)
> }
> out:
> current->signal->rlim[RLIMIT_FSIZE].rlim_cur = flim;
> -revert_creds(orig_cred);
> +if (orig_cred == current->real_cred)// [2]
> +revert_creds(orig_cred);
> }
>
> /**
> diff --git a/kernel/cred.c b/kernel/cred.c
> index ecf03657e71c..c4d5ba92fb9b 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -522,6 +522,9 @@ const struct cred *override_creds(const struct cred
> *new)
> {
> const struct cred *old = current->cred;
>
> +if (old == new)// [3]
> +return old;
> +
> kdebug("override_creds(%p{%d,%d})", new,
> atomic_read(>usage),
> read_cred_subscribers(new));
> @@ -551,6 +554,9 @@ void revert_creds(const struct cred *old)
> {
> const struct cred *override = current->cred;
>
> +if (override == old)// [3]
> +return;
> +
> kdebug("revert_creds(%p{%d,%d})", old,
> atomic_read(>usage),
> read_cred_subscribers(old));
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index b5017beb4ef7..bc8108e4e90f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6590,6 +6590,8 @@ static int selinux_setprocattr(const char *name,
> void *value, size_t size)
> goto abort_change;
> }
>
> +if (current->cred != current->real_cred)// [1]
> +revert_creds(current->real_cred);
> commit_creds(new);
> return size;
Doing the revert only to then commit the creds seems really ugly to
me. I'm tempted to simply return an error in selinux_setprocattr() if
the task's credentials are not the same as its real_cred; if we do
that I believe we should resolve this problem. The accounting write
to the SELinux file in /proc would fail of course, but I think we can
all consider that as a positive side-effect.
While I don't think this should have a negative impact on anything
else, I haven't convinced myself of that just yet.
--
paul moore
www.paul-moore.com
Re: kernel BUG at kernel/cred.c:434!
On 2019/4/16 11:40, Kees Cook wrote:
On Mon, Apr 15, 2019 at 11:20 AM Paul Moore wrote:
On Mon, Apr 15, 2019 at 11:05 AM Oleg Nesterov wrote:
On 04/15, Paul Moore wrote:
On Mon, Apr 15, 2019 at 9:43 AM Oleg Nesterov wrote:
Well, acct("/proc/self/attr/current") doesn't look like a good idea, but I do
not know where should we put the additional check... And probably
"echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit the
same problem, do_coredump() does override_creds() too.
May be just add
if (current->cred != current->real_cred)
return -EACCES;
into proc_pid_attr_write(), I dunno.
Is the problem that do_acct_process() is calling override_creds() and
the returned/old credentials are being freed before do_acct_process()
can reinstall the creds via revert_creds()? Presumably because the
process accounting is causing the credentials to be replaced?
Afaics, the problem is that do_acct_process() does override_creds() and
then __kernel_write(). Which calls proc_pid_attr_write(), which in turn calls
selinux_setprocattr(), which does another prepare_creds() + commit_creds();
and commit_creds() hits
BUG_ON(task->cred != old);
Gotcha. In the process of looking at the backtrace I forgot about the
BUG_ON() at the top of the oops message.
I wonder what terrible things would happen if we changed the BUG_ON()
in commit_creds to simple returning an error an error code to the
caller. There is a warning/requirement in commit_creds() function
header comment that it should always return 0.
Would callers be expected to call abort_creds() on failure? There are
a number of places where it'd need fixing up. And would likely be best
with a __must_check marking.
It seems like avoiding the pathological case might be simpler?
Yeah, Avoiding this pathological case is a better solution.
It seems like that we can't commit_creds() during
override_creds() and revert_creds().
So how about just put commit_creds outside !
just like:
override_creds() // cred -=> new
// may BUG_ON if commit_creds done.
revert_creds() // cred -=> old
<---|
commit_creds // cred = real_cred = new |
[revert_creds]--|
[1]--Before we call commit_creds in selinux_setprocattr(),
if we find that cred != real_cred, it may have been overridden
before, we should revert it.
[2]--The same to revert_creds, when we found someone have committed,
orig_cred != current->real_cred may hits, this means that
we have reverted before(see [1]).
[3]--Sometimes new and old are the same, then we need to consider this
situation specially.
The code just like:
From: Yang Yingliang
Date: Sat, 13 Apr 2019 21:56:01 +0800
Subject: [PATCH] fix cred bug_on
Signed-off-by: Yang Yingliang
---
kernel/acct.c | 3 ++-
kernel/cred.c | 6 ++
security/selinux/hooks.c | 2 ++
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/kernel/acct.c b/kernel/acct.c
index addf7732fb56..f2065f899eee 100644
--- a/kernel/acct.c
+++ b/kernel/acct.c
@@ -522,7 +522,8 @@ static void do_acct_process(struct bsd_acct_struct
*acct)
}
out:
current->signal->rlim[RLIMIT_FSIZE].rlim_cur = flim;
- revert_creds(orig_cred);
+ if (orig_cred == current->real_cred) // [2]
+ revert_creds(orig_cred);
}
/**
diff --git a/kernel/cred.c b/kernel/cred.c
index ecf03657e71c..c4d5ba92fb9b 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -522,6 +522,9 @@ const struct cred *override_creds(const struct cred
*new)
{
const struct cred *old = current->cred;
+ if (old == new) // [3]
+ return old;
+
kdebug("override_creds(%p{%d,%d})", new,
atomic_read(>usage),
read_cred_subscribers(new));
@@ -551,6 +554,9 @@ void revert_creds(const struct cred *old)
{
const struct cred *override = current->cred;
+ if (override == old) // [3]
+ return;
+
kdebug("revert_creds(%p{%d,%d})", old,
atomic_read(>usage),
read_cred_subscribers(old));
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b5017beb4ef7..bc8108e4e90f 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6590,6 +6590,8 @@ static int selinux_setprocattr(const char *name,
void *value, size_t size)
goto abort_change;
}
+ if (current->cred != current->real_cred) // [1]
+ revert_creds(current->real_cred);
commit_creds(new);
return size;
--
2.17.1
We have tested this patch for 3 days and it works well.
Are there any cases that are not covered here ?
Thanks.
Cheng Jian
Re: kernel BUG at kernel/cred.c:434!
On Mon, Apr 15, 2019 at 11:20 AM Paul Moore wrote:
>
> On Mon, Apr 15, 2019 at 11:05 AM Oleg Nesterov wrote:
> > On 04/15, Paul Moore wrote:
> > >
> > > On Mon, Apr 15, 2019 at 9:43 AM Oleg Nesterov wrote:
> > > > Well, acct("/proc/self/attr/current") doesn't look like a good idea,
> > > > but I do
> > > > not know where should we put the additional check... And probably
> > > > "echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit
> > > > the
> > > > same problem, do_coredump() does override_creds() too.
> > > >
> > > > May be just add
> > > >
> > > > if (current->cred != current->real_cred)
> > > > return -EACCES;
> > > >
> > > > into proc_pid_attr_write(), I dunno.
> > >
> > > Is the problem that do_acct_process() is calling override_creds() and
> > > the returned/old credentials are being freed before do_acct_process()
> > > can reinstall the creds via revert_creds()? Presumably because the
> > > process accounting is causing the credentials to be replaced?
> >
> > Afaics, the problem is that do_acct_process() does override_creds() and
> > then __kernel_write(). Which calls proc_pid_attr_write(), which in turn
> > calls
> > selinux_setprocattr(), which does another prepare_creds() + commit_creds();
> > and commit_creds() hits
> >
> > BUG_ON(task->cred != old);
>
> Gotcha. In the process of looking at the backtrace I forgot about the
> BUG_ON() at the top of the oops message.
>
> I wonder what terrible things would happen if we changed the BUG_ON()
> in commit_creds to simple returning an error an error code to the
> caller. There is a warning/requirement in commit_creds() function
> header comment that it should always return 0.
Would callers be expected to call abort_creds() on failure? There are
a number of places where it'd need fixing up. And would likely be best
with a __must_check marking.
It seems like avoiding the pathological case might be simpler?
--
Kees Cook
Re: kernel BUG at kernel/cred.c:434!
On Mon, Apr 15, 2019 at 11:05 AM Oleg Nesterov wrote:
> On 04/15, Paul Moore wrote:
> >
> > On Mon, Apr 15, 2019 at 9:43 AM Oleg Nesterov wrote:
> > > Well, acct("/proc/self/attr/current") doesn't look like a good idea, but
> > > I do
> > > not know where should we put the additional check... And probably
> > > "echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit the
> > > same problem, do_coredump() does override_creds() too.
> > >
> > > May be just add
> > >
> > > if (current->cred != current->real_cred)
> > > return -EACCES;
> > >
> > > into proc_pid_attr_write(), I dunno.
> >
> > Is the problem that do_acct_process() is calling override_creds() and
> > the returned/old credentials are being freed before do_acct_process()
> > can reinstall the creds via revert_creds()? Presumably because the
> > process accounting is causing the credentials to be replaced?
>
> Afaics, the problem is that do_acct_process() does override_creds() and
> then __kernel_write(). Which calls proc_pid_attr_write(), which in turn calls
> selinux_setprocattr(), which does another prepare_creds() + commit_creds();
> and commit_creds() hits
>
> BUG_ON(task->cred != old);
Gotcha. In the process of looking at the backtrace I forgot about the
BUG_ON() at the top of the oops message.
I wonder what terrible things would happen if we changed the BUG_ON()
in commit_creds to simple returning an error an error code to the
caller. There is a warning/requirement in commit_creds() function
header comment that it should always return 0.
--
paul moore
www.paul-moore.com
Re: kernel BUG at kernel/cred.c:434!
On 04/15, Paul Moore wrote:
>
> On Mon, Apr 15, 2019 at 9:43 AM Oleg Nesterov wrote:
> > Well, acct("/proc/self/attr/current") doesn't look like a good idea, but I
> > do
> > not know where should we put the additional check... And probably
> > "echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit the
> > same problem, do_coredump() does override_creds() too.
> >
> > May be just add
> >
> > if (current->cred != current->real_cred)
> > return -EACCES;
> >
> > into proc_pid_attr_write(), I dunno.
>
> Is the problem that do_acct_process() is calling override_creds() and
> the returned/old credentials are being freed before do_acct_process()
> can reinstall the creds via revert_creds()? Presumably because the
> process accounting is causing the credentials to be replaced?
Afaics, the problem is that do_acct_process() does override_creds() and
then __kernel_write(). Which calls proc_pid_attr_write(), which in turn calls
selinux_setprocattr(), which does another prepare_creds() + commit_creds();
and commit_creds() hits
BUG_ON(task->cred != old);
Oleg.
Re: kernel BUG at kernel/cred.c:434!
On Mon, Apr 15, 2019 at 9:43 AM Oleg Nesterov wrote:
> Well, acct("/proc/self/attr/current") doesn't look like a good idea, but I do
> not know where should we put the additional check... And probably
> "echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit the
> same problem, do_coredump() does override_creds() too.
>
> May be just add
>
> if (current->cred != current->real_cred)
> return -EACCES;
>
> into proc_pid_attr_write(), I dunno.
Is the problem that do_acct_process() is calling override_creds() and
the returned/old credentials are being freed before do_acct_process()
can reinstall the creds via revert_creds()? Presumably because the
process accounting is causing the credentials to be replaced?
> On 04/12, Casey Schaufler wrote:
> >
> > On 4/11/2019 11:21 PM, chengjian (D) wrote:
> >
> > Added LSM and SELinux lists.
> >
> >
> > >Hi.
> > >
> > >
> > >syzkaller reported the following BUG:
> > >
> > >[ 73.146973] kernel BUG at kernel/cred.c:434!
> > >[ 73.150231] invalid opcode: [#1] SMP KASAN PTI
> > >[ 73.151928] CPU: 2 PID: 4058 Comm: syz-executor.6 Not tainted
> > >5.1.0-rc4-00062-g2d06b235815e-dirty #2
> > >[ 73.155174] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > >rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> > >[ 73.159798] RIP: 0010:commit_creds+0xadb/0xe50
> > >[ 73.161426] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f
> > >8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05 a2 25 00
> > ><0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f 0b 48
> > >[ 73.167852] RSP: :88836e65f5d0 EFLAGS: 00010293
> > >[ 73.169636] RAX: 8883767b RBX: 88837f111300 RCX:
> > >8124b5db
> > >[ 73.171962] RDX: RSI: 83c9b140 RDI:
> > >88837f111300
> > >[ 73.174310] RBP: 888376610400 R08: R09:
> > >0004
> > >[ 73.176646] R10: 0001 R11: ed107c655acf R12:
> > >8883767b
> > >[ 73.178527] Process accounting resumed
> > >[ 73.179021] R13: 88837f111900 R14: 88837f111300 R15:
> > >8883767b0ac0
> > >[ 73.179029] FS: 7f2d207f9700() GS:8883e328()
> > >knlGS:
> > >[ 73.179034] CS: 0010 DS: ES: CR0: 80050033
> > >[ 73.179039] CR2: 7f1500bd36c0 CR3: 0003df304003 CR4:
> > >000206e0
> > >[ 73.179047] Call Trace:
> > >[ 73.190461] selinux_setprocattr+0x2ea/0x8f0
> > >[ 73.191925] ? ptrace_parent_sid+0x530/0x530
> > >[ 73.193436] ? proc_pid_attr_write+0x185/0x5a0
> > >[ 73.194967] security_setprocattr+0xa1/0x100
> > >[ 73.196408] proc_pid_attr_write+0x307/0x5a0
> > >[ 73.197869] ? mem_read+0x40/0x40
> > >[ 73.199013] __vfs_write+0x81/0x100
> > >[ 73.200222] __kernel_write+0xf8/0x330
> > >[ 73.201562] do_acct_process+0xca5/0x1340
> > >[ 73.202969] ? __ia32_sys_acct+0x1e0/0x1e0
> > >[ 73.204498] ? find_held_lock+0x2f/0x1e0
> > >[ 73.205857] ? rcu_irq_exit+0xec/0x2c0
> > >[ 73.207160] ? lock_downgrade+0x630/0x630
> > >[ 73.208541] acct_pin_kill+0x63/0x150
> > >[ 73.209816] pin_kill+0x16d/0x7c0
> > >[ 73.210934] ? lockdep_hardirqs_on+0x5e0/0x5e0
> > >[ 73.212452] ? xas_start+0x155/0x510
> > >[ 73.213705] ? pin_insert+0x50/0x50
> > >[ 73.214903] ? finish_wait+0x270/0x270
> > >[ 73.216213] ? cpumask_next+0x57/0x90
> > >[ 73.217442] ? mnt_pin_kill+0x68/0x1d0
> > >[ 73.218851] mnt_pin_kill+0x68/0x1d0
> > >[ 73.220398] cleanup_mnt+0x11b/0x150
> > >[ 73.221970] task_work_run+0x136/0x1b0
> > >[ 73.223427] do_exit+0x830/0x2ca0
> > >[ 73.224586] ? trace_hardirqs_off+0x3b/0x180
> > >[ 73.226088] ? mm_update_next_owner+0x6a0/0x6a0
> > >[ 73.227622] ? find_held_lock+0x2f/0x1e0
> > >[ 73.228954] ? get_signal+0x2cf/0x1c00
> > >[ 73.230236] ? lock_downgrade+0x630/0x630
> > >[ 73.231628] ? rwlock_bug.part.0+0x90/0x90
> > >[ 73.233020] do_group_exit+0x106/0x2f0
> > >[ 73.234330] get_signal+0x325/0x1c00
> > >[ 73.235571] do_signal+0x97/0x1670
> > >[ 73.236739] ? do_send_specific+0x12d/0x220
> > >[ 73.238213] ? lock_downgrade+0x630/0x630
> > >[ 73.239566] ? setup_sigcontext
Re: kernel BUG at kernel/cred.c:434!
Well, acct("/proc/self/attr/current") doesn't look like a good idea, but I do
not know where should we put the additional check... And probably
"echo /proc/self/attr/current > /proc/sys/kernel/core_pattern" can hit the
same problem, do_coredump() does override_creds() too.
May be just add
if (current->cred != current->real_cred)
return -EACCES;
into proc_pid_attr_write(), I dunno.
On 04/12, Casey Schaufler wrote:
>
> On 4/11/2019 11:21 PM, chengjian (D) wrote:
>
> Added LSM and SELinux lists.
>
>
> >Hi.
> >
> >
> >syzkaller reported the following BUG:
> >
> >[ 73.146973] kernel BUG at kernel/cred.c:434!
> >[ 73.150231] invalid opcode: [#1] SMP KASAN PTI
> >[ 73.151928] CPU: 2 PID: 4058 Comm: syz-executor.6 Not tainted
> >5.1.0-rc4-00062-g2d06b235815e-dirty #2
> >[ 73.155174] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> >rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> >[ 73.159798] RIP: 0010:commit_creds+0xadb/0xe50
> >[ 73.161426] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f
> >8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05 a2 25 00
> ><0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f 0b 48
> >[ 73.167852] RSP: :88836e65f5d0 EFLAGS: 00010293
> >[ 73.169636] RAX: 8883767b RBX: 88837f111300 RCX:
> >8124b5db
> >[ 73.171962] RDX: RSI: 83c9b140 RDI:
> >88837f111300
> >[ 73.174310] RBP: 888376610400 R08: R09:
> >0004
> >[ 73.176646] R10: 0001 R11: ed107c655acf R12:
> >8883767b
> >[ 73.178527] Process accounting resumed
> >[ 73.179021] R13: 88837f111900 R14: 88837f111300 R15:
> >8883767b0ac0
> >[ 73.179029] FS: 7f2d207f9700() GS:8883e328()
> >knlGS:
> >[ 73.179034] CS: 0010 DS: ES: CR0: 80050033
> >[ 73.179039] CR2: 7f1500bd36c0 CR3: 0003df304003 CR4:
> >000206e0
> >[ 73.179047] Call Trace:
> >[ 73.190461] selinux_setprocattr+0x2ea/0x8f0
> >[ 73.191925] ? ptrace_parent_sid+0x530/0x530
> >[ 73.193436] ? proc_pid_attr_write+0x185/0x5a0
> >[ 73.194967] security_setprocattr+0xa1/0x100
> >[ 73.196408] proc_pid_attr_write+0x307/0x5a0
> >[ 73.197869] ? mem_read+0x40/0x40
> >[ 73.199013] __vfs_write+0x81/0x100
> >[ 73.200222] __kernel_write+0xf8/0x330
> >[ 73.201562] do_acct_process+0xca5/0x1340
> >[ 73.202969] ? __ia32_sys_acct+0x1e0/0x1e0
> >[ 73.204498] ? find_held_lock+0x2f/0x1e0
> >[ 73.205857] ? rcu_irq_exit+0xec/0x2c0
> >[ 73.207160] ? lock_downgrade+0x630/0x630
> >[ 73.208541] acct_pin_kill+0x63/0x150
> >[ 73.209816] pin_kill+0x16d/0x7c0
> >[ 73.210934] ? lockdep_hardirqs_on+0x5e0/0x5e0
> >[ 73.212452] ? xas_start+0x155/0x510
> >[ 73.213705] ? pin_insert+0x50/0x50
> >[ 73.214903] ? finish_wait+0x270/0x270
> >[ 73.216213] ? cpumask_next+0x57/0x90
> >[ 73.217442] ? mnt_pin_kill+0x68/0x1d0
> >[ 73.218851] mnt_pin_kill+0x68/0x1d0
> >[ 73.220398] cleanup_mnt+0x11b/0x150
> >[ 73.221970] task_work_run+0x136/0x1b0
> >[ 73.223427] do_exit+0x830/0x2ca0
> >[ 73.224586] ? trace_hardirqs_off+0x3b/0x180
> >[ 73.226088] ? mm_update_next_owner+0x6a0/0x6a0
> >[ 73.227622] ? find_held_lock+0x2f/0x1e0
> >[ 73.228954] ? get_signal+0x2cf/0x1c00
> >[ 73.230236] ? lock_downgrade+0x630/0x630
> >[ 73.231628] ? rwlock_bug.part.0+0x90/0x90
> >[ 73.233020] do_group_exit+0x106/0x2f0
> >[ 73.234330] get_signal+0x325/0x1c00
> >[ 73.235571] do_signal+0x97/0x1670
> >[ 73.236739] ? do_send_specific+0x12d/0x220
> >[ 73.238213] ? lock_downgrade+0x630/0x630
> >[ 73.239566] ? setup_sigcontext+0x820/0x820
> >[ 73.240982] ? check_kill_permission+0x4a/0x510
> >[ 73.242509] ? do_send_specific+0x156/0x220
> >[ 73.243905] ? do_tkill+0x1c4/0x260
> >[ 73.245081] ? do_send_specific+0x220/0x220
> >[ 73.246514] ? trace_hardirqs_on_thunk+0x1a/0x1c
> >[ 73.248061] ? exit_to_usermode_loop+0x97/0x1d0
> >[ 73.249619] exit_to_usermode_loop+0x108/0x1d0
> >[ 73.251129] do_syscall_64+0x461/0x580
> >[ 73.252454] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> >[ 73.254219] RIP: 0033:0x462eb9
> >[ 73.255327] Code: Bad RIP value.
> >[ 73.256539] RSP: 002b:7f2d207f8c58 EFLAGS: 0246 ORIG_RAX:
> >00c8
> >[ 73.259454] RAX:
Re: kernel BUG at kernel/cred.c:434!
On 4/11/2019 11:21 PM, chengjian (D) wrote: Added LSM and SELinux lists. Hi. syzkaller reported the following BUG: [ 73.146973] kernel BUG at kernel/cred.c:434! [ 73.150231] invalid opcode: [#1] SMP KASAN PTI [ 73.151928] CPU: 2 PID: 4058 Comm: syz-executor.6 Not tainted 5.1.0-rc4-00062-g2d06b235815e-dirty #2 [ 73.155174] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 [ 73.159798] RIP: 0010:commit_creds+0xadb/0xe50 [ 73.161426] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f 8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05 a2 25 00 <0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f 0b 48 [ 73.167852] RSP: :88836e65f5d0 EFLAGS: 00010293 [ 73.169636] RAX: 8883767b RBX: 88837f111300 RCX: 8124b5db [ 73.171962] RDX: RSI: 83c9b140 RDI: 88837f111300 [ 73.174310] RBP: 888376610400 R08: R09: 0004 [ 73.176646] R10: 0001 R11: ed107c655acf R12: 8883767b [ 73.178527] Process accounting resumed [ 73.179021] R13: 88837f111900 R14: 88837f111300 R15: 8883767b0ac0 [ 73.179029] FS: 7f2d207f9700() GS:8883e328() knlGS: [ 73.179034] CS: 0010 DS: ES: CR0: 80050033 [ 73.179039] CR2: 7f1500bd36c0 CR3: 0003df304003 CR4: 000206e0 [ 73.179047] Call Trace: [ 73.190461] selinux_setprocattr+0x2ea/0x8f0 [ 73.191925] ? ptrace_parent_sid+0x530/0x530 [ 73.193436] ? proc_pid_attr_write+0x185/0x5a0 [ 73.194967] security_setprocattr+0xa1/0x100 [ 73.196408] proc_pid_attr_write+0x307/0x5a0 [ 73.197869] ? mem_read+0x40/0x40 [ 73.199013] __vfs_write+0x81/0x100 [ 73.200222] __kernel_write+0xf8/0x330 [ 73.201562] do_acct_process+0xca5/0x1340 [ 73.202969] ? __ia32_sys_acct+0x1e0/0x1e0 [ 73.204498] ? find_held_lock+0x2f/0x1e0 [ 73.205857] ? rcu_irq_exit+0xec/0x2c0 [ 73.207160] ? lock_downgrade+0x630/0x630 [ 73.208541] acct_pin_kill+0x63/0x150 [ 73.209816] pin_kill+0x16d/0x7c0 [ 73.210934] ? lockdep_hardirqs_on+0x5e0/0x5e0 [ 73.212452] ? xas_start+0x155/0x510 [ 73.213705] ? pin_insert+0x50/0x50 [ 73.214903] ? finish_wait+0x270/0x270 [ 73.216213] ? cpumask_next+0x57/0x90 [ 73.217442] ? mnt_pin_kill+0x68/0x1d0 [ 73.218851] mnt_pin_kill+0x68/0x1d0 [ 73.220398] cleanup_mnt+0x11b/0x150 [ 73.221970] task_work_run+0x136/0x1b0 [ 73.223427] do_exit+0x830/0x2ca0 [ 73.224586] ? trace_hardirqs_off+0x3b/0x180 [ 73.226088] ? mm_update_next_owner+0x6a0/0x6a0 [ 73.227622] ? find_held_lock+0x2f/0x1e0 [ 73.228954] ? get_signal+0x2cf/0x1c00 [ 73.230236] ? lock_downgrade+0x630/0x630 [ 73.231628] ? rwlock_bug.part.0+0x90/0x90 [ 73.233020] do_group_exit+0x106/0x2f0 [ 73.234330] get_signal+0x325/0x1c00 [ 73.235571] do_signal+0x97/0x1670 [ 73.236739] ? do_send_specific+0x12d/0x220 [ 73.238213] ? lock_downgrade+0x630/0x630 [ 73.239566] ? setup_sigcontext+0x820/0x820 [ 73.240982] ? check_kill_permission+0x4a/0x510 [ 73.242509] ? do_send_specific+0x156/0x220 [ 73.243905] ? do_tkill+0x1c4/0x260 [ 73.245081] ? do_send_specific+0x220/0x220 [ 73.246514] ? trace_hardirqs_on_thunk+0x1a/0x1c [ 73.248061] ? exit_to_usermode_loop+0x97/0x1d0 [ 73.249619] exit_to_usermode_loop+0x108/0x1d0 [ 73.251129] do_syscall_64+0x461/0x580 [ 73.252454] entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 73.254219] RIP: 0033:0x462eb9 [ 73.255327] Code: Bad RIP value. [ 73.256539] RSP: 002b:7f2d207f8c58 EFLAGS: 0246 ORIG_RAX: 00c8 [ 73.259454] RAX: RBX: 0073bf00 RCX: 00462eb9 [ 73.262309] RDX: RSI: 001e RDI: 0005 [ 73.265064] RBP: 0002 R08: R09: [ 73.267774] R10: R11: 0246 R12: 7f2d207f96bc [ 73.270546] R13: 004c5509 R14: 007042f0 R15: [ 73.273542] Modules linked in: [ 73.274670] Dumping ftrace buffer: [ 73.275852] (ftrace buffer empty) [ 73.277187] ---[ end trace dde36a95f458175d ]--- [ 73.278834] RIP: 0010:commit_creds+0xadb/0xe50 [ 73.280549] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f 8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05 a2 25 00 <0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f 0b 48 [ 73.287090] RSP: :88836e65f5d0 EFLAGS: 00010293 [ 73.288917] RAX: 8883767b RBX: 88837f111300 RCX: 8124b5db [ 73.291390] RDX: RSI: 83c9b140 RDI: 88837f111300 [ 73.293864] RBP: 888376610400 R08: R09: 0004 [ 73.296370] R10: 0001 R11: ed107c655acf R12: 8883767b [ 73.299275] R13: 8883
kernel BUG at kernel/cred.c:434!
Hi. syzkaller reported the following BUG: [ 73.146973] kernel BUG at kernel/cred.c:434! [ 73.150231] invalid opcode: [#1] SMP KASAN PTI [ 73.151928] CPU: 2 PID: 4058 Comm: syz-executor.6 Not tainted 5.1.0-rc4-00062-g2d06b235815e-dirty #2 [ 73.155174] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 [ 73.159798] RIP: 0010:commit_creds+0xadb/0xe50 [ 73.161426] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f 8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05 a2 25 00 <0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f 0b 48 [ 73.167852] RSP: :88836e65f5d0 EFLAGS: 00010293 [ 73.169636] RAX: 8883767b RBX: 88837f111300 RCX: 8124b5db [ 73.171962] RDX: RSI: 83c9b140 RDI: 88837f111300 [ 73.174310] RBP: 888376610400 R08: R09: 0004 [ 73.176646] R10: 0001 R11: ed107c655acf R12: 8883767b [ 73.178527] Process accounting resumed [ 73.179021] R13: 88837f111900 R14: 88837f111300 R15: 8883767b0ac0 [ 73.179029] FS: 7f2d207f9700() GS:8883e328() knlGS: [ 73.179034] CS: 0010 DS: ES: CR0: 80050033 [ 73.179039] CR2: 7f1500bd36c0 CR3: 0003df304003 CR4: 000206e0 [ 73.179047] Call Trace: [ 73.190461] selinux_setprocattr+0x2ea/0x8f0 [ 73.191925] ? ptrace_parent_sid+0x530/0x530 [ 73.193436] ? proc_pid_attr_write+0x185/0x5a0 [ 73.194967] security_setprocattr+0xa1/0x100 [ 73.196408] proc_pid_attr_write+0x307/0x5a0 [ 73.197869] ? mem_read+0x40/0x40 [ 73.199013] __vfs_write+0x81/0x100 [ 73.200222] __kernel_write+0xf8/0x330 [ 73.201562] do_acct_process+0xca5/0x1340 [ 73.202969] ? __ia32_sys_acct+0x1e0/0x1e0 [ 73.204498] ? find_held_lock+0x2f/0x1e0 [ 73.205857] ? rcu_irq_exit+0xec/0x2c0 [ 73.207160] ? lock_downgrade+0x630/0x630 [ 73.208541] acct_pin_kill+0x63/0x150 [ 73.209816] pin_kill+0x16d/0x7c0 [ 73.210934] ? lockdep_hardirqs_on+0x5e0/0x5e0 [ 73.212452] ? xas_start+0x155/0x510 [ 73.213705] ? pin_insert+0x50/0x50 [ 73.214903] ? finish_wait+0x270/0x270 [ 73.216213] ? cpumask_next+0x57/0x90 [ 73.217442] ? mnt_pin_kill+0x68/0x1d0 [ 73.218851] mnt_pin_kill+0x68/0x1d0 [ 73.220398] cleanup_mnt+0x11b/0x150 [ 73.221970] task_work_run+0x136/0x1b0 [ 73.223427] do_exit+0x830/0x2ca0 [ 73.224586] ? trace_hardirqs_off+0x3b/0x180 [ 73.226088] ? mm_update_next_owner+0x6a0/0x6a0 [ 73.227622] ? find_held_lock+0x2f/0x1e0 [ 73.228954] ? get_signal+0x2cf/0x1c00 [ 73.230236] ? lock_downgrade+0x630/0x630 [ 73.231628] ? rwlock_bug.part.0+0x90/0x90 [ 73.233020] do_group_exit+0x106/0x2f0 [ 73.234330] get_signal+0x325/0x1c00 [ 73.235571] do_signal+0x97/0x1670 [ 73.236739] ? do_send_specific+0x12d/0x220 [ 73.238213] ? lock_downgrade+0x630/0x630 [ 73.239566] ? setup_sigcontext+0x820/0x820 [ 73.240982] ? check_kill_permission+0x4a/0x510 [ 73.242509] ? do_send_specific+0x156/0x220 [ 73.243905] ? do_tkill+0x1c4/0x260 [ 73.245081] ? do_send_specific+0x220/0x220 [ 73.246514] ? trace_hardirqs_on_thunk+0x1a/0x1c [ 73.248061] ? exit_to_usermode_loop+0x97/0x1d0 [ 73.249619] exit_to_usermode_loop+0x108/0x1d0 [ 73.251129] do_syscall_64+0x461/0x580 [ 73.252454] entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 73.254219] RIP: 0033:0x462eb9 [ 73.255327] Code: Bad RIP value. [ 73.256539] RSP: 002b:7f2d207f8c58 EFLAGS: 0246 ORIG_RAX: 00c8 [ 73.259454] RAX: RBX: 0073bf00 RCX: 00462eb9 [ 73.262309] RDX: RSI: 001e RDI: 0005 [ 73.265064] RBP: 0002 R08: R09: [ 73.267774] R10: R11: 0246 R12: 7f2d207f96bc [ 73.270546] R13: 004c5509 R14: 007042f0 R15: [ 73.273542] Modules linked in: [ 73.274670] Dumping ftrace buffer: [ 73.275852] (ftrace buffer empty) [ 73.277187] ---[ end trace dde36a95f458175d ]--- [ 73.278834] RIP: 0010:commit_creds+0xadb/0xe50 [ 73.280549] Code: 8b 5b 20 48 c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f 8e 06 03 00 00 39 5d 20 0f 85 ff fa ff ff e9 0c fb ff ff e8 05 a2 25 00 <0f> 0b 48 c7 c7 80 56 c0 83 e8 95 22 b1 00 e8 f2 a1 25 00 0f 0b 48 [ 73.287090] RSP: :88836e65f5d0 EFLAGS: 00010293 [ 73.288917] RAX: 8883767b RBX: 88837f111300 RCX: 8124b5db [ 73.291390] RDX: RSI: 83c9b140 RDI: 88837f111300 [ 73.293864] RBP: 888376610400 R08: R09: 0004 [ 73.296370] R10: 0001 R11: ed107c655acf R12: 8883767b [ 73.299275] R13: 88837f111900 R14: 88837f111300 R15: 8883767b0ac0 [ 73.301351] Process accoun
