SELinuxProject / selinux-testsuite

This is the upstream SELinux testsuite which is designed as a basic set of regression tests for the SELinux kernel functionality.
GNU General Public License v2.0
54 stars 43 forks source link

Add checks for mounter permissions, entrypoint and setfattr/getfattr calls #4

Closed rhatdan closed 7 years ago

rhatdan commented 8 years ago

This seems to be failing where it should not. Not sure if these is an issue in the kernel or in the test suite.

I am seeing this AVC required when trying to create a file in a directory that mounter can use but client can not, in the case where the mounter is using context=LABEL.

allow test_overlay_client_t test_overlay_mounter_files_t:dir getattr;

rhatdan commented 8 years ago

@rhvgoyal PTAL

rhvgoyal commented 8 years ago

Ran new tests. I think in entrypoint tests stderr needs to be redirected.

Bad Entrypoint tests.

Attempting to enter domain with bad entrypoint, should fail. runcon: ‘./container1/merged/badentrypoint’: Permission denied ok 114

rhvgoyal commented 8 years ago

Also it is complaining about running less tests than planned.

Looks like you planned 122 tests but ran 119.

stephensmalley commented 7 years ago

What is the status on these tests? Still see a failure on rawhide.

rhvgoyal commented 7 years ago

@stephensmalley One of the patch is yet to be merged upstream. Once that gets merged, failure on rawhide should go away.

This overlay patch where we first switch to mounter's cred before doing getattr. Otherwise getatt fails.

rhvgoyal commented 7 years ago

@stephensmalley @pcmoore I tested this PR with -rc1 kernel and these tests are now passing. Can we merge this too.

rhvgoyal commented 7 years ago

BTW, with -rc1 kernel, doing "make -C policy/ load" throws lots of backtraces and warnings about locking. It might be a kernel selinux issue.

``vm5-f24 login: [ 53.983102] ------------[ cut here ]------------ [ 53.985841] WARNING: CPU: 3 PID: 1628 at kernel/softirq.c:161 local_bh_enable_ip+0x9d/0xc0 [ 53.987111] Modules linked in: bridge stp llc xt_conntrack nfnetlink iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_security dm_thin_pool dm_persistent_data dm_bio_prison libcrc32c joydev ppdev crct10dif_pclmul crc32_pclmul ghash_clmulni_intel virtio_balloon parport_pc parport nfsd i2c_piix4 acpi_cpufreq auth_rpcgss nfs_acl lockd grace sunrpc virtio_console virtio_blk virtio_net qxl drm_kms_helper ttm ata_generic drm crc32c_intel serio_raw pata_acpi virtio_pci virtio_ring virtio [ 53.994983] CPU: 3 PID: 1628 Comm: setsebool Not tainted 4.9.0-rc1 #196 [ 53.995984] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.2-20150714_191134- 04/01/2014 [ 53.997433] ffffc90002623b90 ffffffff8144b0e3 0000000000000000 0000000000000000 [ 53.998633] ffffc90002623bd0 ffffffff810b3b1b 000000a181742906 0000000000000201 [ 53.999826] ffffffff81742924 ffffffff81f18640 ffff88022effa000 ffff88022f16b000 [ 54.001050] Call Trace: [ 54.001460] [] dump_stack+0x86/0xc3 [ 54.002253] [] warn+0xcb/0xf0 [ 54.003573] [] ? peernet2id+0x54/0x80 [ 54.004400] [] warn_slowpath_null+0x1d/0x20 [ 54.005301] [] local_bh_enable_ip+0x9d/0xc0 [ 54.006225] [] _raw_spin_unlock_bh+0x35/0x40 [ 54.007119] [] peernet2id+0x54/0x80 [ 54.007916] [] netlink_broadcast_filtered+0x216/0x3b0 [ 54.008945] [] netlink_broadcast+0x1d/0x20 [ 54.009827] [] audit_log_end+0x2bf/0x2d0 [ 54.010686] [] ? audit_log_end+0x30/0x2d0 [ 54.011548] [] audit_log+0x6a/0x90 [ 54.012331] [] security_set_bools+0xee/0x200 [ 54.013236] [] sel_commit_bools_write+0xc7/0x120 [ 54.014201] [] vfs_write+0x37/0x140 [ 54.015013] [] ? rcu_read_lock_sched_held+0x45/0x80 [ 54.016006] [] ? rcu_sync_lockdep_assert+0x2f/0x60 [ 54.016987] [] ? sb_start_write+0xd0/0x1d0 [ 54.017889] [] ? vfs_write+0x17d/0x1a0 [ 54.018718] [] vfs_write+0xb5/0x1a0 [ 54.019514] [] SyS_write+0x58/0xc0 [ 54.020292] [] entry_SYSCALL_64_fastpath+0x1f/0xc2 [ 54.021287] ---[ end trace b7493f5e8734e551 ]--- [ 54.021994] [ 54.022242] ================================= [ 54.022904] [ INFO: inconsistent lock state ] [ 54.023577] 4.9.0-rc1 #196 Tainted: G W
[ 54.024341] --------------------------------- [ 54.024999] inconsistent {IN-SOFTIRQ-R} -> {SOFTIRQ-ON-W} usage. [ 54.025921] setsebool/1628 [HC0[0]:SC0[0]:HE1:SE1] takes: [ 54.026745]([ 54.026988] policy_rwlock){+++?..}[ 54.027561] , at: [ 54.027897] [] security_set_bools+0x27/0x200 [ 54.028789] {IN-SOFTIRQ-R} state was registered at: [ 54.029544] [ 54.029791] []
lock_acquire+0x565/0x12a0 [ 54.030676] [ 54.030914] [] lock_acquire+0xf6/0x1f0 [ 54.031749] [ 54.031991] [] _raw_read_lock+0x3b/0x50 [ 54.032848] [ 54.033090] [] security_netlbl_sid_to_secattr+0x37/0xc0 [ 54.034144] [ 54.034397] [] selinux_netlbl_inet_conn_request+0x6f/0x100 [ 54.035496] [ 54.035738] [] selinux_inet_conn_request+0x71/0xb0 [ 54.036722] [ 54.036965] [] security_inet_conn_request+0x43/0x60 [ 54.037968] [ 54.038218] [] tcp_conn_request+0x3a7/0xab0 [ 54.039103] [ 54.039350] [] tcp_v4_conn_request+0x82/0xb0 [ 54.040261] [ 54.040505] [] tcp_rcv_state_process+0x1ac/0xf20 [ 54.041472] [ 54.041716] [] tcp_v4_do_rcv+0xb2/0x200 [ 54.042561] [ 54.042801] [] tcp_v4_rcv+0xb57/0xba0 [ 54.043620] [ 54.043862] [] ip_local_deliver_finish+0xe6/0x380 [ 54.044843] [ 54.045087] [] ip_local_deliver+0x74/0x220 [ 54.045969] [ 54.046219] [] ip_rcv_finish+0x17a/0x540 [ 54.047076] [ 54.047328] [] ip_rcv+0x293/0x4e0 [ 54.048091] [ 54.048338] [] netif_receive_skb_core+0x34b/0xc70 [ 54.049338] [ 54.049582] [] __netif_receive_skb+0x18/0x60 [ 54.050489] [ 54.050734] [] netif_receive_skb_internal+0xc0/0x200 [ 54.051744] [ 54.051986] [] napi_gro_receive+0x13c/0x200 [ 54.052883] [ 54.053128] [] virtnet_receive+0x425/0x8e0 [virtio_net] [ 54.054189] [ 54.054429] [] virtnet_poll+0x1d/0x80 [virtio_net] [ 54.055425] [ 54.055665] [] net_rx_action+0x1d1/0x400 [ 54.056522] [ 54.056762] [] do_softirq+0xc5/0x4a3 [ 54.057598] [ 54.057838] [] irq_exit+0xf7/0x100 [ 54.058614] [ 54.058853] [] do_IRQ+0x6a/0x120 [ 54.059609] [ 54.059850] [] ret_from_intr+0x0/0x20 [ 54.060671] [ 54.060912] [] default_idle+0x25/0x190 [ 54.061746] [ 54.061988] [] arch_cpu_idle+0xf/0x20 [ 54.062808] [ 54.063060] [] default_idle_call+0x23/0x40 [ 54.063949] [ 54.064198] [] cpu_startup_entry+0x1d5/0x250 [ 54.065105] [ 54.065354] [] rest_init+0x135/0x140 [ 54.066153] [ 54.066404] [] start_kernel+0x45e/0x47f [ 54.067249] [ 54.067490] [] x86_64_start_reservations+0x2a/0x2c [ 54.068925] [ 54.069174] [] x86_64_start_kernel+0x14c/0x16f [ 54.070125] irq event stamp: 4850 [ 54.070647] hardirqs last enabled at (4847): [ 54.071306] [] mutex_lock_nested+0x25d/0x3c0 [ 54.072218] hardirqs last disabled at (4848): [ 54.072871] [] _raw_write_lock_irq+0x1d/0x60 [ 54.073781] softirqs last enabled at (4850): [ 54.074442] [] peernet2id+0x54/0x80 [ 54.075226] softirqs last disabled at (4849): [ 54.075877] [] peernet2id+0x36/0x80 [ 54.076665] [ 54.076665] other info that might help us debug this: [ 54.077658] Possible unsafe locking scenario: [ 54.077658] [ 54.078559] CPU0 [ 54.078941] ---- [ 54.079331] lock([ 54.079636] policy_rwlock [ 54.080058] ); [ 54.080332] [ 54.080734] lock([ 54.081060] policy_rwlock [ 54.081492] ); [ 54.081758] [ 54.081758] * DEADLOCK * [ 54.081758] [ 54.084265] 3 locks held by setsebool/1628: [ 54.084902] #0: [ 54.085189](sb_writers[ 54.085619] #9){.+.+.+}[ 54.086036] , at: [ 54.086380] [] vfs_write+0x17d/0x1a0 [ 54.087153] #1: [ 54.087440](sel_mutex[ 54.087858]){+.+.+.} , at: [ 54.088334] [] sel_commit_bools_write+0x3f/0x120 [ 54.089283] #2: [ 54.089560](policy_rwlock[ 54.090028]){+++?..} , at: [ 54.090508] [] security_set_bools+0x27/0x200

pcmoore commented 7 years ago

The locking problem existed in 4.9-rc0 as well, this was the locking problem I mentioned in the other thread. I'm currently working on trying to isolate the change but I have a fair number of distractions these days so progress is slow.

stephensmalley commented 7 years ago

For that one at least, in retrospect it was a mistake to add audit_log() calls inside the write_lock_irq() in security_set_bools(). We could split that into an audit phase and the actual boolean update phase, with only the latter under write_lock_irq. Interleaving policy load isn't possible because sel_write_load() and sel_write_bool() both take and hold sel_mutex, so the policy can't change underneath security_set_bools().

stephensmalley commented 7 years ago

All tests pass with 4.9-rc1

stephensmalley commented 7 years ago

NM, that's not the only case triggering these stack traces, e.g. security_task_setpgid() is called while holding write_lock_irq(&tasklist_lock). So I guess we need a more general fix.

stephensmalley commented 7 years ago

[ 74.757661] Call Trace: [ 74.757669] [] dump_stack+0x63/0x86 [ 74.757670] [] warn+0xcb/0xf0 [ 74.757671] [] warn_slowpath_null+0x1d/0x20 [ 74.757671] [] local_bh_enable_ip+0x6b/0x80 [ 74.757678] [] _raw_spin_unlock_bh+0x1a/0x20 [ 74.757681] [] peernet2id+0x51/0x80 [ 74.757684] [] netlink_broadcast_filtered+0x21e/0x3c0 [ 74.757685] [] netlink_broadcast+0x1d/0x20 [ 74.757688] [] audit_log_end+0x16a/0x190 [ 74.757697] [] ? securityfs_remove+0x90/0x90 [ 74.757698] [] common_lsm_audit+0x207/0x730 [ 74.757701] [] ? filemap_map_pages+0x4c3/0x4e0 [ 74.757703] [] slow_avc_audit+0x6a/0xa0 [ 74.757704] [] avc_has_perm+0x172/0x1a0 [ 74.757705] [] current_has_perm+0x3d/0x40 [ 74.757705] [] selinux_task_setpgid+0x13/0x20 [ 74.757706] [] security_task_setpgid+0x3b/0x50 [ 74.757709] [] SyS_setpgid+0x15d/0x1c0 [ 74.757710] [] entry_SYSCALL_64_fastpath+0x1a/0xa9 [ 74.757711] ---[ end trace c6e01b9ab8bd045c ]---

stephensmalley commented 7 years ago

Revert this commit and stack dumps go away. commit bc51dddf98c907b598e645ae4b277ed1295b6d5f Author: WANG Cong xiyou.wangcong@gmail.com Date: Thu Sep 1 21:53:45 2016 -0700

netns: avoid disabling irq for netns id

We never read or change netns id in hardirq context,
the only place we read netns id in softirq context
is in vxlan_xmit(). So, it should be enough to just
disable BH.

Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>