Update xperm parsing

native/src/sepolicy/api.cpp

@@ -9,7 +9,7 @@ static void dprint(const char *action, Args ...args) {
     std::string s(action);
     for (int i = 0; i < sizeof...(args); ++i) s += " %s";
     s += "\n";
-    LOGD(s.data(), (args ? args : "*")...);
+    LOGD(s.data(), as_str(args)...);
 }
 #else
 #define dprint(...)
@@ -35,19 +35,19 @@ bool sepolicy::dontaudit(const char *s, const char *t, const char *c, const char
     return impl->add_rule(s, t, c, p, AVTAB_AUDITDENY, true);
 }
 
-bool sepolicy::allowxperm(const char *s, const char *t, const char *c, const char *range) {
-    dprint(__FUNCTION__, s, t, c, "ioctl", range);
-    return impl->add_xperm_rule(s, t, c, range, AVTAB_XPERMS_ALLOWED, false);
+bool sepolicy::allowxperm(const char *s, const char *t, const char *c, const argument &xperm) {
+    dprint(__FUNCTION__, s, t, c, "ioctl", xperm);
+    return impl->add_xperm_rule(s, t, c, xperm, AVTAB_XPERMS_ALLOWED);
 }
 
-bool sepolicy::auditallowxperm(const char *s, const char *t, const char *c, const char *range) {
-    dprint(__FUNCTION__, s, t, c, "ioctl", range);
-    return impl->add_xperm_rule(s, t, c, range, AVTAB_XPERMS_AUDITALLOW, false);
+bool sepolicy::auditallowxperm(const char *s, const char *t, const char *c, const argument &xperm) {
+    dprint(__FUNCTION__, s, t, c, "ioctl", xperm);
+    return impl->add_xperm_rule(s, t, c, xperm, AVTAB_XPERMS_AUDITALLOW);
 }
 
-bool sepolicy::dontauditxperm(const char *s, const char *t, const char *c, const char *range) {
-    dprint(__FUNCTION__, s, t, c, "ioctl", range);
-    return impl->add_xperm_rule(s, t, c, range, AVTAB_XPERMS_DONTAUDIT, false);
+bool sepolicy::dontauditxperm(const char *s, const char *t, const char *c, const argument &xperm) {
+    dprint(__FUNCTION__, s, t, c, "ioctl", xperm);
+    return impl->add_xperm_rule(s, t, c, xperm, AVTAB_XPERMS_DONTAUDIT);
 }
 
 bool sepolicy::type_change(const char *s, const char *t, const char *c, const char *d) {

native/src/sepolicy/include/sepolicy.hpp

@@ -5,7 +5,14 @@
 
 #include <selinux.hpp>
 
-#define ALL nullptr
+using token_list = std::vector<const char *>;
+using argument = std::pair<token_list, bool>;
+using argument_list = std::vector<argument>;
+
+const argument &all_xperm();
+
+#define ALL       nullptr
+#define ALL_XPERM all_xperm()
 
 struct sepolicy {
     using c_str = const char *;
@@ -38,9 +45,9 @@ struct sepolicy {
     bool dontaudit(c_str src, c_str tgt, c_str cls, c_str perm);
 
     // Extended permissions access vector rules
-    bool allowxperm(c_str src, c_str tgt, c_str cls, c_str range);
-    bool auditallowxperm(c_str src, c_str tgt, c_str cls, c_str range);
-    bool dontauditxperm(c_str src, c_str tgt, c_str cls, c_str range);
+    bool allowxperm(c_str src, c_str tgt, c_str cls, const argument &xperm);
+    bool auditallowxperm(c_str src, c_str tgt, c_str cls, const argument &xperm);
+    bool dontauditxperm(c_str src, c_str tgt, c_str cls, const argument &xperm);
 
     // Type rules
     bool type_transition(c_str src, c_str tgt, c_str cls, c_str def, c_str obj = nullptr);

native/src/sepolicy/policy.hpp

@@ -8,12 +8,14 @@
 #include "policy-rs.hpp"
 
 struct sepol_impl : public sepolicy {
+    avtab_ptr_t find_avtab_node(avtab_key_t *key, avtab_extended_perms_t *xperms);
+    avtab_ptr_t insert_avtab_node(avtab_key_t *key);
     avtab_ptr_t get_avtab_node(avtab_key_t *key, avtab_extended_perms_t *xperms);
+
     bool add_rule(const char *s, const char *t, const char *c, const char *p, int effect, bool invert);
     void add_rule(type_datum_t *src, type_datum_t *tgt, class_datum_t *cls, perm_datum_t *perm, int effect, bool invert);
-    void add_xperm_rule(type_datum_t *src, type_datum_t *tgt,
-            class_datum_t *cls, uint16_t low, uint16_t high, int effect, bool invert);
-    bool add_xperm_rule(const char *s, const char *t, const char *c, const char *range, int effect, bool invert);
+    void add_xperm_rule(type_datum_t *src, type_datum_t *tgt, class_datum_t *cls, const argument &xperm, int effect);
+    bool add_xperm_rule(const char *s, const char *t, const char *c, const argument &xperm, int effect);
     bool add_type_rule(const char *s, const char *t, const char *c, const char *d, int effect);
     bool add_filename_trans(const char *s, const char *t, const char *c, const char *d, const char *o);
     bool add_genfscon(const char *fs_name, const char *path, const char *context);
@@ -31,4 +33,8 @@ struct sepol_impl : public sepolicy {
 
 #define impl reinterpret_cast<sepol_impl *>(this)
 
+const char *as_str(const argument &arg);
+const char *as_str(const char *arg);
+
 void statement_help();
+void test_parse_statements();

native/src/sepolicy/rules.cpp

@@ -23,9 +23,9 @@ void sepolicy::magisk_rules() {
     allow(SEPOL_PROC_DOMAIN, ALL, ALL, ALL);
     // Allow us to do any ioctl
     if (impl->db->policyvers >= POLICYDB_VERSION_XPERMS_IOCTL) {
-        allowxperm(SEPOL_PROC_DOMAIN, ALL, "blk_file", ALL);
-        allowxperm(SEPOL_PROC_DOMAIN, ALL, "fifo_file", ALL);
-        allowxperm(SEPOL_PROC_DOMAIN, ALL, "chr_file", ALL);
+        allowxperm(SEPOL_PROC_DOMAIN, ALL, "blk_file", ALL_XPERM);
+        allowxperm(SEPOL_PROC_DOMAIN, ALL, "fifo_file", ALL_XPERM);
+        allowxperm(SEPOL_PROC_DOMAIN, ALL, "chr_file", ALL_XPERM);
     }
 
     // Create unconstrained file type

native/src/sepolicy/sepolicy.cpp

@@ -2,6 +2,8 @@
 
 #include "policy.hpp"
 
+using namespace std;
+
 // Invert is adding rules for auditdeny; in other cases, invert is removing rules
 #define strip_av(effect, invert) ((effect == AVTAB_AUDITDENY) == !invert)
 
@@ -101,38 +103,42 @@ static bool is_redundant(avtab_ptr_t node) {
     }
 }
 
-avtab_ptr_t sepol_impl::get_avtab_node(avtab_key_t *key, avtab_extended_perms_t *xperms) {
+avtab_ptr_t sepol_impl::find_avtab_node(avtab_key_t *key, avtab_extended_perms_t *xperms) {
     avtab_ptr_t node;
 
-    /* AVTAB_XPERMS entries are not necessarily unique */
+    // AVTAB_XPERMS entries are not necessarily unique
     if (key->specified & AVTAB_XPERMS) {
-        bool match = false;
+        if (xperms == nullptr)
+            return nullptr;
         node = avtab_search_node(&db->te_avtab, key);
         while (node) {
             if ((node->datum.xperms->specified == xperms->specified) &&
                 (node->datum.xperms->driver == xperms->driver)) {
-                match = true;
+                node = nullptr;
                 break;
             }
             node = avtab_search_node_next(node, key->specified);
         }
-        if (!match)
-            node = nullptr;
     } else {
         node = avtab_search_node(&db->te_avtab, key);
     }
 
-    if (!node) {
-        avtab_datum_t avdatum{};
-        /*
-         * AUDITDENY, aka DONTAUDIT, are &= assigned, versus |= for
-         * others. Initialize the data accordingly.
-         */
-        avdatum.data = key->specified == AVTAB_AUDITDENY ? ~0U : 0U;
-        /* this is used to get the node - insertion is actually unique */
-        node = avtab_insert_nonunique(&db->te_avtab, key, &avdatum);
-    }
+    return node;
+}
 
+avtab_ptr_t sepol_impl::insert_avtab_node(avtab_key_t *key) {
+    avtab_datum_t avdatum{};
+    // AUDITDENY, aka DONTAUDIT, are &= assigned, versus |= for others.
+    // Initialize the data accordingly.
+    avdatum.data = key->specified == AVTAB_AUDITDENY ? ~0U : 0U;
+    return avtab_insert_nonunique(&db->te_avtab, key, &avdatum);
+}
+
+avtab_ptr_t sepol_impl::get_avtab_node(avtab_key_t *key, avtab_extended_perms_t *xperms) {
+    avtab_ptr_t node = find_avtab_node(key, xperms);
+    if (!node) {
+        node = insert_avtab_node(key);
+    }
     return node;
 }
 
@@ -239,19 +245,18 @@ bool sepol_impl::add_rule(const char *s, const char *t, const char *c, const cha
 #define ioctl_driver(x) (x>>8 & 0xFF)
 #define ioctl_func(x) (x & 0xFF)
 
-void sepol_impl::add_xperm_rule(type_datum_t *src, type_datum_t *tgt,
-        class_datum_t *cls, uint16_t low, uint16_t high, int effect, bool invert) {
+void sepol_impl::add_xperm_rule(type_datum_t *src, type_datum_t *tgt, class_datum_t *cls, const argument &xperm, int effect) {
     if (src == nullptr) {
         for_each_attr(db->p_types.table, [&](type_datum_t *type) {
-            add_xperm_rule(type, tgt, cls, low, high, effect, invert);
+            add_xperm_rule(type, tgt, cls, xperm, effect);
         });
     } else if (tgt == nullptr) {
         for_each_attr(db->p_types.table, [&](type_datum_t *type) {
-            add_xperm_rule(src, type, cls, low, high, effect, invert);
+            add_xperm_rule(src, type, cls, xperm, effect);
         });
     } else if (cls == nullptr) {
         hashtab_for_each(db->p_classes.table, [&](hashtab_ptr_t node) {
-            add_xperm_rule(src, tgt, auto_cast(node->datum), low, high, effect, invert);
+            add_xperm_rule(src, tgt, auto_cast(node->datum), xperm, effect);
         });
     } else {
         avtab_key_t key;
@@ -260,46 +265,130 @@ void sepol_impl::add_xperm_rule(type_datum_t *src, type_datum_t *tgt,
         key.target_class = cls->s.value;
         key.specified = effect;
 
-        avtab_datum_t *datum;
-        avtab_extended_perms_t xperms;
+        // Each key may contain 1 driver node and 256 function nodes
+        avtab_ptr_t node_list[257] = { nullptr };
+#define driver_node (node_list[256])
 
-        memset(&xperms, 0, sizeof(xperms));
-        if (ioctl_driver(low) != ioctl_driver(high)) {
-            xperms.specified = AVTAB_XPERMS_IOCTLDRIVER;
-            xperms.driver = 0;
-        } else {
-            xperms.specified = AVTAB_XPERMS_IOCTLFUNCTION;
-            xperms.driver = ioctl_driver(low);
+        // Find all rules with key
+        for (avtab_ptr_t node = avtab_search_node(&db->te_avtab, &key); node;) {
+            if (node->datum.xperms->specified == AVTAB_XPERMS_IOCTLDRIVER) {
+                driver_node = node;
+            } else if (node->datum.xperms->specified == AVTAB_XPERMS_IOCTLFUNCTION) {
+                node_list[node->datum.xperms->driver] = node;
+            }
+            node = avtab_search_node_next(node, key.specified);
         }
 
-        datum = &get_avtab_node(&key, &xperms)->datum;
-        if (datum->xperms != nullptr)
-            memcpy(xperms.perms, datum->xperms->perms, sizeof(xperms.perms));
+        bool reset = xperm.second;
+        vector<pair<uint16_t, uint16_t>> ranges;
 
-        if (xperms.specified == AVTAB_XPERMS_IOCTLDRIVER) {
-            for (int i = ioctl_driver(low); i <= ioctl_driver(high); ++i) {
-                if (invert)
-                    xperm_clear(i, xperms.perms);
-                else
-                    xperm_set(i, xperms.perms);
+        for (const char *tok : xperm.first) {
+            uint16_t low = 0;
+            uint16_t high = 0;
+            if (tok == nullptr) {
+                low = 0x0000;
+                high = 0xFF00;
+                reset = true;
+            } else if (strchr(tok, '-')) {
+                if (sscanf(tok, "%hx-%hx", &low, &high) != 2) {
+                    // Invalid token, skip
+                    continue;
+                }
+            } else {
+                if (sscanf(tok, "%hx", &low) != 1) {
+                    // Invalid token, skip
+                    continue;
+                }
+                high = low;
             }
-        } else {
-            for (int i = ioctl_func(low); i <= ioctl_func(high); ++i) {
-                if (invert)
-                    xperm_clear(i, xperms.perms);
-                else
-                    xperm_set(i, xperms.perms);
+            if (high == 0) {
+                reset = true;
+            } else {
+                ranges.emplace_back(make_pair(low, high));
             }
         }
 
-        if (datum->xperms == nullptr)
-            datum->xperms = auto_cast(malloc(sizeof(xperms)));
+        if (reset) {
+            for (int i = 0; i <= 0xFF; ++i) {
+                if (node_list[i]) {
+                    avtab_remove_node(&db->te_avtab, node_list[i]);
+                    node_list[i] = nullptr;
+                }
+            }
+            if (driver_node) {
+                memset(driver_node->datum.xperms->perms, 0, sizeof(avtab_extended_perms_t::perms));
+            }
+        }
 
-        memcpy(datum->xperms, &xperms, sizeof(xperms));
+        auto new_driver_node = [&]() -> avtab_ptr_t {
+            auto node = insert_avtab_node(&key);
+            node->datum.xperms = auto_cast(calloc(1, sizeof(avtab_extended_perms_t)));
+            node->datum.xperms->specified = AVTAB_XPERMS_IOCTLDRIVER;
+            node->datum.xperms->driver = 0;
+            return node;
+        };
+
+        auto new_func_node = [&](uint8_t driver) -> avtab_ptr_t {
+            auto node = insert_avtab_node(&key);
+            node->datum.xperms = auto_cast(calloc(1, sizeof(avtab_extended_perms_t)));
+            node->datum.xperms->specified = AVTAB_XPERMS_IOCTLFUNCTION;
+            node->datum.xperms->driver = driver;
+            return node;
+        };
+
+        if (!xperm.second) {
+            for (auto [low, high] : ranges) {
+                if (ioctl_driver(low) != ioctl_driver(high)) {
+                    if (driver_node == nullptr) {
+                        driver_node = new_driver_node();
+                    }
+                    for (int i = ioctl_driver(low); i <= ioctl_driver(high); ++i) {
+                        xperm_set(i, driver_node->datum.xperms->perms);
+                    }
+                } else {
+                    uint8_t driver = ioctl_driver(low);
+                    auto node = node_list[driver];
+                    if (node == nullptr) {
+                        node = new_func_node(driver);
+                        node_list[driver] = node;
+                    }
+                    for (int i = ioctl_func(low); i <= ioctl_func(high); ++i) {
+                        xperm_set(i, node->datum.xperms->perms);
+                    }
+                }
+            }
+        } else {
+            if (driver_node == nullptr) {
+                driver_node = new_driver_node();
+            }
+            // Fill the driver perms
+            memset(driver_node->datum.xperms->perms, ~0, sizeof(avtab_extended_perms_t::perms));
+
+            for (auto [low, high] : ranges) {
+                if (ioctl_driver(low) != ioctl_driver(high)) {
+                    for (int i = ioctl_driver(low); i <= ioctl_driver(high); ++i) {
+                        xperm_clear(i, driver_node->datum.xperms->perms);
+                    }
+                } else {
+                    uint8_t driver = ioctl_driver(low);
+                    auto node = node_list[driver];
+                    if (node == nullptr) {
+                        node = new_func_node(driver);
+                        // Fill the func perms
+                        memset(node->datum.xperms->perms, ~0, sizeof(avtab_extended_perms_t::perms));
+                        node_list[driver] = node;
+                    }
+                    xperm_clear(driver, driver_node->datum.xperms->perms);
+                    for (int i = ioctl_func(low); i <= ioctl_func(high); ++i) {
+                        xperm_clear(i, node->datum.xperms->perms);
+                    }
+                }
+            }
+        }
     }
 }
 
-bool sepol_impl::add_xperm_rule(const char *s, const char *t, const char *c, const char *range, int effect, bool invert) {
+bool sepol_impl::add_xperm_rule(const char *s, const char *t, const char *c, const argument &xperm, int effect) {
     type_datum_t *src = nullptr, *tgt = nullptr;
     class_datum_t *cls = nullptr;
 
@@ -327,21 +416,7 @@ bool sepol_impl::add_xperm_rule(const char *s, const char *t, const char *c, con
         }
     }
 
-    uint16_t low, high;
-
-    if (range) {
-        if (strchr(range, '-')){
-            sscanf(range, "%hx-%hx", &low, &high);
-        } else {
-            sscanf(range, "%hx", &low);
-            high = low;
-        }
-    } else {
-        low = 0;
-        high = 0xFFFF;
-    }
-
-    add_xperm_rule(src, tgt, cls, low, high, effect, invert);
+    add_xperm_rule(src, tgt, cls, xperm, effect);
     return true;
 }
 

native/src/sepolicy/statement.cpp

@@ -8,6 +8,13 @@
 
 using namespace std;
 
+const argument &all_xperm() {
+    static argument arg;
+    if (arg.first.empty())
+        arg.first.push_back(nullptr);
+    return arg;
+}
+
 static const char *type_msg_1 =
 R"EOF("allow *source_type *target_type *class *perm_set"
 "deny *source_type *target_type *class *perm_set"
@@ -19,15 +26,23 @@ static const char *type_msg_2 =
 R"EOF("allowxperm *source_type *target_type *class operation xperm_set"
 "auditallowxperm *source_type *target_type *class operation xperm_set"
 "dontauditxperm *source_type *target_type *class operation xperm_set"
-- The only supported operation is 'ioctl'
-- xperm_set format is either 'low-high', 'value', or '*'.
-  '*' will be treated as '0x0000-0xFFFF'.
-  All values should be written in hexadecimal.
+- The only supported operation right now is 'ioctl'
+- xperm_set is one or multiple hexadecimal numeric values ranging from 0x0000 to 0xFFFF.
+  Multiple values consist of a space separated list enclosed in braces ({}).
+  Use the complement operator (~) to specify all permissions except those explicitly listed.
+  Use the range operator (-) to specify all permissions within the low – high range.
+  Use the match all operator (*) to match all ioctl commands.
+  The special value 0 is used to clear all rules.
+  Some examples:
+  allowxperm source target class ioctl 0x8910
+  allowxperm source target class ioctl { 0x8910-0x8926 0x892A-0x8935 }
+  allowxperm source target class ioctl ~{ 0x8910 0x892A }
+  allowxperm source target class ioctl *
 )EOF";
 
 static const char *type_msg_3 =
-R"EOF("permissive ^type"
-"enforce ^type"
+R"EOF("permissive *type"
+"enforce *type"
 )EOF";
 
 static const char *type_msg_4 =
@@ -64,8 +79,8 @@ this means each policy statement should be enclosed in quotes.
 Multiple policy statements can be provided in a single command.
 
 Statements has a format of "<rule_name> [args...]".
-Arguments labeled with (^) can accept one or more entries. Multiple
-entries consist of a space separated list enclosed in braces ({}).
+Arguments labeled with (^) can accept one or more entries.
+Multiple entries consist of a space separated list enclosed in braces ({}).
 Arguments labeled with (*) are the same as (^), but additionally
 support the match-all operator (*).
 
@@ -93,101 +108,230 @@ type_msg_5, type_msg_6, type_msg_7, type_msg_8, type_msg_9);
     exit(0);
 }
 
-using parsed_tokens = vector<vector<const char *>>;
-
-static bool tokenize_string(char *stmt, parsed_tokens &arr) {
-    // cur is the pointer to where the top level is parsing
+// Return value:
+//  0: success
+// -1: unclosed bracket
+// -2: nested brackets
+// -3: double complement
+static int tokenize_string(char *stmt, argument_list &args) {
     char *cur = stmt;
-    for (char *tok; (tok = strtok_r(nullptr, " ", &cur)) != nullptr;) {
-        vector<const char *> token;
-        if (tok[0] == '{') {
-            // cur could point to somewhere in the braces, restore the string
+    bool complement = false;
+
+    auto add_new_arg = [&] (char *str) {
+        argument arg;
+        for (char *tok; (tok = strtok_r(nullptr, " ", &str)) != nullptr;) {
+            if (tok == "*"sv) {
+                // If any of the tokens is "*", the result is a single nullptr
+                arg.first.clear();
+                arg.first.push_back(nullptr);
+                break;
+            } else {
+                arg.first.push_back(tok);
+            }
+        }
+        arg.second = complement;
+        complement = false;
+        args.push_back(std::move(arg));
+    };
+
+    char *tok = strtok_r(nullptr, " ", &cur);
+    while (tok) {
+        if (tok[0] == '~') {
+            if (complement) {
+                // Double complement is not supported
+                return -3;
+            }
+            complement = true;
+            if (tok[1] == '\0') {
+                // If '~' is followed by a space, find the next token
+                tok = strtok_r(nullptr, " ", &cur);
+            } else {
+                // Reparse the rest of the string
+                ++tok;
+            }
+            continue;
+        }
+
+        // Find brackets
+        char *begin = strchr(tok, '{');
+        char *end = strchr(tok, '}');
+
+        if (begin == nullptr && end) {
+            // Bracket not closed, syntax error
+            return -1;
+        }
+
+        if (begin) {
+            if (end && (begin > end)) {
+                // Bracket not closed, syntax error
+                return -1;
+            }
+
+            // Restore the string so we can properly find the closing bracket
             if (cur)
                 cur[-1] = ' ';
-            ++tok;
-            char *end = strchr(tok, '}');
+
             if (end == nullptr) {
-                // Bracket not closed, syntax error
-                return false;
+                // Find again
+                end = strchr(begin + 1, '}');
+                if (end == nullptr) {
+                    // Bracket not closed, syntax error
+                    return -1;
+                }
             }
+
+            // Close bracket and start the next parsing iteration after that
             *end = '\0';
-            for (char *sub_tok; (sub_tok = strtok_r(nullptr, " ", &tok)) != nullptr;)
-                token.push_back(sub_tok);
             cur = end + 1;
-        } else if (tok[0] == '*') {
-            token.push_back(nullptr);
+
+            if (strchr(begin + 1, '{')) {
+                // We don't support nested brackets
+                return -2;
+            }
+
+            if (begin != tok) {
+                // There is an argument before the opening bracket
+                *begin = '\0';
+                add_new_arg(tok);
+            }
+
+            // Parse the arguments enclosed in the brackets
+            add_new_arg(begin + 1);
         } else {
-            token.push_back(tok);
+            add_new_arg(tok);
         }
-        arr.push_back(std::move(token));
+
+        tok = strtok_r(nullptr, " ", &cur);
     }
-    return true;
+    return 0;
 }
 
-// Check array size and all args listed in 'ones' have size = 1 (no multiple entries)
-template <int size, int ...ones>
-static bool check_tokens(parsed_tokens &arr) {
-    if (arr.size() != size)
-        return false;
-    initializer_list<int> list{ones...};
+// Check all args listed have size = 1 (no multiple entries)
+template <int ...indices>
+static bool enforce_single(const argument_list &args) {
+    initializer_list<int> list{indices...};
     for (int i : list)
-        if (arr[i].size() != 1)
+        if (args[i].first.size() != 1)
             return false;
     return true;
 }
 
-template <int size, int ...ones>
-static bool tokenize_and_check(char *stmt, parsed_tokens &arr) {
-    return tokenize_string(stmt, arr) && check_tokens<size, ones...>(arr);
+// Check all args listed are not null (no match all operator)
+template <int ...indices>
+static bool enforce_non_null(const argument_list &args) {
+    initializer_list<int> list{indices...};
+    for (int i : list)
+        if (args[i].first.size() == 1 && args[i].first[0] == nullptr)
+            return false;
+    return true;
 }
 
+template <int size>
+static bool enforce_size(const argument_list &args) {
+    return args.size() == size;
+}
+
+// Check all args are not complements except those listed
+template <int ...except>
+static bool check_complements(const argument_list &args) {
+    initializer_list<int> list{except...};
+    for (int i = 0; i < args.size(); ++i) {
+        bool disallow = true;
+        for (int e : list) {
+            if (i == e) {
+                disallow = false;
+                break;
+            }
+        }
+        if (disallow && args[i].second)
+            return false;
+    }
+    return true;
+}
+
+// Tokenize and check argument count
+template <int size>
+static bool tokenize_and_check(char *stmt, argument_list &args) {
+    return tokenize_string(stmt, args) == 0 &&
+        enforce_size<size>(args) &&
+        check_complements<>(args);
+}
+
+#define sprint(...) off += ssprintf(buf + off, sizeof(buf) - off, __VA_ARGS__)
+
+const char *as_str(const argument &arg) {
+    size_t off = 0;
+    static char buf[4096];
+
+    if (arg.second) {
+        sprint("~");
+    }
+    sprint("{ ");
+    for (const char *tok : arg.first) {
+        if (tok == nullptr) {
+            sprint("0x0000-0xFF00 ");
+        } else {
+            sprint("%s ", tok);
+        }
+    }
+    sprint("}");
+
+    return buf;
+}
+
+const char *as_str(const char *arg) { return arg ?: "*"; }
+
 template <typename Func, typename ...Args>
 static void run_and_check(const Func &fn, const char *action, Args ...args) {
     if (!fn(args...)) {
         string s = "Error in: %s";
         for (int i = 0; i < sizeof...(args); ++i) s += " %s";
         s += "\n";
-        LOGW(s.data(), action, (args ? args : "*")...);
+        LOGW(s.data(), action, as_str(args)...);
     }
 }
 
 #define run_fn(...) run_and_check(fn, action, __VA_ARGS__)
 
-// Pattern 1: allow { source } { target } { class } { permission }
+// Pattern 1: allow *{ source } *{ target } *{ class } *{ permission }
 template <typename Func>
 static bool parse_pattern_1(const Func &fn, const char *action, char *stmt) {
-    parsed_tokens arr;
-    if (!tokenize_and_check<4>(stmt, arr))
+    argument_list args;
+    if (!tokenize_and_check<4>(stmt, args))
         return false;
-    for (auto src : arr[0])
-        for (auto tgt : arr[1])
-            for (auto cls : arr[2])
-                for (auto perm : arr[3])
+    for (auto src : args[0].first)
+        for (auto tgt : args[1].first)
+            for (auto cls : args[2].first)
+                for (auto perm : args[3].first)
                     run_fn(src, tgt, cls, perm);
     return true;
 }
 
-// Pattern 2: allowxperm { source } { target } { class } ioctl range
+// Pattern 2: allowxperm *{ source } *{ target } *{ class } ioctl xperm_set
 template <typename Func>
 static bool parse_pattern_2(const Func &fn, const char *action, char *stmt) {
-    parsed_tokens arr;
-    if (!tokenize_and_check<5, 3, 4>(stmt, arr) || arr[3][0] != "ioctl"sv)
+    argument_list args;
+    if (tokenize_string(stmt, args) != 0 ||
+        !enforce_size<5>(args) ||
+        !check_complements<4>(args) ||
+        !enforce_single<3>(args) ||
+        !enforce_non_null<3>(args) ||
+        args[3].first[0] != "ioctl"sv)
         return false;
-    auto range = arr[4][0];
-    for (auto src : arr[0])
-        for (auto tgt : arr[1])
-            for (auto cls : arr[2])
-                run_fn(src, tgt, cls, range);
+    for (auto src : args[0].first)
+        for (auto tgt : args[1].first)
+            for (auto cls : args[2].first)
+                run_fn(src, tgt, cls, args[4]);
     return true;
 }
 
-// Pattern 3: permissive { type }
+// Pattern 3: permissive *{ type }
 template <typename Func>
 static bool parse_pattern_3(const Func &fn, const char *action, char *stmt) {
-    parsed_tokens arr;
-    if (!tokenize_and_check<1>(stmt, arr))
+    argument_list args;
+    if (!tokenize_and_check<1>(stmt, args))
         return false;
-    for (auto type : arr[0])
+    for (auto type : args[0].first)
         run_fn(type);
     return true;
 }
@@ -195,11 +339,11 @@ static bool parse_pattern_3(const Func &fn, const char *action, char *stmt) {
 // Pattern 4: typeattribute { type } { attribute }
 template <typename Func>
 static bool parse_pattern_4(const Func &fn, const char *action, char *stmt) {
-    parsed_tokens arr;
-    if (!tokenize_and_check<2>(stmt, arr))
+    argument_list args;
+    if (!tokenize_and_check<2>(stmt, args) || !enforce_non_null<0, 1>(args))
         return false;
-    for (auto type : arr[0])
-        for (auto attr : arr[1])
+    for (auto type : args[0].first)
+        for (auto attr : args[1].first)
             run_fn(type, attr);
     return true;
 }
@@ -207,61 +351,65 @@ static bool parse_pattern_4(const Func &fn, const char *action, char *stmt) {
 // Pattern 5: type name { attribute }
 template <typename Func>
 static bool parse_pattern_5(const Func &fn, const char *action, char *stmt) {
-    parsed_tokens arr;
+    argument_list args;
     string tmp_str;
-    if (!tokenize_string(stmt, arr))
+    if (tokenize_string(stmt, args) != 0 || !enforce_single<0>(args))
         return false;
-    if (arr.size() == 1) {
-        arr.emplace_back(initializer_list<const char*>{ "domain" });
+    if (args.size() == 1) {
+        args.emplace_back(make_pair(initializer_list<const char*>{ "domain" }, false));
     }
-    if (!check_tokens<2, 0>(arr))
+    if (!enforce_size<2>(args) || !enforce_non_null<1>(args) || !check_complements<>(args))
         return false;
-    for (auto attr : arr[1])
-        run_fn(arr[0][0], attr);
+    for (auto attr : args[1].first)
+        run_fn(args[0].first[0], attr);
     return true;
 }
 
 // Pattern 6: attribute name
 template <typename Func>
 static bool parse_pattern_6(const Func &fn, const char *action, char *stmt) {
-    parsed_tokens arr;
-    if (!tokenize_and_check<1, 0>(stmt, arr))
+    argument_list args;
+    if (!tokenize_and_check<1>(stmt, args) || !enforce_single<0>(args))
         return false;
-    run_fn(arr[0][0]);
+    run_fn(args[0].first[0]);
     return true;
 }
 
 // Pattern 7: type_transition source target class default (filename)
 template <typename Func>
 static bool parse_pattern_7(const Func &fn, const char *action, char *stmt) {
-    parsed_tokens arr;
-    if (!tokenize_string(stmt, arr))
+    argument_list args;
+    if (tokenize_string(stmt, args) != 0)
         return false;
-    if (arr.size() == 4)
-        arr.emplace_back(initializer_list<const char*>{nullptr});
-    if (!check_tokens<5, 0, 1, 2, 3, 4>(arr))
+    if (args.size() == 4)
+        args.emplace_back(make_pair(initializer_list<const char*>{ nullptr }, false));
+    if (!enforce_size<5>(args) ||
+        !enforce_non_null<0, 1, 2, 3>(args) ||
+        !enforce_single<0, 1, 2, 3, 4>(args) ||
+        !check_complements<>(args))
         return false;
-    run_fn(arr[0][0], arr[1][0], arr[2][0], arr[3][0], arr[4][0]);
+    run_fn(args[0].first[0], args[1].first[0], args[2].first[0],
+           args[3].first[0], args[4].first[0]);
     return true;
 }
 
 // Pattern 8: type_change source target class default
 template <typename Func>
 static bool parse_pattern_8(const Func &fn, const char *action, char *stmt) {
-    parsed_tokens arr;
-    if (!tokenize_and_check<4, 0, 1, 2, 3>(stmt, arr))
+    argument_list args;
+    if (!tokenize_and_check<4>(stmt, args) || !enforce_single<0, 1, 2, 3>(args))
         return false;
-    run_fn(arr[0][0], arr[1][0], arr[2][0], arr[3][0]);
+    run_fn(args[0].first[0], args[1].first[0], args[2].first[0], args[3].first[0]);
     return true;
 }
 
 // Pattern 9: genfscon name path context
 template <typename Func>
 static bool parse_pattern_9(const Func &fn, const char *action, char *stmt) {
-    parsed_tokens arr;
-    if (!tokenize_and_check<3, 0, 1, 2>(stmt, arr))
+    argument_list args;
+    if (!tokenize_and_check<3>(stmt, args) || !enforce_single<0, 1, 2>(args))
         return false;
-    run_fn(arr[0][0], arr[1][0], arr[2][0]);
+    run_fn(args[0].first[0], args[1].first[0], args[2].first[0]);
     return true;
 }
 
@@ -310,3 +458,119 @@ void sepolicy::parse_statement(const char *stmt, int len) {
 
     else { LOGW("Unknown action: '%s'\n\n", action); }
 }
+
+// Parsing is hard, the following is a test suite to ensure correctness
+
+static void test_parse_stmt(const char *stmt, const char *expect, int code = 0) {
+    auto print_error = [](int code) {
+        switch (code) {
+            case -1:
+                fprintf(stderr, "unclosed bracket\n");
+                break;
+            case -2:
+                fprintf(stderr, "nested brackets\n");
+                break;
+            case -3:
+                fprintf(stderr, "double complement\n");
+                break;
+        }
+    };
+
+    char cpy[4096];
+    strncpy(cpy, stmt, sizeof(cpy));
+    argument_list args;
+    int result = tokenize_string(cpy, args);
+    if (result != 0) {
+        if (expect != nullptr) {
+            fprintf(stderr, "Parsing: '%s' with unexpected error: ", stmt);
+            print_error(result);
+            exit(1);
+        } else {
+            if (code != result) {
+                fprintf(stderr, "Parsing: '%s'\n", stmt);
+                fprintf(stderr, "Expect error: ");
+                print_error(code);
+                fprintf(stderr, "Result error: ");
+                print_error(result);
+                exit(1);
+            }
+            return;
+        }
+    }
+
+    char buf[4096];
+    size_t off = 0;
+    for (const auto &arg : args) {
+        sprint("(%d,[", arg.second);
+        bool first = true;
+        for (const char *tok : arg.first) {
+            if (first) {
+                first = false;
+            } else {
+                sprint(",");
+            }
+            sprint("'%s'", tok ?: "(null)");
+        }
+        sprint("])\n");
+    }
+    if (strncmp(buf, expect, sizeof(buf)) != 0) {
+        fprintf(stderr, "Parsing: '%s'\n", stmt);
+        fprintf(stderr, "Expect:\n%s", expect);
+        fprintf(stderr, "-------------------\n");
+        fprintf(stderr, "Result:\n%s", buf);
+        fprintf(stderr, "-------------------\n");
+        exit(1);
+    }
+}
+
+static void assert_msg(bool b, const char *msg) {
+    if (!b) {
+        fprintf(stderr, "Assertion failed: %s\n", msg);
+        exit(1);
+    }
+}
+
+[[maybe_unused]]
+void test_parse_statements() {
+    // Test parsing correctness
+    test_parse_stmt("a b c",
+                    "(0,['a'])\n(0,['b'])\n(0,['c'])\n");
+    test_parse_stmt("  a   b {c}",
+                    "(0,['a'])\n(0,['b'])\n(0,['c'])\n");
+    test_parse_stmt("a b{c }",
+                    "(0,['a'])\n(0,['b'])\n(0,['c'])\n");
+    test_parse_stmt("a b{c}d",
+                    "(0,['a'])\n(0,['b'])\n(0,['c'])\n(0,['d'])\n");
+    test_parse_stmt("a b {   c d  }",
+                    "(0,['a'])\n(0,['b'])\n(0,['c','d'])\n");
+    test_parse_stmt("a {b} ~c",
+                    "(0,['a'])\n(0,['b'])\n(1,['c'])\n");
+    test_parse_stmt("a b ~ { c d }",
+                    "(0,['a'])\n(0,['b'])\n(1,['c','d'])\n");
+    test_parse_stmt("a b *",
+                    "(0,['a'])\n(0,['b'])\n(0,['(null)'])\n");
+    test_parse_stmt("a b { c * }",
+                    "(0,['a'])\n(0,['b'])\n(0,['(null)'])\n");
+
+    // Invalid syntax tests
+    test_parse_stmt("a b { c", nullptr, -1);
+    test_parse_stmt("a {b}}c}", nullptr, -1);
+    test_parse_stmt("a }b{c}", nullptr, -1);
+    test_parse_stmt("{a} b } {c}", nullptr, -1);
+    test_parse_stmt("a { b } { c", nullptr, -1);
+    test_parse_stmt("a b {{ c }}", nullptr, -2);
+    test_parse_stmt("a b ~ ~c", nullptr, -3);
+    test_parse_stmt("a b ~~c", nullptr, -3);
+
+    // Test enforcement functions
+    string s = "a * { b c } { d e } ~f *";
+    argument_list args;
+    assert_msg(tokenize_string(s.data(), args) == 0, "parse failure");
+    assert_msg(enforce_size<6>(args), "size != 6");
+    assert_msg(enforce_non_null<0, 2, 3, 4>(args), "non-null enforcement failed");
+    assert_msg(!enforce_non_null<0, 1, 2, 3, 4>(args), "non-null enforcement should fail");
+    assert_msg(enforce_single<0, 1, 4, 5>(args), "single enforcement check failed");
+    assert_msg(!enforce_single<0, 1, 2>(args), "single enforcement check should fail");
+    assert_msg(check_complements<4>(args), "check complements failed");
+    assert_msg(!check_complements<>(args), "check complements should fail");
+}