* [PATCH v1] ucounts: Increase ucounts reference counter before the security hook [not found] <000000000000b1f4d305c9ef72ad@google.com> @ 2021-08-23 16:16 ` Alexey Gladkov 2021-08-23 21:31 ` Eric W. Biederman 0 siblings, 1 reply; 2+ messages in thread From: Alexey Gladkov @ 2021-08-23 16:16 UTC (permalink / raw) To: LKML, Eric W . Biederman; +Cc: Linux Containers, syzbot+01985d7909f9468f013c We need to increment the ucounts reference counter befor security_prepare_creds() because this function may fail and abort_creds() will try to decrement this reference. [ 96.465056][ T8641] FAULT_INJECTION: forcing a failure. [ 96.465056][ T8641] name fail_page_alloc, interval 1, probability 0, space 0, times 0 [ 96.478453][ T8641] CPU: 1 PID: 8641 Comm: syz-executor668 Not tainted 5.14.0-rc6-syzkaller #0 [ 96.487215][ T8641] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 [ 96.497254][ T8641] Call Trace: [ 96.500517][ T8641] dump_stack_lvl+0x1d3/0x29f [ 96.505758][ T8641] ? show_regs_print_info+0x12/0x12 [ 96.510944][ T8641] ? log_buf_vmcoreinfo_setup+0x498/0x498 [ 96.516652][ T8641] should_fail+0x384/0x4b0 [ 96.521141][ T8641] prepare_alloc_pages+0x1d1/0x5a0 [ 96.526236][ T8641] __alloc_pages+0x14d/0x5f0 [ 96.530808][ T8641] ? __rmqueue_pcplist+0x2030/0x2030 [ 96.536073][ T8641] ? lockdep_hardirqs_on_prepare+0x3e2/0x750 [ 96.542056][ T8641] ? alloc_pages+0x3f3/0x500 [ 96.546635][ T8641] allocate_slab+0xf1/0x540 [ 96.551120][ T8641] ___slab_alloc+0x1cf/0x350 [ 96.555689][ T8641] ? kzalloc+0x1d/0x30 [ 96.559740][ T8641] __kmalloc+0x2e7/0x390 [ 96.563980][ T8641] ? kzalloc+0x1d/0x30 [ 96.568029][ T8641] kzalloc+0x1d/0x30 [ 96.571903][ T8641] security_prepare_creds+0x46/0x220 [ 96.577174][ T8641] prepare_creds+0x411/0x640 [ 96.581747][ T8641] __sys_setfsuid+0xe2/0x3a0 [ 96.586333][ T8641] do_syscall_64+0x3d/0xb0 [ 96.590739][ T8641] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 96.596611][ T8641] RIP: 0033:0x445a69 [ 96.600483][ T8641] Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 11 15 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 [ 96.620152][ T8641] RSP: 002b:00007f1054173318 EFLAGS: 00000246 ORIG_RAX: 000000000000007a [ 96.628543][ T8641] RAX: ffffffffffffffda RBX: 00000000004ca4c8 RCX: 0000000000445a69 [ 96.636600][ T8641] RDX: 0000000000000010 RSI: 00007f10541732f0 RDI: 0000000000000000 [ 96.644550][ T8641] RBP: 00000000004ca4c0 R08: 0000000000000001 R09: 0000000000000000 [ 96.652500][ T8641] R10: 0000000000000000 R11: 0000000000000246 R12: 00000000004ca4cc [ 96.660631][ T8641] R13: 00007fffffe0b62f R14: 00007f1054173400 R15: 0000000000022000 Fixes: 905ae01c4ae2 ("Add a reference to ucounts for each cred") Reported-by: syzbot+01985d7909f9468f013c@syzkaller.appspotmail.com Signed-off-by: Alexey Gladkov <legion@kernel.org> --- kernel/cred.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/cred.c b/kernel/cred.c index e6fd2b3fc31f..f784e08c2fbd 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -286,13 +286,13 @@ struct cred *prepare_creds(void) new->security = NULL; #endif - if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0) - goto error; - new->ucounts = get_ucounts(new->ucounts); if (!new->ucounts) goto error; + if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0) + goto error; + validate_creds(new); return new; @@ -753,13 +753,13 @@ struct cred *prepare_kernel_cred(struct task_struct *daemon) #ifdef CONFIG_SECURITY new->security = NULL; #endif - if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0) - goto error; - new->ucounts = get_ucounts(new->ucounts); if (!new->ucounts) goto error; + if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0) + goto error; + put_cred(old); validate_creds(new); return new; -- 2.32.0 ^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v1] ucounts: Increase ucounts reference counter before the security hook 2021-08-23 16:16 ` [PATCH v1] ucounts: Increase ucounts reference counter before the security hook Alexey Gladkov @ 2021-08-23 21:31 ` Eric W. Biederman 0 siblings, 0 replies; 2+ messages in thread From: Eric W. Biederman @ 2021-08-23 21:31 UTC (permalink / raw) To: Alexey Gladkov; +Cc: LKML, Linux Containers, syzbot+01985d7909f9468f013c Alexey Gladkov <legion@kernel.org> writes: > We need to increment the ucounts reference counter befor security_prepare_creds() > because this function may fail and abort_creds() will try to decrement > this reference. > > [ 96.465056][ T8641] FAULT_INJECTION: forcing a failure. > [ 96.465056][ T8641] name fail_page_alloc, interval 1, probability 0, space 0, times 0 > [ 96.478453][ T8641] CPU: 1 PID: 8641 Comm: syz-executor668 Not tainted 5.14.0-rc6-syzkaller #0 > [ 96.487215][ T8641] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > [ 96.497254][ T8641] Call Trace: > [ 96.500517][ T8641] dump_stack_lvl+0x1d3/0x29f > [ 96.505758][ T8641] ? show_regs_print_info+0x12/0x12 > [ 96.510944][ T8641] ? log_buf_vmcoreinfo_setup+0x498/0x498 > [ 96.516652][ T8641] should_fail+0x384/0x4b0 > [ 96.521141][ T8641] prepare_alloc_pages+0x1d1/0x5a0 > [ 96.526236][ T8641] __alloc_pages+0x14d/0x5f0 > [ 96.530808][ T8641] ? __rmqueue_pcplist+0x2030/0x2030 > [ 96.536073][ T8641] ? lockdep_hardirqs_on_prepare+0x3e2/0x750 > [ 96.542056][ T8641] ? alloc_pages+0x3f3/0x500 > [ 96.546635][ T8641] allocate_slab+0xf1/0x540 > [ 96.551120][ T8641] ___slab_alloc+0x1cf/0x350 > [ 96.555689][ T8641] ? kzalloc+0x1d/0x30 > [ 96.559740][ T8641] __kmalloc+0x2e7/0x390 > [ 96.563980][ T8641] ? kzalloc+0x1d/0x30 > [ 96.568029][ T8641] kzalloc+0x1d/0x30 > [ 96.571903][ T8641] security_prepare_creds+0x46/0x220 > [ 96.577174][ T8641] prepare_creds+0x411/0x640 > [ 96.581747][ T8641] __sys_setfsuid+0xe2/0x3a0 > [ 96.586333][ T8641] do_syscall_64+0x3d/0xb0 > [ 96.590739][ T8641] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 96.596611][ T8641] RIP: 0033:0x445a69 > [ 96.600483][ T8641] Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 11 15 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 > [ 96.620152][ T8641] RSP: 002b:00007f1054173318 EFLAGS: 00000246 ORIG_RAX: 000000000000007a > [ 96.628543][ T8641] RAX: ffffffffffffffda RBX: 00000000004ca4c8 RCX: 0000000000445a69 > [ 96.636600][ T8641] RDX: 0000000000000010 RSI: 00007f10541732f0 RDI: 0000000000000000 > [ 96.644550][ T8641] RBP: 00000000004ca4c0 R08: 0000000000000001 R09: 0000000000000000 > [ 96.652500][ T8641] R10: 0000000000000000 R11: 0000000000000246 R12: 00000000004ca4cc > [ 96.660631][ T8641] R13: 00007fffffe0b62f R14: 00007f1054173400 R15: 0000000000022000 > > Fixes: 905ae01c4ae2 ("Add a reference to ucounts for each cred") > Reported-by: syzbot+01985d7909f9468f013c@syzkaller.appspotmail.com > Signed-off-by: Alexey Gladkov <legion@kernel.org> Applied. It is obvious how a failure of security_prepare_creds calling abort_creds could result a decrement without an increment. So it looks like the fault injection did it's job. Eric > --- > kernel/cred.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/kernel/cred.c b/kernel/cred.c > index e6fd2b3fc31f..f784e08c2fbd 100644 > --- a/kernel/cred.c > +++ b/kernel/cred.c > @@ -286,13 +286,13 @@ struct cred *prepare_creds(void) > new->security = NULL; > #endif > > - if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0) > - goto error; > - > new->ucounts = get_ucounts(new->ucounts); > if (!new->ucounts) > goto error; > > + if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0) > + goto error; > + > validate_creds(new); > return new; > > @@ -753,13 +753,13 @@ struct cred *prepare_kernel_cred(struct task_struct *daemon) > #ifdef CONFIG_SECURITY > new->security = NULL; > #endif > - if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0) > - goto error; > - > new->ucounts = get_ucounts(new->ucounts); > if (!new->ucounts) > goto error; > > + if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0) > + goto error; > + > put_cred(old); > validate_creds(new); > return new; ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-08-23 21:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <000000000000b1f4d305c9ef72ad@google.com>
2021-08-23 16:16 ` [PATCH v1] ucounts: Increase ucounts reference counter before the security hook Alexey Gladkov
2021-08-23 21:31 ` Eric W. Biederman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox