| Message ID | 20211215162014.957848-1-smayhew@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | selinux: Fix sleeping function called from invalid context | expand |
On Wed, Dec 15, 2021 at 11:20 AM Scott Mayhew <smayhew@redhat.com> wrote: > > selinux_sb_mnt_opts_compat() is called via sget_fc() under the sb_lock > spinlock, so it can't use GFP_KERNEL allocations: > > [ 868.565200] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:230 > [ 868.568246] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 4914, name: mount.nfs > [ 868.569626] preempt_count: 1, expected: 0 > [ 868.570215] RCU nest depth: 0, expected: 0 > [ 868.570809] Preemption disabled at: > [ 868.570810] [<0000000000000000>] 0x0 > [ 868.571848] CPU: 1 PID: 4914 Comm: mount.nfs Kdump: loaded Tainted: G W 5.16.0-rc5.2585cf9dfa #1 > [ 868.573273] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-4.fc34 04/01/2014 > [ 868.574478] Call Trace: > [ 868.574844] <TASK> > [ 868.575156] dump_stack_lvl+0x34/0x44 > [ 868.575692] __might_resched.cold+0xd6/0x10f > [ 868.576308] slab_pre_alloc_hook.constprop.0+0x89/0xf0 > [ 868.577046] __kmalloc_track_caller+0x72/0x420 > [ 868.577684] ? security_context_to_sid_core+0x48/0x2b0 > [ 868.578569] kmemdup_nul+0x22/0x50 > [ 868.579108] security_context_to_sid_core+0x48/0x2b0 > [ 868.579854] ? _nfs4_proc_pathconf+0xff/0x110 [nfsv4] > [ 868.580742] ? nfs_reconfigure+0x80/0x80 [nfs] > [ 868.581355] security_context_str_to_sid+0x36/0x40 > [ 868.581960] selinux_sb_mnt_opts_compat+0xb5/0x1e0 > [ 868.582550] ? nfs_reconfigure+0x80/0x80 [nfs] > [ 868.583098] security_sb_mnt_opts_compat+0x2a/0x40 > [ 868.583676] nfs_compare_super+0x113/0x220 [nfs] > [ 868.584249] ? nfs_try_mount_request+0x210/0x210 [nfs] > [ 868.584879] sget_fc+0xb5/0x2f0 > [ 868.585267] nfs_get_tree_common+0x91/0x4a0 [nfs] > [ 868.585834] vfs_get_tree+0x25/0xb0 > [ 868.586241] fc_mount+0xe/0x30 > [ 868.586605] do_nfs4_mount+0x130/0x380 [nfsv4] > [ 868.587160] nfs4_try_get_tree+0x47/0xb0 [nfsv4] > [ 868.587724] vfs_get_tree+0x25/0xb0 > [ 868.588193] do_new_mount+0x176/0x310 > [ 868.588782] __x64_sys_mount+0x103/0x140 > [ 868.589388] do_syscall_64+0x3b/0x90 > [ 868.589935] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 868.590699] RIP: 0033:0x7f2b371c6c4e > [ 868.591239] Code: 48 8b 0d dd 71 0e 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d aa 71 0e 00 f7 d8 64 89 01 48 > [ 868.593810] RSP: 002b:00007ffc83775d88 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 > [ 868.594691] RAX: ffffffffffffffda RBX: 00007ffc83775f10 RCX: 00007f2b371c6c4e > [ 868.595504] RDX: 0000555d517247a0 RSI: 0000555d51724700 RDI: 0000555d51724540 > [ 868.596317] RBP: 00007ffc83775f10 R08: 0000555d51726890 R09: 0000555d51726890 > [ 868.597162] R10: 0000000000000000 R11: 0000000000000246 R12: 0000555d51726890 > [ 868.598005] R13: 0000000000000003 R14: 0000555d517246e0 R15: 0000555d511ac925 > [ 868.598826] </TASK> > > Fixes: 69c4a42d72eb ("lsm,selinux: add new hook to compare new mount to an existing mount") > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > --- > security/selinux/hooks.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 62d30c0a30c2..534e8a4747f8 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -611,10 +611,11 @@ static int bad_option(struct superblock_security_struct *sbsec, char flag, > return 0; > } > > -static int parse_sid(struct super_block *sb, const char *s, u32 *sid) > +static int __parse_sid(struct super_block *sb, const char *s, u32 *sid, > + gfp_t gfp) > { > int rc = security_context_str_to_sid(&selinux_state, s, > - sid, GFP_KERNEL); > + sid, gfp); > if (rc) > pr_warn("SELinux: security_context_str_to_sid" > "(%s) failed for (dev %s, type %s) errno=%d\n", > @@ -622,6 +623,11 @@ static int parse_sid(struct super_block *sb, const char *s, u32 *sid) > return rc; > } > > +static int parse_sid(struct super_block *sb, const char *s, u32 *sid) > +{ > + return __parse_sid(sb, s, sid, GFP_KERNEL); > +} Thanks for both reporting the problem and supplying a patch to fix it! I think I would prefer to see parse_sid() updated to take a gfp parameter and skip the __parse_sid()/parse_sid() wrapper. Yes, the patch will be a bit more intrusive with all of the caller updates, but we only have to backport up to v5.13 and the resulting code will be cleaner moving forward.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 62d30c0a30c2..534e8a4747f8 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -611,10 +611,11 @@ static int bad_option(struct superblock_security_struct *sbsec, char flag, return 0; } -static int parse_sid(struct super_block *sb, const char *s, u32 *sid) +static int __parse_sid(struct super_block *sb, const char *s, u32 *sid, + gfp_t gfp) { int rc = security_context_str_to_sid(&selinux_state, s, - sid, GFP_KERNEL); + sid, gfp); if (rc) pr_warn("SELinux: security_context_str_to_sid" "(%s) failed for (dev %s, type %s) errno=%d\n", @@ -622,6 +623,11 @@ static int parse_sid(struct super_block *sb, const char *s, u32 *sid) return rc; } +static int parse_sid(struct super_block *sb, const char *s, u32 *sid) +{ + return __parse_sid(sb, s, sid, GFP_KERNEL); +} + /* * Allow filesystems with binary mount data to explicitly set mount point * labeling information. @@ -2702,14 +2708,14 @@ static int selinux_sb_mnt_opts_compat(struct super_block *sb, void *mnt_opts) return (sbsec->flags & SE_MNTMASK) ? 1 : 0; if (opts->fscontext) { - rc = parse_sid(sb, opts->fscontext, &sid); + rc = __parse_sid(sb, opts->fscontext, &sid, GFP_NOWAIT); if (rc) return 1; if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, sid)) return 1; } if (opts->context) { - rc = parse_sid(sb, opts->context, &sid); + rc = __parse_sid(sb, opts->context, &sid, GFP_NOWAIT); if (rc) return 1; if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, sid)) @@ -2719,14 +2725,14 @@ static int selinux_sb_mnt_opts_compat(struct super_block *sb, void *mnt_opts) struct inode_security_struct *root_isec; root_isec = backing_inode_security(sb->s_root); - rc = parse_sid(sb, opts->rootcontext, &sid); + rc = __parse_sid(sb, opts->rootcontext, &sid, GFP_NOWAIT); if (rc) return 1; if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, sid)) return 1; } if (opts->defcontext) { - rc = parse_sid(sb, opts->defcontext, &sid); + rc = __parse_sid(sb, opts->defcontext, &sid, GFP_NOWAIT); if (rc) return 1; if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, sid))
selinux_sb_mnt_opts_compat() is called via sget_fc() under the sb_lock spinlock, so it can't use GFP_KERNEL allocations: [ 868.565200] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:230 [ 868.568246] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 4914, name: mount.nfs [ 868.569626] preempt_count: 1, expected: 0 [ 868.570215] RCU nest depth: 0, expected: 0 [ 868.570809] Preemption disabled at: [ 868.570810] [<0000000000000000>] 0x0 [ 868.571848] CPU: 1 PID: 4914 Comm: mount.nfs Kdump: loaded Tainted: G W 5.16.0-rc5.2585cf9dfa #1 [ 868.573273] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-4.fc34 04/01/2014 [ 868.574478] Call Trace: [ 868.574844] <TASK> [ 868.575156] dump_stack_lvl+0x34/0x44 [ 868.575692] __might_resched.cold+0xd6/0x10f [ 868.576308] slab_pre_alloc_hook.constprop.0+0x89/0xf0 [ 868.577046] __kmalloc_track_caller+0x72/0x420 [ 868.577684] ? security_context_to_sid_core+0x48/0x2b0 [ 868.578569] kmemdup_nul+0x22/0x50 [ 868.579108] security_context_to_sid_core+0x48/0x2b0 [ 868.579854] ? _nfs4_proc_pathconf+0xff/0x110 [nfsv4] [ 868.580742] ? nfs_reconfigure+0x80/0x80 [nfs] [ 868.581355] security_context_str_to_sid+0x36/0x40 [ 868.581960] selinux_sb_mnt_opts_compat+0xb5/0x1e0 [ 868.582550] ? nfs_reconfigure+0x80/0x80 [nfs] [ 868.583098] security_sb_mnt_opts_compat+0x2a/0x40 [ 868.583676] nfs_compare_super+0x113/0x220 [nfs] [ 868.584249] ? nfs_try_mount_request+0x210/0x210 [nfs] [ 868.584879] sget_fc+0xb5/0x2f0 [ 868.585267] nfs_get_tree_common+0x91/0x4a0 [nfs] [ 868.585834] vfs_get_tree+0x25/0xb0 [ 868.586241] fc_mount+0xe/0x30 [ 868.586605] do_nfs4_mount+0x130/0x380 [nfsv4] [ 868.587160] nfs4_try_get_tree+0x47/0xb0 [nfsv4] [ 868.587724] vfs_get_tree+0x25/0xb0 [ 868.588193] do_new_mount+0x176/0x310 [ 868.588782] __x64_sys_mount+0x103/0x140 [ 868.589388] do_syscall_64+0x3b/0x90 [ 868.589935] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 868.590699] RIP: 0033:0x7f2b371c6c4e [ 868.591239] Code: 48 8b 0d dd 71 0e 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d aa 71 0e 00 f7 d8 64 89 01 48 [ 868.593810] RSP: 002b:00007ffc83775d88 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 [ 868.594691] RAX: ffffffffffffffda RBX: 00007ffc83775f10 RCX: 00007f2b371c6c4e [ 868.595504] RDX: 0000555d517247a0 RSI: 0000555d51724700 RDI: 0000555d51724540 [ 868.596317] RBP: 00007ffc83775f10 R08: 0000555d51726890 R09: 0000555d51726890 [ 868.597162] R10: 0000000000000000 R11: 0000000000000246 R12: 0000555d51726890 [ 868.598005] R13: 0000000000000003 R14: 0000555d517246e0 R15: 0000555d511ac925 [ 868.598826] </TASK> Fixes: 69c4a42d72eb ("lsm,selinux: add new hook to compare new mount to an existing mount") Signed-off-by: Scott Mayhew <smayhew@redhat.com> --- security/selinux/hooks.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)