| Message ID | 20230401214151.1243189-1-vvidic@valentin-vidic.from.hr (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | security, lsm: security_old_inode_init_security() Handle multi LSM registration | expand |
On Sat, Apr 1, 2023 at 5:42 PM Valentin Vidic <vvidic@valentin-vidic.from.hr> wrote: > > Copying files to an ocfs2 filesystem causes a crash with NULL pointer > dereference in strlen. > > [ 27.386786] BUG: kernel NULL pointer dereference, address: 0000000000000000 > [ 27.386818] #PF: supervisor read access in kernel mode > [ 27.386832] #PF: error_code(0x0000) - not-present page > [ 27.386844] PGD 0 P4D 0=20 > [ 27.386855] Oops: 0000 [#1] PREEMPT SMP PTI > [ 27.386867] CPU: 0 PID: 1792 Comm: cp Not tainted 6.1.0-5-amd64 #1 Debian 6.1.12-1 > [ 27.386887] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 > [ 27.386904] RIP: 0010:strlen+0x0/0x20 > [ 27.386928] Code: b6 07 38 d0 74 14 48 83 c7 01 84 c0 74 05 48 39 f7 75 ec 31 c0 c3 cc cc cc cc 48 89 f8 c3 cc cc cc cc 0f 1f 84 00 00 00 00 00 <80> 3f 00 74 14 48 89 f8 48 83 c0 01 80 38 00 75 f7 48 29 f8 c3 cc > [ 27.386966] RSP: 0018:ffffa33340e4fbc0 EFLAGS: 00010202 > [ 27.386980] RAX: ffff8b578c3b1800 RBX: 0000000000000001 RCX: 0000000000000000 > [ 27.386996] RDX: 0000000000000100 RSI: ffff8b57843d86e8 RDI: 0000000000000000 > [ 27.387012] RBP: ffff8b57849ca608 R08: ffffa33340e4fc7c R09: ffffa33340e4fc84 > [ 27.387027] R10: ffff8b578f1e6000 R11: ffffa33340e4fc80 R12: ffffa33340e4fcb8 > [ 27.387043] R13: ffffa33340e4fc84 R14: 00000000000041c0 R15: ffffa33340e4fc7c > [ 27.387059] FS: 00007f7b36d50500(0000) GS:ffff8b57bec00000(0000) knlGS:0000000000000000 > [ 27.387077] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 27.387091] CR2: 0000000000000000 CR3: 000000003cfe2003 CR4: 0000000000370ef0 > [ 27.387111] Call Trace: > [ 27.387130] <TASK> > [ 27.387141] ocfs2_calc_xattr_init+0x7d/0x330 [ocfs2] > [ 27.387382] ocfs2_mknod+0x471/0x1020 [ocfs2] > [ 27.387471] ? preempt_count_add+0x6a/0xa0 > [ 27.387487] ? _raw_spin_lock+0x13/0x40 > [ 27.387506] ocfs2_mkdir+0x44/0x130 [ocfs2] > [ 27.387583] ? security_inode_mkdir+0x3e/0x70 > [ 27.387598] vfs_mkdir+0x9c/0x140 > [ 27.387617] do_mkdirat+0x142/0x170 > [ 27.387631] __x64_sys_mkdirat+0x47/0x80 > [ 27.387643] do_syscall_64+0x58/0xc0 > [ 27.387659] ? vfs_fstatat+0x5b/0x70 > [ 27.387671] ? __do_sys_newfstatat+0x3f/0x80 > [ 27.387684] ? fpregs_assert_state_consistent+0x22/0x50 > [ 27.387698] ? exit_to_user_mode_prepare+0x3c/0x1c0 > [ 27.387712] ? syscall_exit_to_user_mode+0x17/0x40 > [ 27.387726] ? do_syscall_64+0x67/0xc0 > [ 27.387738] ? exit_to_user_mode_prepare+0x3c/0x1c0 > [ 27.387752] entry_SYSCALL_64_after_hwframe+0x63/0xcd > > Similar to security_dentry_init_security fix in 7f5056b9e7b, the problem > is that ocfs2 checks the return code from security_old_inode_init_security > and if the return code is 0, it assumes everything is fine and continues > to call strlen(name), which crashes. > > Typically SELinux LSM returns 0 and sets name to "security.selinux" and > it is not a problem. Or if SELinux is not compiled in or disabled, it > returns -EOPNOTSUP and ocfs2 deals with it. > > However if BPF LSM is enabled, it registeres every hook and returns the > default return value, in this case 0. > > This patch copies the behaviour of security_dentry_init_security() to > allow only one LSM to initialize security context (or return the default > value of -EOPNOTSUP). > > Signed-off-by: Valentin Vidic <vvidic@valentin-vidic.from.hr> > --- > include/linux/lsm_hook_defs.h | 2 +- > security/security.c | 16 ++++++++++++++-- > 2 files changed, 15 insertions(+), 3 deletions(-) Hi Valentin, Thanks for the problem report and a patch. It's always good to get bug reports, and it's even better when they come with a patch :) If you have the time, could you try a patch we have queued up in the lsm/next branch? We are in the process of removing security_old_inode_init_security() and transitioning all the callers over to security_inode_init_security(), and I believe the ocfs2 patch for this should solve the problem you are seeing, can you test it on your system and let us know? https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/patch/?id=de3004c874e740304cc4f4a83d6200acb511bbda
On Sun, Apr 02, 2023 at 11:14:33AM -0400, Paul Moore wrote: > If you have the time, could you try a patch we have queued up in the > lsm/next branch? We are in the process of removing > security_old_inode_init_security() and transitioning all the callers > over to security_inode_init_security(), and I believe the ocfs2 patch > for this should solve the problem you are seeing, can you test it on > your system and let us know? > > https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/patch/?id=de3004c874e740304cc4f4a83d6200acb511bbda Great, thanks for the pointer. This patch also works for me as I don't see the crash anymore. Can it also be included in the 6.1 LTS kernel since this is were I first noticed the problem?
On Sun, Apr 2, 2023 at 4:03 PM Valentin Vidić <vvidic@valentin-vidic.from.hr> wrote: > > On Sun, Apr 02, 2023 at 11:14:33AM -0400, Paul Moore wrote: > > If you have the time, could you try a patch we have queued up in the > > lsm/next branch? We are in the process of removing > > security_old_inode_init_security() and transitioning all the callers > > over to security_inode_init_security(), and I believe the ocfs2 patch > > for this should solve the problem you are seeing, can you test it on > > your system and let us know? > > > > https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/patch/?id=de3004c874e740304cc4f4a83d6200acb511bbda > > Great, thanks for the pointer. This patch also works for me as I don't > see the crash anymore. Can it also be included in the 6.1 LTS kernel > since this is were I first noticed the problem? I'm glad that solved your problem, thanks for taking the time to test it out. I think backporting it to the stable kernels would be okay, but I'd prefer to let it get some more testing in linux-next first if that's okay with you. Since we are currently at v6.3-rc5 and this patch is scheduled to go up to Linus during the next merge window, it might make the most sense to give this two more weeks in -next, let it land in Linus' tree, and they ask the stable team for a backport ("Option 2" in the stable kernel docs): https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html Thoughts? In addition to the ocfs2 patch mentioned above, there is a similar reiserfs patch which should probably also be backported: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/patch/?id=52ca4b6435a493e47aaa98e7345e19e1e8710b13
On Mon, Apr 03, 2023 at 03:28:11PM -0400, Paul Moore wrote: > I think backporting it to the stable kernels would be okay, but I'd > prefer to let it get some more testing in linux-next first if that's > okay with you. Since we are currently at v6.3-rc5 and this patch is > scheduled to go up to Linus during the next merge window, it might > make the most sense to give this two more weeks in -next, let it land > in Linus' tree, and they ask the stable team for a backport ("Option > 2" in the stable kernel docs): > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > > Thoughts? No problem for me, as long as it gets included in the LTS at some point :)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 094b76dc7164..ea152b6da56b 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -111,7 +111,7 @@ LSM_HOOK(int, 0, path_notify, const struct path *path, u64 mask, unsigned int obj_type) LSM_HOOK(int, 0, inode_alloc_security, struct inode *inode) LSM_HOOK(void, LSM_RET_VOID, inode_free_security, struct inode *inode) -LSM_HOOK(int, 0, inode_init_security, struct inode *inode, +LSM_HOOK(int, -EOPNOTSUPP, inode_init_security, struct inode *inode, struct inode *dir, const struct qstr *qstr, const char **name, void **value, size_t *len) LSM_HOOK(int, 0, inode_init_security_anon, struct inode *inode, diff --git a/security/security.c b/security/security.c index cf6cc576736f..a25d84950a97 100644 --- a/security/security.c +++ b/security/security.c @@ -1164,10 +1164,22 @@ int security_old_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, const char **name, void **value, size_t *len) { + struct security_hook_list *hp; + int rc; + if (unlikely(IS_PRIVATE(inode))) return -EOPNOTSUPP; - return call_int_hook(inode_init_security, -EOPNOTSUPP, inode, dir, - qstr, name, value, len); + + /* + * Only one module will provide a security context. + */ + hlist_for_each_entry(hp, &security_hook_heads.inode_init_security, list) { + rc = hp->hook.inode_init_security(inode, dir, qstr, name, + value, len); + if (rc != LSM_RET_DEFAULT(inode_init_security)) + return rc; + } + return LSM_RET_DEFAULT(inode_init_security); } EXPORT_SYMBOL(security_old_inode_init_security);
Copying files to an ocfs2 filesystem causes a crash with NULL pointer dereference in strlen. [ 27.386786] BUG: kernel NULL pointer dereference, address: 0000000000000000 [ 27.386818] #PF: supervisor read access in kernel mode [ 27.386832] #PF: error_code(0x0000) - not-present page [ 27.386844] PGD 0 P4D 0=20 [ 27.386855] Oops: 0000 [#1] PREEMPT SMP PTI [ 27.386867] CPU: 0 PID: 1792 Comm: cp Not tainted 6.1.0-5-amd64 #1 Debian 6.1.12-1 [ 27.386887] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 [ 27.386904] RIP: 0010:strlen+0x0/0x20 [ 27.386928] Code: b6 07 38 d0 74 14 48 83 c7 01 84 c0 74 05 48 39 f7 75 ec 31 c0 c3 cc cc cc cc 48 89 f8 c3 cc cc cc cc 0f 1f 84 00 00 00 00 00 <80> 3f 00 74 14 48 89 f8 48 83 c0 01 80 38 00 75 f7 48 29 f8 c3 cc [ 27.386966] RSP: 0018:ffffa33340e4fbc0 EFLAGS: 00010202 [ 27.386980] RAX: ffff8b578c3b1800 RBX: 0000000000000001 RCX: 0000000000000000 [ 27.386996] RDX: 0000000000000100 RSI: ffff8b57843d86e8 RDI: 0000000000000000 [ 27.387012] RBP: ffff8b57849ca608 R08: ffffa33340e4fc7c R09: ffffa33340e4fc84 [ 27.387027] R10: ffff8b578f1e6000 R11: ffffa33340e4fc80 R12: ffffa33340e4fcb8 [ 27.387043] R13: ffffa33340e4fc84 R14: 00000000000041c0 R15: ffffa33340e4fc7c [ 27.387059] FS: 00007f7b36d50500(0000) GS:ffff8b57bec00000(0000) knlGS:0000000000000000 [ 27.387077] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 27.387091] CR2: 0000000000000000 CR3: 000000003cfe2003 CR4: 0000000000370ef0 [ 27.387111] Call Trace: [ 27.387130] <TASK> [ 27.387141] ocfs2_calc_xattr_init+0x7d/0x330 [ocfs2] [ 27.387382] ocfs2_mknod+0x471/0x1020 [ocfs2] [ 27.387471] ? preempt_count_add+0x6a/0xa0 [ 27.387487] ? _raw_spin_lock+0x13/0x40 [ 27.387506] ocfs2_mkdir+0x44/0x130 [ocfs2] [ 27.387583] ? security_inode_mkdir+0x3e/0x70 [ 27.387598] vfs_mkdir+0x9c/0x140 [ 27.387617] do_mkdirat+0x142/0x170 [ 27.387631] __x64_sys_mkdirat+0x47/0x80 [ 27.387643] do_syscall_64+0x58/0xc0 [ 27.387659] ? vfs_fstatat+0x5b/0x70 [ 27.387671] ? __do_sys_newfstatat+0x3f/0x80 [ 27.387684] ? fpregs_assert_state_consistent+0x22/0x50 [ 27.387698] ? exit_to_user_mode_prepare+0x3c/0x1c0 [ 27.387712] ? syscall_exit_to_user_mode+0x17/0x40 [ 27.387726] ? do_syscall_64+0x67/0xc0 [ 27.387738] ? exit_to_user_mode_prepare+0x3c/0x1c0 [ 27.387752] entry_SYSCALL_64_after_hwframe+0x63/0xcd Similar to security_dentry_init_security fix in 7f5056b9e7b, the problem is that ocfs2 checks the return code from security_old_inode_init_security and if the return code is 0, it assumes everything is fine and continues to call strlen(name), which crashes. Typically SELinux LSM returns 0 and sets name to "security.selinux" and it is not a problem. Or if SELinux is not compiled in or disabled, it returns -EOPNOTSUP and ocfs2 deals with it. However if BPF LSM is enabled, it registeres every hook and returns the default return value, in this case 0. This patch copies the behaviour of security_dentry_init_security() to allow only one LSM to initialize security context (or return the default value of -EOPNOTSUP). Signed-off-by: Valentin Vidic <vvidic@valentin-vidic.from.hr> --- include/linux/lsm_hook_defs.h | 2 +- security/security.c | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-)