Greetings, Recently, I submitted a patch to permit rules to accept mount calls that specify both MS_REMOUNT and MS_BIND (or MS_REC | MS_BIND). However, there's a bug remaining. There are a few different ways that it could be addressed and I wanted to ask for feedback from someone more familiar with the codebase than myself before I submitted a fix.
The problem occurs in the mnt_rule::gen_policy_re method in parser/mount.cc. When handling rules that specify options=(remount, bind), *two* rules are added to the policy: one to permit calls with MS_REMOUNT | MS_BIND and another that permits all calls to MS_BIND, with all other options masked out! The second rule is almost certainly unintentional. The quickest fixes would be to add a clause to make the if-statements mutually exclusive again, or to convert all of the ifs to a chain of if-elses, if the intention is, in fact, for only one of them to ever apply. However, I'm curious about the motivation for doing the flag masking in the first place. I assume that it's done to prevent the creation of rules that would never match because mount() doesn't permit certain combinations of flags. But that doesn't seem desirable to me, because: * It results in the final rules enforced by the policy being silently different from the rules specified by the user, sometimes in subtle ways; * Different kernel versions might accept different combinations of parameters (the man page for mount hints at this already). It seems to me that it would be better to catch invalid flag combinations during parsing and either fail or emit a warning, instead. I'd be happy to work on a patch to implement this if anyone agrees that it's the way to go. - Ash
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
