linux-kernel - Re: [PATCH] security, lsm: security_old_inode_init

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] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 2 Apr 2023 11:14:33 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Valentin Vidic <vvidic@...entin-vidic.from.hr>
Cc:     James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Christian Brauner <brauner@...nel.org>,
        Dave Chinner <dchinner@...hat.com>,
        Casey Schaufler <casey@...aufler-ca.com>,
        John Johansen <john.johansen@...onical.com>,
        Micah Morton <mortonm@...omium.org>,
        Günther Noack <gnoack3000@...il.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Mickaël Salaün <mic@...ikod.net>,
        linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Vivek Goyal <vgoyal@...hat.com>
Subject: Re: [PATCH] security, lsm: security_old_inode_init_security() Handle
 multi LSM registration

On Sat, Apr 1, 2023 at 5:42 PM Valentin Vidic
<vvidic@...entin-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@...entin-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

-- 
paul-moore.com