Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFE: add MLS support to scripts/selinux/mdp #44

Closed
pcmoore opened this issue Feb 14, 2019 · 8 comments
Closed

RFE: add MLS support to scripts/selinux/mdp #44

pcmoore opened this issue Feb 14, 2019 · 8 comments

Comments

@pcmoore
Copy link
Member

pcmoore commented Feb 14, 2019

With the popularity of MCS it's virtualization/container policy based on sVirt, it would be nice to add MLS support to the scripts/selinux/mdp tool in the kernel.

@pcmoore
Copy link
Member Author

pcmoore commented Feb 14, 2019

See related issue #45.

@stephensmalley
Copy link
Member

Does anyone use mdp? I don't think it is useful in its current form. You can't use it to even do regression testing of SELinux since selinux-testsuite depends on Fedora/refpolicy-based policies in a variety of ways. And it doesn't provide a good starting point for a new policy since it only generates a single type, has no macros, etc, so anyone writing a new policy would have to look elsewhere (e.g. refpolicy, Android sepolicy, the original example policy). Maybe if scripts/selinux contained a minimalist TE policy with some macros, at least two domains (kernel and init), etc and mdp just generated the boilerplate prefix and postfix, it might be useful. The fs_use_* rules really ought to be generated based on the set of configured filesystems. If we could get to the point where selinux-testsuite worked with the mdp-generated policy, that would be nice too, but that's a lot of work.

@pcmoore
Copy link
Member Author

pcmoore commented Feb 14, 2019

Does anyone use mdp? I don't think it is useful in its current form.

I'm not sure, but my guess is "no", which is part of my concern. A mdp generated policy is always going to be overly simplistic, but I think it should be possible to create a working, full-featured (e.g. provides MLS) SELinux policy for use as a starting point or example.

You can't use it to even do regression testing of SELinux since selinux-testsuite depends on Fedora/refpolicy-based policies in a variety of ways.

I think using the mdp policy as a base for the selinux-testsuite is not a realistic goal at this point. Of course it is possible, and in some ways it would be nice to have that ability, but that is effort that I think could be better spent elsewhere right now.

And it doesn't provide a good starting point for a new policy since it only generates a single type, has no macros, etc, so anyone writing a new policy would have to look elsewhere (e.g. refpolicy, Android sepolicy, the original example policy).

I think it could be a good starting point for a new policy as a simplistic example of the bare minimum needed to get a working SELinux policy. While I agree that having at small number of domains could be beneficial to demonstrate transitions and interactions, even having just a single domain with all of the necessary support and definitions in place is helpful.

Maybe if scripts/selinux contained a minimalist TE policy with some macros, at least two domains (kernel and init), etc and mdp just generated the boilerplate prefix and postfix, it might be useful.

I agree that would be a nice addition. I don't think it removes the need for this feature request, in fact I think adding MLS support to mdp would make the approach you describe much more useful.

The fs_use_* rules really ought to be generated based on the set of configured filesystems.

Agreed.

If we could get to the point where selinux-testsuite worked with the mdp-generated policy, that would be nice too, but that's a lot of work.

Agreed, and not something I think is worth spending a lot of time on at this point. Maybe at some point in the future, but not now.

@stephensmalley
Copy link
Member

stephensmalley commented Feb 14, 2019

To generate a MLS policy that can be compiled via checkpolicy -M, the output of mdp -m would need to be augmented to include:

  • at least one sensitivity declaration (ala "sensitivity s0;")
  • a dominance definition ordering the sensitivities (ala "dominance { s0 }")
  • optionally any category declarations (ala "category c0;") - not strictly required
  • a level declaration per sensitivity with its allowable categories (ala "level s0:c0;")
  • at least one mls constraint (ala "mlsconstrain file { read write } (l1 eq l2 and h1 eq h2);"), possibly auto-generated for every class/permission as with the class definitions,
  • a default level and allowable range definition for each user declaration (appended to the existing user declaration, ala "level s0 range s0 - s0:c0" appended to the user declaration),
  • a level/range field for every security context (appended to every security context in both the policy and the file_contexts, ala ":s0" appended to each security context in an initial sid, fs_use_* or file contexts entry).

Not clear what if anything you could do with it when you're done; if you don't define any mls constraints you won't be restricted in any way by mls and if you require equivalence for all permissions you won't be able to start a process in another level/range.

@stephensmalley
Copy link
Member

Like this un-tested patch,
stephensmalley@e6c52bc

@stephensmalley
Copy link
Member

By un-tested, I mean I didn't try to load the resulting policy. But it did compile ok with checkpolicy -M, and setfiles -c policy.31 file_contexts passed. Logins would probably break though; should probably switch the default level for user_u to just s0 there. Even then, you wouldn't be able to switch to another category if enforcing since it requires equivalence for all permissions and no types are exempted (and since we only have one type, we can't exempt that without effectively disabling mls altogether). So I don't know what use it would have.

@stephensmalley
Copy link
Member

Fixed up the default level and a few nits in stephensmalley@7245f3c

@pcmoore
Copy link
Member Author

pcmoore commented Aug 14, 2019

We have basic MLS support now in mdp, so I'm going to close this out.

@pcmoore pcmoore closed this as completed Aug 14, 2019
pcmoore pushed a commit that referenced this issue Sep 11, 2023
KCSAN has discovered a data race in kernel/workqueue.c:2598:

[ 1863.554079] ==================================================================
[ 1863.554118] BUG: KCSAN: data-race in process_one_work / process_one_work

[ 1863.554142] write to 0xffff963d99d79998 of 8 bytes by task 5394 on cpu 27:
[ 1863.554154] process_one_work (kernel/workqueue.c:2598)
[ 1863.554166] worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2752)
[ 1863.554177] kthread (kernel/kthread.c:389)
[ 1863.554186] ret_from_fork (arch/x86/kernel/process.c:145)
[ 1863.554197] ret_from_fork_asm (arch/x86/entry/entry_64.S:312)

[ 1863.554213] read to 0xffff963d99d79998 of 8 bytes by task 5450 on cpu 12:
[ 1863.554224] process_one_work (kernel/workqueue.c:2598)
[ 1863.554235] worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2752)
[ 1863.554247] kthread (kernel/kthread.c:389)
[ 1863.554255] ret_from_fork (arch/x86/kernel/process.c:145)
[ 1863.554266] ret_from_fork_asm (arch/x86/entry/entry_64.S:312)

[ 1863.554280] value changed: 0x0000000000001766 -> 0x000000000000176a

[ 1863.554295] Reported by Kernel Concurrency Sanitizer on:
[ 1863.554303] CPU: 12 PID: 5450 Comm: kworker/u64:1 Tainted: G             L     6.5.0-rc6+ #44
[ 1863.554314] Hardware name: ASRock X670E PG Lightning/X670E PG Lightning, BIOS 1.21 04/26/2023
[ 1863.554322] Workqueue: btrfs-endio btrfs_end_bio_work [btrfs]
[ 1863.554941] ==================================================================

    lockdep_invariant_state(true);
→   pwq->stats[PWQ_STAT_STARTED]++;
    trace_workqueue_execute_start(work);
    worker->current_func(work);

Moving pwq->stats[PWQ_STAT_STARTED]++; before the line

    raw_spin_unlock_irq(&pool->lock);

resolves the data race without performance penalty.

KCSAN detected at least one additional data race:

[  157.834751] ==================================================================
[  157.834770] BUG: KCSAN: data-race in process_one_work / process_one_work

[  157.834793] write to 0xffff9934453f77a0 of 8 bytes by task 468 on cpu 29:
[  157.834804] process_one_work (/home/marvin/linux/kernel/linux_torvalds/kernel/workqueue.c:2606)
[  157.834815] worker_thread (/home/marvin/linux/kernel/linux_torvalds/./include/linux/list.h:292 /home/marvin/linux/kernel/linux_torvalds/kernel/workqueue.c:2752)
[  157.834826] kthread (/home/marvin/linux/kernel/linux_torvalds/kernel/kthread.c:389)
[  157.834834] ret_from_fork (/home/marvin/linux/kernel/linux_torvalds/arch/x86/kernel/process.c:145)
[  157.834845] ret_from_fork_asm (/home/marvin/linux/kernel/linux_torvalds/arch/x86/entry/entry_64.S:312)

[  157.834859] read to 0xffff9934453f77a0 of 8 bytes by task 214 on cpu 7:
[  157.834868] process_one_work (/home/marvin/linux/kernel/linux_torvalds/kernel/workqueue.c:2606)
[  157.834879] worker_thread (/home/marvin/linux/kernel/linux_torvalds/./include/linux/list.h:292 /home/marvin/linux/kernel/linux_torvalds/kernel/workqueue.c:2752)
[  157.834890] kthread (/home/marvin/linux/kernel/linux_torvalds/kernel/kthread.c:389)
[  157.834897] ret_from_fork (/home/marvin/linux/kernel/linux_torvalds/arch/x86/kernel/process.c:145)
[  157.834907] ret_from_fork_asm (/home/marvin/linux/kernel/linux_torvalds/arch/x86/entry/entry_64.S:312)

[  157.834920] value changed: 0x000000000000052a -> 0x0000000000000532

[  157.834933] Reported by Kernel Concurrency Sanitizer on:
[  157.834941] CPU: 7 PID: 214 Comm: kworker/u64:2 Tainted: G             L     6.5.0-rc7-kcsan-00169-g81eaf55a60fc #4
[  157.834951] Hardware name: ASRock X670E PG Lightning/X670E PG Lightning, BIOS 1.21 04/26/2023
[  157.834958] Workqueue: btrfs-endio btrfs_end_bio_work [btrfs]
[  157.835567] ==================================================================

in code:

        trace_workqueue_execute_end(work, worker->current_func);
→       pwq->stats[PWQ_STAT_COMPLETED]++;
        lock_map_release(&lockdep_map);
        lock_map_release(&pwq->wq->lockdep_map);

which needs to be resolved separately.

Fixes: 725e8ec ("workqueue: Add pwq->stats[] and a monitoring script")
Cc: Tejun Heo <[email protected]>
Suggested-by: Lai Jiangshan <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Mirsad Goran Todorovac <[email protected]>
Signed-off-by: Tejun Heo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants